feat: add scene-based video generation with per-scene material binding - #1275
feat: add scene-based video generation with per-scene material binding#1275BroBFG wants to merge 1 commit into
Conversation
|
Thank you for working on scene-based video generation. This is a useful direction. I tested the PR locally, including the full test suite and a real FFmpeg composition with two scene materials. There are a few issues that need to be addressed before we can merge it:
The existing test suite passes, but the new tests currently cover schema and CLI parsing rather than the end-to-end scene pipeline. Please keep the scene structure through audio, subtitle, material, and composition stages, preserve scene order, and reuse the existing local-material security checks. Thanks again, and I will be happy to review an update. |
|
Thank you for the detailed review. I've addressed all issues: 1. Narration and subtitles now use scene scriptsAfter processing scenes, scripts are concatenated with scene_scripts = [s.script for s in processed_scenes if s.script]
if scene_scripts:
video_script = "\n\n".join(scene_scripts)TTS and subtitles now use the concatenated scene scripts, not the top-level 2. Scene boundaries, durations, and transitions preserved
3. Local materials use managed-storage path validationScene local materials now use 4. Scene duration auto-calculated when not specified
5.
|
|
Thank you for the update and for adding the additional tests. I reviewed the latest changes and also ran a real FFmpeg composition test. There are still a few issues to address before this can be merged:
Please make each scene duration control its actual output segment, preserve transitions for each scene boundary, add a real timeline-level test, and rebase onto the latest |
f312a35 to
33e4ad5
Compare
Rewrite scene handling so that each scene is a self-contained video with its own TTS audio, subtitles, materials, and transitions. This resolves the reviewer's feedback that scene durations, transitions, and boundaries were not properly preserved. Architecture (variant A): Scene 1: script → audio + subtitles → materials → scene video Scene 2: script → audio + subtitles → materials → scene video ... Final: concat(scene videos) + background music → final.mp4 Key changes: - schema.py: Add SceneConfig with clip_transition field, add scene_transition and clip_transition defaults to VideoParams - video.py: Add concat_scene_videos_with_transitions() for final scene concatenation with per-boundary transitions and BGM overlay, _apply_scene_transition() for transition effects, _overlay_bgm_on_video() for background music mixing - task.py: Add _generate_single_scene() for per-scene video assembly, _resolve_bgm_for_final() for BGM resolution. Rewrite _run_pipeline() to process scenes independently. Delete old _combine_scene_videos() and scene mode from generate_final_videos(). - material.py: Remove dead download_scene_videos() function - cli.py: Add --scene-transition and --clip-transition options Each scene's duration is determined by its TTS audio length (or silent audio estimation for no-voice mode). Per-scene SceneConfig.transition takes precedence over global scene_transition default. Tests: 42 scene pipeline tests covering schema validation, scene assembly, transitions, CLI parsing, and timeline verification. Parallelism: TODO stubs added for future ThreadPoolExecutor-based scene processing. Base: rebased onto latest upstream/main, resolving material.py conflict.
|
Hi again, thank you for the thorough review. I apologize for the issues in my previous attempts — I've learned from those mistakes and took a more careful approach this time. Architectural RethinkThe previous approach tried to split a single combined video into scene segments after the fact, which was fundamentally flawed. I've now completely rearchitected the scene pipeline to build each scene as a fully self-contained video first, then concatenate at the end. New pipeline (variant A): Each scene runs through the same How each feedback item was addressed:1. Scene duration now controls its actual output segmentBefore: After: # task.py — _generate_single_scene()
audio_file = os.path.join(scene_dir, "audio.mp3")
sub_maker = voice.tts(text=script, voice_name=params.voice_name, ...)
audio_clip = AudioFileClip(audio_file)
audio_duration = audio_clip.duration # This IS the scene duration
...
video.combine_videos(
audio_file=audio_file, # Scene's own audio
...
)2. Per-scene transitions preserved at each boundaryBefore: First scene's transition was applied globally to all clips. After: Two-level transition system:
# task.py — _generate_single_scene()
video.combine_videos(
video_transition_mode=scene.clip_transition or params.clip_transition,
# Within-scene clips use clip_transition
)
# task.py — _run_pipeline() scene mode
for i, scene in enumerate(processed_scenes):
scene_transitions.append(scene.transition or params.scene_transition)
# Between-scene uses scene.transition
video.concat_scene_videos_with_transitions(
scene_transitions=scene_transitions,
# Applied per-boundary
)3. Scene duration from audio, not equal splitBefore: Remaining time was split equally among scenes without explicit duration. After: Each scene's duration is its TTS audio length. No equal-split heuristic. The approach is deterministic: whoever speaks more words gets more screen time. For 4. Rebased onto latest main, no conflictsResolved the 5. Real timeline-level tests added42 tests across 8 test classes:
Parallelism stubs The current architectural approach opens up the possibility of parallelizing the preparation of individual scenes, since each scene's pipeline (download materials → combine video) is fully independent. Before deciding whether to implement this, I'd like to hear your thoughts on the matter. This feature could be implemented either as part of this PR or separately as a follow-up task. Added TODO comments in # TODO(future): Parallelize scene processing
# Each scene (download materials + combine video) is independent.
# Current code processes scenes sequentially for simplicity.
# Future: use ThreadPoolExecutor to process multiple scenes concurrently. |
Summary
Add the ability to define individual scenes within a single video, where each scene has its own script text, search keywords, materials, duration, and transition effect.
Changes
New Model:
SceneConfigscene_id: int— scene number (1-based, auto-assigned)script: str— scene narration textsearch_terms: Optional[List[str]]— keywords for material searchmaterials: Optional[List[MaterialInfo]]— local materials for this sceneduration: Optional[float]— target scene duration in secondstransition: Optional[VideoTransitionMode]— transition to next sceneModified Files
app/models/schema.py— AddedSceneConfigmodel andscenesfield toVideoParamsapp/services/task.py— Addedgenerate_scenes()function and scene support in pipelineapp/services/material.py— Addeddownload_scene_videos()for per-scene material downloadcli.py— Added--scenesand--scenes-fileCLI parameterstest/services/test_schema.py— Added 15 tests forSceneConfigandVideoParamstest/services/test_cli.py— Added 10 tests for CLI scene featuresUsage Examples
CLI with inline JSON:
CLI with JSON file:
uv run python cli.py \ --video-subject "The World of Japanese Drift" \ --scenes-file scenes.jsonAPI request:
{ "video_subject": "The World of Japanese Drift", "scenes": [ {"scene_id": 1, "script": "Scene 1", "search_terms": ["term1"], "duration": 5}, {"scene_id": 2, "script": "Scene 2", "search_terms": ["term2"], "duration": 10} ] }Backward Compatibility
scenesis not set (None), the existing single-script pipeline is used unchangedTesting