From 035df52b26ffd8fed67c6af8aaef534c2eb15003 Mon Sep 17 00:00:00 2001 From: An-idd Date: Sat, 27 Jun 2026 11:56:49 +0800 Subject: [PATCH] fix(video_compose): honor target resolution in FFmpeg compose (vertical/9:16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compose target resolution was resolved from `profile` (and documented as overridable via edit_decisions.metadata.compose_target) but the per-segment scale/pad filter hardcoded 1920x1080. As a result, vertical profiles such as `tiktok` / `youtube_shorts` / `instagram_reels` silently produced landscape 1920x1080 output instead of 1080x1920 — a silent dimension bug, no error raised. - Use the resolved target width/height in the per-segment scale/pad filter. - Implement the previously-stubbed `metadata.compose_target` extension point: {"width", "height", "fit"} where fit="pad" (letterbox, default, unchanged behavior) or fit="cover" (scale-to-fill + centre-crop, ideal for vertical). - Default with no profile/target stays 1920x1080 (backward compatible). Adds tests/tools/test_video_compose_vertical.py covering default landscape, profile=tiktok vertical, and compose_target cover override. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/tools/test_video_compose_vertical.py | 91 ++++++++++++++++++++++ tools/video/video_compose.py | 47 ++++++++--- 2 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 tests/tools/test_video_compose_vertical.py diff --git a/tests/tools/test_video_compose_vertical.py b/tests/tools/test_video_compose_vertical.py new file mode 100644 index 00000000..7eb36d5e --- /dev/null +++ b/tests/tools/test_video_compose_vertical.py @@ -0,0 +1,91 @@ +"""Vertical / arbitrary-resolution support for video_compose's FFmpeg compose. + +Regression test for a silent-dimension bug: the compose target resolution was +resolved from `profile` (and the documented `metadata.compose_target` hook) but +the per-segment scale/pad filter hardcoded 1920x1080, so vertical profiles like +`tiktok` silently produced landscape output. These tests run the real FFmpeg +path on a tiny lavfi fixture and assert the output dimensions. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +from tools.video.video_compose import VideoCompose + +pytestmark = pytest.mark.skipif( + shutil.which("ffmpeg") is None or shutil.which("ffprobe") is None, + reason="ffmpeg/ffprobe not available", +) + + +def _make_clip(path: Path, w: int = 1280, h: int = 720, d: int = 2) -> None: + subprocess.run( + ["ffmpeg", "-y", "-f", "lavfi", "-i", + f"color=c=teal:s={w}x{h}:d={d}:r=30", + "-c:v", "libx264", "-crf", "28", "-pix_fmt", "yuv420p", + "-g", "30", "-keyint_min", "30", str(path)], + capture_output=True, check=True, + ) + + +def _dims(path: Path) -> tuple[int, int]: + out = subprocess.check_output( + ["ffprobe", "-v", "error", "-select_streams", "v:0", + "-show_entries", "stream=width,height", "-of", "csv=p=0", str(path)] + ).decode().strip() + w, h = out.split(",") + return int(w), int(h) + + +def _edit_decisions(src: Path, metadata: dict | None = None) -> dict: + ed = { + "version": "1.0", + "render_runtime": "ffmpeg", + "cuts": [{"id": "c1", "source": str(src), "in_seconds": 0, "out_seconds": 2}], + } + if metadata: + ed["metadata"] = metadata + return ed + + +def test_compose_default_is_landscape_hd(tmp_path): + """No profile / no target → unchanged 1920x1080 default (backward compatible).""" + src = tmp_path / "in.mp4" + _make_clip(src) + out = tmp_path / "out.mp4" + r = VideoCompose().execute( + {"operation": "compose", "edit_decisions": _edit_decisions(src), "output_path": str(out)} + ) + assert r.success, r.error + assert _dims(out) == (1920, 1080) + + +def test_compose_vertical_profile(tmp_path): + """profile='tiktok' → 1080x1920 (the bug: previously stayed 1920x1080).""" + src = tmp_path / "in.mp4" + _make_clip(src) + out = tmp_path / "out.mp4" + r = VideoCompose().execute( + {"operation": "compose", "edit_decisions": _edit_decisions(src), + "profile": "tiktok", "output_path": str(out)} + ) + assert r.success, r.error + assert _dims(out) == (1080, 1920) + + +def test_compose_target_override_cover(tmp_path): + """metadata.compose_target with fit='cover' → exact requested dims, cropped to fill.""" + src = tmp_path / "in.mp4" + _make_clip(src) + out = tmp_path / "out.mp4" + ed = _edit_decisions(src, metadata={"compose_target": {"width": 720, "height": 1280, "fit": "cover"}}) + r = VideoCompose().execute( + {"operation": "compose", "edit_decisions": ed, "output_path": str(out)} + ) + assert r.success, r.error + assert _dims(out) == (720, 1280) diff --git a/tools/video/video_compose.py b/tools/video/video_compose.py index f450183e..ec14a1dc 100644 --- a/tools/video/video_compose.py +++ b/tools/video/video_compose.py @@ -383,8 +383,22 @@ class VideoCompose(BaseTool): preset = inputs.get("preset", "medium") profile_name = inputs.get("profile") - # Resolve target resolution from profile or default + # Resolve target resolution + fit mode. Priority: explicit `profile` + # arg > edit_decisions.metadata.compose_target > default (landscape HD). + # compose_target = {"width": W, "height": H, "fit": "pad"|"cover"} lets a + # caller request vertical (9:16) or any aspect without a named profile. + # fit="pad" letterboxes (no content loss, the historical default); + # fit="cover" scales-to-fill and centre-crops (better for vertical social). resolution = "1920x1080" + fit_mode = "pad" + compose_target = (edit_decisions.get("metadata") or {}).get("compose_target") + if isinstance(compose_target, dict): + try: + resolution = f"{int(compose_target['width'])}x{int(compose_target['height'])}" + except (KeyError, ValueError, TypeError): + pass + if compose_target.get("fit") in ("pad", "cover"): + fit_mode = compose_target["fit"] if profile_name: try: from lib.media_profiles import get_profile @@ -392,6 +406,10 @@ class VideoCompose(BaseTool): resolution = f"{p.width}x{p.height}" except (ImportError, ValueError): pass + try: + target_w, target_h = (int(v) for v in resolution.split("x")) + except ValueError: + target_w, target_h = 1920, 1080 cuts = edit_decisions.get("cuts", []) if not cuts: @@ -468,17 +486,22 @@ class VideoCompose(BaseTool): # pix_fmt / sar across ALL segments — otherwise it throws # "Non-monotonous DTS" or silently produces corrupt output. # - # Default target is 1920x1080 @ 30fps, yuv420p, sar=1. If the - # source is smaller it letterboxes; if larger it downscales. - # Callers can override via edit_decisions.metadata.compose_target - # (future extension) but the defaults match the most common - # delivery profile (YouTube landscape). - vf_parts: list[str] = [ - "scale=1920:1080:force_original_aspect_ratio=decrease", - "pad=1920:1080:(ow-iw)/2:(oh-ih)/2:color=black", - "setsar=1", - "fps=30", - ] + # Target is target_w x target_h @ 30fps, yuv420p, sar=1 + # (default 1920x1080; overridable via `profile` or + # edit_decisions.metadata.compose_target — see above). + # fit="pad" letterboxes to preserve all content; fit="cover" + # scales-to-fill then centre-crops (no bars, for vertical social). + if fit_mode == "cover": + geom = [ + f"scale={target_w}:{target_h}:force_original_aspect_ratio=increase", + f"crop={target_w}:{target_h}", + ] + else: + geom = [ + f"scale={target_w}:{target_h}:force_original_aspect_ratio=decrease", + f"pad={target_w}:{target_h}:(ow-iw)/2:(oh-ih)/2:color=black", + ] + vf_parts: list[str] = [*geom, "setsar=1", "fps=30"] af_parts: list[str] = [] if speed != 1.0: vf_parts.append(f"setpts={1.0/speed}*PTS")