Skip to content

Commit 771db0f

Browse files
fralapoclaude
andcommitted
feat(pipeline): A/V-sync hardening ported from Autocrop-vertical (study r6)
Sixth external reframe study (kamilstanuch/Autocrop-vertical). Its reframe algorithm is simpler than ClippyMe's, but its production-hardening layer closes real, previously-unhandled gaps in the clip/reframe render path — all confirmed absent by an audit (no VFR detection, no start_time compensation, no corrupt- frame guard, missing pix_fmt on two encoders). New cv2-free, host-tested module media_probe.py (pure helpers + never-raise ffprobe wrappers): - VFR detection (is_vfr / probe_is_variable_frame_rate) - stream start_time → audio seek args (parse_start_time / audio_sync_seek_args / probe_stream_start_time) - frame-rate parsing (parse_frame_rate) Wiring (gated so the proven path stays byte-identical on CFR, zero-start input): - reframe.py: VFR→CFR pre-normalize; -pix_fmt yuv420p + -vsync cfr on the raw encoder; try/except per-frame → duplicate last good frame; audio start_time compensation; -shortest mux; temp_cfr_input cleanup. - main.py: -pix_fmt yuv420p + -vsync cfr on the persisted source-slice cut. Fixes the classic yt-dlp A/V desync (non-zero video start_time) and VFR drift (phone uploads) — YouTube/phone are ClippyMe's primary inputs. Deferred (documented): HW encoder, quality/ratio CLI flags, fps cross-check. Verification: - host: pytest -m "not integration" → 279 passed, 2 skipped (248 baseline + 31 new) - Docker: pytest -m integration → 13 passed (reframe path unaffected) Deliverables (per /goal): docs/autocrop-vertical-analysis.md (report + prioritized list + learnings), media_probe.py + tests (implementation + coverage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d26bcd6 commit 771db0f

6 files changed

Lines changed: 529 additions & 51 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ ClippyMe is a self-hosted AI video platform that transforms long-form videos (Yo
1111
All Python backend code lives under `src/clippyme/` (src-layout, installed via `pip install -e .`):
1212

1313
- `src/clippyme/api/` — FastAPI app (`app.py`), `schemas.py`, `security.py`
14-
- `src/clippyme/pipeline/``main.py` orchestrator (~800 LOC, was 2216), `deepgram_transcribe.py`, `gemini_parser.py`, `gemini_service.py`, plus extracted stages: `transcribe_cache.py`, `download.py`, `scene_detection.py`, `postprocess.py`, `diarization.py`, `hardware.py` (DEVICE/Whisper-model selection), `reframe.py` (cv2/YOLO/MediaPipe core: `process_video_to_vertical` + `SmoothedCameraman`/`SpeakerTracker`/`DetectionSmoother` + face/person detection; `ASPECT_RATIO` is a module global `main` sets per-job via `reframe.ASPECT_RATIO = ...`), and pure-math `reframe_ops.py`. `main` re-exports the reframe classes for back-compat. Verify pipeline changes with `docker compose run --rm -u root backend sh -lc "pip install -q pytest && pytest -m integration"`.
14+
- `src/clippyme/pipeline/` — `main.py` orchestrator (~800 LOC, was 2216), `deepgram_transcribe.py`, `gemini_parser.py`, `gemini_service.py`, plus extracted stages: `transcribe_cache.py`, `download.py`, `scene_detection.py`, `postprocess.py`, `diarization.py`, `hardware.py` (DEVICE/Whisper-model selection), `reframe.py` (cv2/YOLO/MediaPipe core: `process_video_to_vertical` + `SmoothedCameraman`/`SpeakerTracker`/`DetectionSmoother` + face/person detection; `ASPECT_RATIO` is a module global `main` sets per-job via `reframe.ASPECT_RATIO = ...`), pure-math `reframe_ops.py`, and `media_probe.py` (cv2-free ffprobe wrappers + pure A/V-sync helpers: VFR detection, stream `start_time` compensation, frame-rate parsing — host-unit-tested; wired into `reframe.py`/`main.py` to fix VFR drift + YouTube audio desync. Ported from `kamilstanuch/Autocrop-vertical`, see `docs/autocrop-vertical-analysis.md`). `main` re-exports the reframe classes for back-compat. Verify pipeline changes with `docker compose run --rm -u root backend sh -lc "pip install -q pytest && pytest -m integration"`.
1515
- `src/clippyme/domain/``compose.py`, `clip_endpoints.py`, `job_results.py`, `job_artifacts.py`, `job_worker.py`, `history_service.py`, `subtitles.py`, `smartcut.py`, `hooks.py`
1616
- `src/clippyme/integrations/``social_publisher.py`, `auto_editor_updater.py`
1717
- `src/clippyme/storage/``config_store.py`

docs/autocrop-vertical-analysis.md

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# Comparative analysis: `kamilstanuch/Autocrop-vertical` → ClippyMe
2+
3+
Date: 2026-06-17
4+
External repo: <https://github.com/kamilstanuch/Autocrop-vertical> @ `main`
5+
Size: single 710-LOC `main.py` + `requirements.txt` + README (v1.4.1).
6+
Sixth external study, after `gauravzazz/smart-reframe`,
7+
`KazKozDev/auto-vertical-reframe`, `obi19999/smart-video-reframe`,
8+
`mfahsold/montage-ai`, and `aregrid/frame`.
9+
10+
---
11+
12+
## 0. Verdict up front
13+
14+
Unlike the fifth study (`aregrid/frame`, vaporware), this is **real, mature,
15+
single-file code** in the same domain: horizontal → vertical social-media
16+
cropping (PySceneDetect → YOLOv8 person detection → per-scene TRACK/LETTERBOX →
17+
OpenCV frame pipe → FFmpeg encode). Its **reframe algorithm is simpler than
18+
ClippyMe's** (static per-scene center-crop on a person box; no camera smoothing,
19+
no active-speaker selection, no continuous zoom, no global trajectory pass) — so
20+
there is nothing to port on the reframe-*math* axis where ClippyMe already runs
21+
ahead (`reframe_ops.py` + five prior studies).
22+
23+
Its real value is the **production-hardening layer** the changelog was written
24+
around: A/V-sync correctness on messy real-world inputs (YouTube downloads, phone
25+
VFR). That layer maps onto concrete, previously-unhandled gaps in ClippyMe's
26+
clip/reframe render path, and is what this study ports.
27+
28+
---
29+
30+
## 1. Architecture comparison
31+
32+
| Aspect | Autocrop-vertical | ClippyMe | Takeaway |
33+
|---|---|---|---|
34+
| Scene detection | PySceneDetect `ContentDetector`, `--frame-skip`/`--downscale` | PySceneDetect (`scene_detection.py`) | Parity. |
35+
| Subject detect | YOLOv8n person + Haar-cascade face (middle frame only) | YOLOv8n + MediaPipe FaceMesh + MAR active-speaker (per frame) | ClippyMe richer. |
36+
| Crop decision | `decide_cropping_strategy`: 0 people→LETTERBOX, 1→track box, group→TRACK if group-width < crop-width else LETTERBOX | `analyze_scenes_strategy` → TRACK/WIDE/GENERAL/DISABLED | Comparable intent; ClippyMe adds WIDE multi-speaker. |
37+
| Camera | **static** center-crop per scene (no motion) | `SmoothedCameraman`: EMA/1€/spring smoothing, continuous zoom, lost-subject drift | ClippyMe far ahead. |
38+
| **VFR handling** | `is_variable_frame_rate` (ffprobe r vs avg rate) → re-mux `-vsync cfr` | **none** | **Gap. Ported.** |
39+
| **Audio start_time** | `get_stream_start_time``-ss` trims audio lead-in | **none** | **Gap. Ported.** |
40+
| **Corrupt-frame** | try/except → duplicate last good frame | loop aborts on exception | **Gap. Ported.** |
41+
| **Output pix_fmt** | explicit `yuv420p` | only on *later* passes; reframe core + cut omit it | **Gap. Ported.** |
42+
| fps source | OpenCV (same backend that reads frames) + `-vsync cfr` | PySceneDetect for `-r`, OpenCV elsewhere (mismatch risk) | Mitigated by CFR + VFR ports. |
43+
| HW encoder | `--encoder hw` (VideoToolbox/NVENC) + fallback | libx264 only | Deferred (see §3). |
44+
| Quality presets | `--quality fast/balanced/high` + CRF/preset overrides | fixed per stage | Deferred (off product axis). |
45+
46+
---
47+
48+
## 2. Prioritised improvement list
49+
50+
| Pri | Improvement | Status | Notes |
51+
|---|---|---|---|
52+
| **High** | Audio `start_time` offset compensation | ✅ implemented | Classic yt-dlp A/V desync; YouTube is ClippyMe's primary input. `audio_sync_seek_args` + `probe_stream_start_time`, wired into reframe Step 5. |
53+
| **High** | VFR → CFR pre-normalization | ✅ implemented | Phone/odd-timebase sources. `probe_is_variable_frame_rate` gate + re-mux in reframe; CFR inputs (the normal clip slices) untouched. |
54+
| **Medium** | `-pix_fmt yuv420p` on the two bare libx264 encoders | ✅ implemented | reframe raw-frame encoder + main.py source-slice cut; guarantees universal decode even when later 420p passes are skipped. |
55+
| **Medium** | Corrupt/failed-frame resilience | ✅ implemented | try/except in the reframe render loop → duplicate last good frame; preserves frame count → no A/V drift. |
56+
| **Medium** | `-vsync cfr` on reframe encoder + cut | ✅ implemented | Locks constant output rate; pairs with the VFR fix. |
57+
| Low | fps cross-check (cv2 ↔ PySceneDetect) | ⏸ deferred | Subsumed: VFR normalize + CFR output remove the drift this guarded against. A hard assert could be added later. |
58+
| Low | Hardware encoder (NVENC/VideoToolbox) | ⏸ deferred | ClippyMe encodes in a Linux/Docker container where libx264 is the portable choice; HW detect adds surface for marginal CPU savings off the product axis. Documented, not wired. |
59+
| Low | `--quality`/`--ratio`/`--plan-only` CLI knobs | ⏸ deferred | ClippyMe's ratio/quality are driven by the dashboard + per-stage tuning, not CLI flags; not a fit. |
60+
61+
---
62+
63+
## 3. What was implemented
64+
65+
New module **`src/clippyme/pipeline/media_probe.py`** (pure helpers + thin
66+
ffprobe wrappers, **no cv2** → host-importable), following the established
67+
`reframe_ops.py` "pure logic in a testable module, thin glue in the cv2 file"
68+
pattern:
69+
70+
- `parse_frame_rate(str) -> float` — "30000/1001" → 29.97; malformed/zero → 0.0.
71+
- `is_vfr(r_rate, avg_rate, threshold=0.5) -> bool` — nominal vs average gap.
72+
- `parse_start_time(str) -> float` — ffprobe `start_time`; "N/A"/garbage → 0.0.
73+
- `audio_sync_seek_args(start, min_offset=0.05) -> list[str]``['-ss', …]` or `[]`.
74+
- `probe_stream_start_time` / `probe_is_variable_frame_rate` — ffprobe wrappers
75+
that **never raise** (missing ffprobe / odd file → safe default).
76+
77+
Wiring (glue, in the integration-tested cv2 files):
78+
79+
- **`reframe.py`** — (a) VFR detection + `-vsync cfr` re-mux pre-step (gated on
80+
`probe_is_variable_frame_rate`, so CFR inputs are byte-identical); (b)
81+
`-pix_fmt yuv420p` + `-vsync cfr` on the raw-frame encoder; (c) try/except
82+
around per-frame strategy → duplicate last good frame (`dropped_frames`
83+
reported); (d) audio extracted with `audio_sync_seek_args(start_time)`;
84+
(e) `-shortest` on the final mux; (f) `temp_cfr_input` cleanup.
85+
- **`main.py`**`-pix_fmt yuv420p` + `-vsync cfr` on the persisted source-slice
86+
cut, so the slice is universally decodable and CFR before reframe ever sees it.
87+
88+
Tests: **`tests/pipeline/test_media_probe.py`** — 31 host (non-integration)
89+
cases covering frame-rate parsing, VFR thresholds, start_time parsing, seek-arg
90+
no-op boundaries, and the never-raise contract.
91+
92+
---
93+
94+
## 4. Conflicts & resolutions
95+
96+
- **"Proven single-pass path must stay byte-identical" (CLAUDE.md).** Every port
97+
is a *no-op on the common case*: `audio_sync_seek_args` returns `[]` for a
98+
zero start_time, VFR normalization only fires when detection is confident, and
99+
the corrupt-frame guard only diverges on an exception that previously aborted
100+
the run. The normal pipeline feeds already-re-encoded CFR, zero-start clip
101+
slices, so its bytes are unchanged — confirmed by the integration suite still
102+
passing (13/13).
103+
- **"Don't rewrite; keep the style" (instruction).** No reframe logic was
104+
rewritten; the static-crop algorithm was *not* adopted (ClippyMe's is better).
105+
Only the orthogonal robustness layer was added, in the same inline-ffmpeg +
106+
pure-helper-module style already used across the pipeline.
107+
- **fps-source mismatch (concern #3).** Rather than re-plumb every fps read, the
108+
VFR normalize + CFR-locked output neutralize the drift the mismatch could
109+
cause. Noted as a deferred hardening (an explicit cv2-vs-PySceneDetect assert).
110+
- **`_render_global_smooth` opt-in path** shares the same single-write model, so
111+
the corrupt-frame guard is not applied there yet — that path is opt-in and
112+
off by default; noted for a follow-up if it graduates.
113+
- **Heavy ports skipped.** HW encoder, quality/ratio CLI flags are real but off
114+
ClippyMe's product axis (dashboard-driven, containerized libx264) — catalogued
115+
as deferred, honouring "Non riscrivere tutto."
116+
117+
---
118+
119+
## 5. Learnings & how they were applied
120+
121+
1. **A simpler upstream can still be a net contribution — via its hardening, not
122+
its core.** Autocrop's *algorithm* is behind ClippyMe's, but its changelog is
123+
a checklist of real-world A/V-sync bugs (VFR drift, start_time desync,
124+
dropped-frame drift) that a more sophisticated reframe core never bothered to
125+
handle. Value lived in the boring layer. *Applied:* ported the robustness,
126+
ignored the algorithm.
127+
2. **Pure-vs-glue split makes "untestable" ffmpeg logic testable.** The decision
128+
content (VFR threshold, seek-arg boundaries, rate parsing) was extracted into
129+
a cv2-free module and host-unit-tested (31 cases), while only the ffmpeg
130+
string-building stayed in the integration-only file — same discipline as
131+
`reframe_ops.py`. *Applied:* `media_probe.py`.
132+
3. **Port as no-ops on the happy path.** Each fix is gated so the proven path is
133+
byte-identical and only pathological inputs take the new branch. This is what
134+
let a change to the integration render loop ship without re-baselining the
135+
golden output. *Applied:* every wiring point above.
136+
137+
---
138+
139+
## 6. Verification
140+
141+
- Host pure-helper suite: `pytest tests/pipeline/test_media_probe.py`**31 passed**.
142+
- Full host (non-integration) suite: `pytest -m "not integration"`**279 passed,
143+
2 skipped** (248 prior baseline + 31 new) — no regression.
144+
- Integration suite in Docker:
145+
`docker compose run --rm -u root backend sh -lc "pip install -q pytest && pytest -m integration"`
146+
**13 passed, 290 deselected** — reframe.py imports (cv2/mediapipe) and the
147+
reframe render path are unaffected by the wiring.
148+
- `py_compile` on `reframe.py`, `main.py`, `media_probe.py` → clean.

src/clippyme/pipeline/main.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,13 @@ def _resolve_output_dir(out: str | None, default: str) -> str:
883883
'-ss', f'{float(start):.3f}',
884884
'-i', input_video,
885885
'-t', f'{clip_duration:.3f}',
886+
# -pix_fmt yuv420p + -vsync cfr guarantee the persisted source
887+
# slice is universally decodable and constant-frame-rate, so the
888+
# downstream reframe render (which writes raw frames at a fixed
889+
# -r) can't drift against audio even if the original download was
890+
# VFR (ported from kamilstanuch/Autocrop-vertical).
886891
'-c:v', 'libx264', '-crf', '18', '-preset', 'fast',
892+
'-pix_fmt', 'yuv420p', '-vsync', 'cfr',
887893
'-c:a', 'aac',
888894
clip_source_path,
889895
]
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
"""ffprobe-backed media inspection + pure A/V-sync helpers.
2+
3+
Robustness utilities ported from ``kamilstanuch/Autocrop-vertical`` (sixth
4+
external study — see ``docs/autocrop-vertical-analysis.md``). They close two
5+
real gaps in the clip / reframe render path:
6+
7+
* **Variable-frame-rate (VFR) detection** — phone uploads and some YouTube
8+
downloads carry a VFR video stream. The reframe loop decodes N frames with
9+
OpenCV and writes them back at a *fixed* ``-r fps``, so an undetected VFR
10+
source drifts against the stream-copied audio. ``is_vfr`` lets the caller
11+
re-mux to constant frame rate first.
12+
* **Stream ``start_time`` compensation** — YouTube downloads frequently have a
13+
non-zero video-stream ``start_time`` (audio at 0.0s, video at e.g. 1.8s).
14+
When the video is re-encoded from frame 0 but the audio is stream-copied
15+
verbatim, that container offset becomes an audible A/V desync.
16+
``audio_sync_seek_args`` trims the audio lead-in to match.
17+
18+
The pure parsing / decision helpers (``parse_frame_rate``, ``is_vfr``,
19+
``parse_start_time``, ``audio_sync_seek_args``) perform no I/O and are
20+
host-unit-tested in ``tests/pipeline/test_media_probe.py`` — no cv2 import, so
21+
they run in the fast (non-integration) suite. The ``probe_*`` wrappers shell out
22+
to ffprobe and **degrade gracefully (never raise)**: a missing ffprobe or an odd
23+
file yields the "assume CFR / zero offset" default, so the pipeline keeps its
24+
prior behaviour instead of breaking.
25+
"""
26+
from __future__ import annotations
27+
28+
import subprocess
29+
30+
31+
def parse_frame_rate(rate: str) -> float:
32+
"""Parse an ffprobe frame-rate string ("30000/1001", "25", "0/0") to fps.
33+
34+
Returns ``0.0`` for empty / malformed / zero-denominator input rather than
35+
raising, so callers can treat "unknown" as "assume CFR / skip".
36+
"""
37+
if not rate:
38+
return 0.0
39+
rate = rate.strip()
40+
try:
41+
if "/" in rate:
42+
num, den = rate.split("/", 1)
43+
den_f = float(den)
44+
if den_f == 0:
45+
return 0.0
46+
return float(num) / den_f
47+
return float(rate)
48+
except (ValueError, ZeroDivisionError):
49+
return 0.0
50+
51+
52+
def is_vfr(r_frame_rate: str, avg_frame_rate: str, threshold: float = 0.5) -> bool:
53+
"""True when nominal (``r_frame_rate``) and average (``avg_frame_rate``) frame
54+
rates differ enough to indicate a variable frame rate.
55+
56+
ffprobe reports both as "num/den". A gap above ``threshold`` fps means the
57+
container's real timing wanders from its nominal rate (classic for
58+
phone-recorded clips). Unknown / zero rates → ``False`` (assume CFR — the
59+
safer default, since a needless re-encode is worse than a no-op).
60+
"""
61+
r = parse_frame_rate(r_frame_rate)
62+
avg = parse_frame_rate(avg_frame_rate)
63+
if r <= 0 or avg <= 0:
64+
return False
65+
return abs(r - avg) > threshold
66+
67+
68+
def parse_start_time(value: str) -> float:
69+
"""Parse an ffprobe stream ``start_time`` to seconds; "N/A"/""/garbage → 0.0."""
70+
if not value:
71+
return 0.0
72+
try:
73+
return float(value.strip())
74+
except ValueError:
75+
return 0.0
76+
77+
78+
def audio_sync_seek_args(video_start_time: float, min_offset: float = 0.05) -> list[str]:
79+
"""Return ffmpeg input-seek args dropping the audio lead-in equal to the video
80+
stream's ``start_time`` — or ``[]`` when the offset is negligible.
81+
82+
Placed BEFORE ``-i`` (fast input seek) so the stream-copied audio is trimmed
83+
to begin where the (re-encoded-from-frame-0) video begins. Offsets under
84+
``min_offset`` (≈1.5 frames @30fps) are ignored — not worth a re-seek, and it
85+
keeps the common zero-start case byte-identical to the old behaviour.
86+
"""
87+
if video_start_time is None or video_start_time <= min_offset:
88+
return []
89+
return ["-ss", f"{video_start_time:.3f}"]
90+
91+
92+
def probe_stream_start_time(video_path: str, stream: str = "v:0") -> float:
93+
"""ffprobe a stream's ``start_time`` in seconds (0.0 if unavailable).
94+
95+
Never raises — a missing ffprobe or unreadable file returns 0.0 so the caller
96+
simply skips the sync adjustment.
97+
"""
98+
try:
99+
result = subprocess.run(
100+
["ffprobe", "-v", "error", "-select_streams", stream,
101+
"-show_entries", "stream=start_time", "-of", "csv=p=0", video_path],
102+
capture_output=True, text=True,
103+
)
104+
if result.returncode == 0:
105+
return parse_start_time(result.stdout)
106+
except (FileNotFoundError, OSError):
107+
pass
108+
return 0.0
109+
110+
111+
def probe_is_variable_frame_rate(video_path: str, threshold: float = 0.5) -> bool:
112+
"""ffprobe whether the first video stream is VFR.
113+
114+
Never raises — a missing ffprobe / unreadable file / odd output returns
115+
``False`` (assume CFR), so the pipeline only ever does *extra* work when it is
116+
confident the source is variable-rate.
117+
"""
118+
try:
119+
result = subprocess.run(
120+
["ffprobe", "-v", "error", "-select_streams", "v:0",
121+
"-show_entries", "stream=r_frame_rate,avg_frame_rate",
122+
"-of", "csv=p=0", video_path],
123+
capture_output=True, text=True,
124+
)
125+
if result.returncode != 0:
126+
return False
127+
parts = result.stdout.strip().split(",")
128+
if len(parts) < 2:
129+
return False
130+
return is_vfr(parts[0], parts[1], threshold=threshold)
131+
except (FileNotFoundError, OSError):
132+
return False

0 commit comments

Comments
 (0)