Skip to content

Commit 3ca4de3

Browse files
dkackmanclaude
andcommitted
fix(mcp): #553 - fill silent concat_videos inputs, load a string pair_audio video
concat_videos was skipping a silent input among videos where at least one other carried audio, contributing zero duration to the joined track and shifting every later shot's audio early. It now fills that input's own length with silence, matching join_into_song's rule (#513) that a shot with no track is silence for its length rather than a gap. Requires fps to size the silence, as every other length-dependent path here already does; every catalog template calling concat_videos already passes it. When no input carries audio, output audio still stays None. pair_audio already loaded a string 'audio' path via load_audio, but not a string 'video' - it now loads a video path/URL/{"location": ...} dict the same way concat_videos loads one of its own inputs, confined the same way audio's own path already is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8375c4e commit 3ca4de3

4 files changed

Lines changed: 150 additions & 10 deletions

File tree

‎dw/tasks/concat_videos.py‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import logging
1212
import os
1313

14+
import numpy
15+
1416
from ..events import emit_log, emit_warning
1517
from ..result import AudioVideo
1618
from ..shots import measured_num_samples, nested_shots, shot_record, trimmed_shots
@@ -226,6 +228,18 @@ def concat_videos(
226228
# long shows up here as the samples it actually took (#378)
227229
shots = []
228230

231+
# Whether the joined track carries anything at all - decides whether a
232+
# silent input among these gets silence of its own length or is skipped
233+
# outright, matching join_into_song's rule that a shot with no track is
234+
# silence for its length rather than a gap that shifts everything after
235+
# it early (#513, #553)
236+
has_audio = any(waveform is not None for waveform in waveforms)
237+
silence_channels = (
238+
next(waveform.shape[0] for waveform in waveforms if waveform is not None)
239+
if has_audio
240+
else None
241+
)
242+
229243
for index, (video, clip) in enumerate(zip(videos, clips)):
230244
head_trim = trim_frames if index > 0 else 0
231245
start_frame = len(frames)
@@ -236,7 +250,7 @@ def concat_videos(
236250
video_shots = nested_shots(
237251
trimmed_shots(inner, head_trim),
238252
start_frame,
239-
start_sample if waveforms[index] is not None else None,
253+
start_sample if has_audio else None,
240254
getattr(video, "sample_rate", None),
241255
sample_rate,
242256
)
@@ -258,13 +272,31 @@ def concat_videos(
258272
shot["source_index"] = index
259273
shots.extend(video_shots)
260274

261-
if waveforms[index] is None:
275+
if not has_audio:
262276
continue
263277

264-
waveform = waveforms[index]
278+
if waveforms[index] is None:
279+
if fps is None:
280+
raise ValueError(
281+
"concat_videos needs 'fps' to fill silence for a video "
282+
"with no audio track of its own"
283+
)
284+
waveform = numpy.zeros(
285+
(silence_channels, frames_to_samples(len(clip), fps, sample_rate)),
286+
dtype=numpy.float32,
287+
)
288+
emit_log(
289+
f"concat_videos: {names[index]} carries no audio - filled with "
290+
"silence",
291+
command="concat_videos",
292+
video=names[index],
293+
)
294+
else:
295+
waveform = waveforms[index]
296+
265297
if audio is None:
266298
audio = waveform
267-
audio_native_rate = video.sample_rate
299+
audio_native_rate = getattr(video, "sample_rate", None)
268300
continue
269301

270302
if head_trim > 0 and fps is None:
@@ -294,7 +326,7 @@ def concat_videos(
294326
crossfade_ms,
295327
seam_fade_ms,
296328
)
297-
audio_native_rate = video.sample_rate
329+
audio_native_rate = getattr(video, "sample_rate", None)
298330

299331
# The rate the caller declared, else the rate the first input carries -
300332
# either beats the result's 8 fps default (#84)

‎dw/tasks/pair_audio.py‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,13 @@ def pair_audio(video, audio, sample_rate=None, fps=None, fit=None):
214214
215215
Args:
216216
video: The frames - a frame list, a frame array or tensor (or a
217-
pipeline's batch of one of those, unwrapped), or an
218-
AudioVideo whose own soundtrack is replaced by this one; the
217+
pipeline's batch of one of those, unwrapped), an AudioVideo whose
218+
own soundtrack is replaced by this one, or the path or URL of a
219+
video file (or a {"location": ...} dict wrapping either - the
220+
same idiom `get_last_frame(video=...)` and a pipeline's `image`
221+
argument accept, #510) - loaded the same way `concat_videos`
222+
loads one of its inputs, confined the same way `audio`'s own
223+
path is (#553); the
219224
frames' own rate is carried through to the output, so set
220225
`result.fps` only to override it (a loaded file brings its rate
221226
along; frames that carry none are written at 8 fps)
@@ -268,6 +273,14 @@ def pair_audio(video, audio, sample_rate=None, fps=None, fit=None):
268273

269274
audio = _Loaded(*load_audio(audio))
270275
waveform = getattr(audio, "audio", audio)
276+
277+
from .video_utils import is_video_location, load_audio_video
278+
279+
if is_video_location(video):
280+
# A path/URL naming a video an earlier run wrote - confined the same
281+
# way concat_videos loads one of its own inputs (#553)
282+
video = load_audio_video(video)
283+
271284
if waveform is None:
272285
raise ValueError(
273286
"pair_audio needs an audio track - the video it was given carries none"

‎tests/test_concat_videos.py‎

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,15 @@ def test_untrimmed_audio_concatenates_whole(self):
6666
assert result.audio.shape == (2, 400)
6767

6868
def test_mixed_inputs_keep_the_audio_that_exists(self):
69+
# #553: the silent first video now fills its own length with
70+
# silence rather than being skipped, which is why it needs fps -
71+
# the same requirement trimming or crossfading audio already has
6972
videos = [frames(4), audio_video(8, 1)]
7073

71-
result = concat_videos(videos)
74+
result = concat_videos(videos, fps=4)
7275

7376
assert len(result.frames) == 12
74-
assert result.audio.shape == (2, 200)
77+
assert result.audio.shape == (2, 100 + 200)
7578

7679
def test_mismatched_sample_rates_are_resampled_to_the_highest(self, caplog):
7780
"""#108: a 24 kHz voice clip paired onto a 32 kHz generation used to
@@ -417,6 +420,56 @@ def test_a_location_dict_is_loaded_with_its_audio(self, tmp_path):
417420
assert result.audio.shape == (2, 8 / 4 * 8000)
418421

419422

423+
class TestSilentInputAmongSoundedOnes:
424+
"""#553: a silent input contributed zero duration to the joined track,
425+
shifting every later shot's audio early instead of the gap it actually
426+
left in the picture."""
427+
428+
def test_a_silent_middle_shot_does_not_shift_the_third_shots_audio(self):
429+
sound, silent, more_sound = (
430+
audio_video(8, 0.5),
431+
frames(8),
432+
audio_video(8, 0.25),
433+
)
434+
435+
result = concat_videos([sound, silent, more_sound], fps=4)
436+
437+
# Each shot is 8 frames @ 4fps @ 100Hz = 200 samples; the silent shot
438+
# must still occupy its own 200 samples rather than contributing none
439+
assert result.audio.shape == (2, 600)
440+
third = result.shots[-1]
441+
assert third["start_sample"] == 400
442+
assert third["start_frame"] == 16
443+
444+
def test_shots_stay_contiguous_across_the_silent_gap(self):
445+
sound, silent, more_sound = (
446+
audio_video(8, 0.5),
447+
frames(8),
448+
audio_video(8, 0.25),
449+
)
450+
451+
result = concat_videos([sound, silent, more_sound], fps=4)
452+
453+
for earlier, later in zip(result.shots, result.shots[1:]):
454+
assert (
455+
earlier["start_sample"] + earlier["num_samples"]
456+
== later["start_sample"]
457+
)
458+
459+
def test_a_silent_first_shot_is_also_filled(self):
460+
silent, sound = frames(8), audio_video(8, 0.5)
461+
462+
result = concat_videos([silent, sound], fps=4)
463+
464+
assert result.audio.shape == (2, 400)
465+
assert result.shots[-1]["start_sample"] == 200
466+
467+
def test_no_input_carrying_audio_still_leaves_audio_none(self):
468+
result = concat_videos([frames(4), frames(4)], fps=4)
469+
470+
assert result.audio is None
471+
472+
420473
class TestLevelMatching:
421474
"""Independently generated shots land at whatever level the model chose,
422475
and cutting a -2.6 dBFS shot against a -12.6 dBFS one is audible as a
@@ -470,7 +523,10 @@ def test_a_silent_shot_is_left_alone(self):
470523
assert float(numpy.abs(result.audio[:, :100]).max()) == 0.0
471524

472525
def test_a_shot_with_no_soundtrack_joins_as_before(self):
473-
result = concat_videos([frames(4), audio_video(4, 0.5)], match_levels="rms")
526+
# #553: fps is now required to fill the silent shot's own length
527+
result = concat_videos(
528+
[frames(4), audio_video(4, 0.5)], match_levels="rms", fps=4
529+
)
474530

475531
assert result.audio is not None
476532
assert len(result.frames) == 8

‎tests/test_pair_audio.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import numpy
1212
import pytest
1313
import torch
14+
from PIL import Image
1415

1516
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
1617

@@ -99,6 +100,44 @@ def test_raises_when_no_sample_rate_can_be_established():
99100
pair_audio(["frame"], _waveform())
100101

101102

103+
def test_loads_a_string_video_path_the_same_way_concat_videos_does(tmp_path):
104+
"""#553: 'audio' already loaded a string path via load_audio; 'video' did
105+
not, so a workflow naming its video by an asset:/output: path (resolved
106+
to a plain string by the time the task runs) failed rather than loading
107+
it the way concat_videos loads one of its own inputs."""
108+
from diffusers.utils.export_utils import encode_video
109+
110+
path = tmp_path / "shot.mp4"
111+
encode_video(
112+
[Image.new("RGB", (16, 16)) for _ in range(4)],
113+
fps=4,
114+
output_path=str(path),
115+
)
116+
117+
paired = pair_audio(str(path), _waveform(samples=40), sample_rate=24000)
118+
119+
assert isinstance(paired, AudioVideo)
120+
assert len(paired.frames) == 4
121+
assert paired.audio.shape == (2, 40)
122+
123+
124+
def test_loads_a_location_dict_video(tmp_path):
125+
"""The same {"location": ...} idiom concat_videos and get_last_frame accept."""
126+
from diffusers.utils.export_utils import encode_video
127+
128+
path = tmp_path / "shot.mp4"
129+
encode_video(
130+
[Image.new("RGB", (16, 16)) for _ in range(4)],
131+
fps=4,
132+
output_path=str(path),
133+
)
134+
135+
paired = pair_audio({"location": str(path)}, _waveform(samples=40), sample_rate=24000)
136+
137+
assert isinstance(paired, AudioVideo)
138+
assert len(paired.frames) == 4
139+
140+
102141
def test_registered_as_a_task_command():
103142
from dw.tasks.task import _COMMAND_REGISTRY
104143

0 commit comments

Comments
 (0)