mirror of
https://github.com/calesthio/OpenMontage.git
synced 2026-08-16 05:26:13 +08:00
fix(video_compose): honor target resolution in FFmpeg compose (vertical/9:16)
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) <noreply@anthropic.com>
This commit is contained in:
91
tests/tools/test_video_compose_vertical.py
Normal file
91
tests/tools/test_video_compose_vertical.py
Normal file
@@ -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)
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user