From 118fc70e08bea620fb7102f6e6e05a02a12ad3e9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 23:12:16 +0200 Subject: [PATCH] fix(downloads): take the duration from the file when yt-dlp gives none A Facebook or Instagram reel extracts with no duration, so the asset kept a NULL duration_s although the file plays for minutes, and the Library row had no trim action. The finalize probe now returns the duration with the codecs (the file wins, info fills a gap), and runs before the sidecars so the .nfo runtime gets it too. --- backend/app/downloads/pipeline.py | 19 +++++++++------ backend/app/downloads/probe.py | 27 ++++++++++++++------- backend/tests/test_upload.py | 40 ++++++++++++++++++++++++++----- 3 files changed, 64 insertions(+), 22 deletions(-) diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 23251899..0a08fdb6 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -483,13 +483,19 @@ def _download( produced = _produced_file(info, staging) produced = _ensure_browser_playable(job_id, produced, spec, info) ext = produced.suffix.lstrip(".").lower() + # Codecs/dims/duration come from a PROBE of the produced file — the ground truth — NOT yt-dlp's + # `info` dict, whose fields are absent for some sources (Facebook among them): deriving + # media_kind from those nulls misclassified a real H.264 video as a download-only 'file', and a + # missing duration hid the trim action. Probed before the sidecars so the .nfo runtime has it + # too. Same probe the upload ingest uses. + vcodec, acodec, width, height, duration_s = probe.resolve_media_facts(produced, info) meta = storage.MediaMeta( video_id=info.get("id") or source_ref, title=normalize_title(info.get("title")) or source_ref, uploader=info.get("uploader") or info.get("channel") or "Unknown Channel", upload_date=_parse_upload_date(info.get("upload_date")), uniqueid_type=adapter.nfo_uniqueid_type(), - duration_s=int(info["duration"]) if info.get("duration") else None, + duration_s=duration_s, description=info.get("description"), thumbnail_url=info.get("thumbnail"), ) @@ -515,12 +521,11 @@ def _download( asset.rel_path = rel asset.size_bytes = size asset.duration_s = meta.duration_s - # Codecs/dims come from a PROBE of the finished file — the ground truth — NOT yt-dlp's - # `info` dict, whose codec fields are absent for some sources (Facebook among them): - # deriving media_kind from those nulls misclassified a real H.264 video as a - # download-only 'file' with no in-app play/edit. Same probe the upload ingest uses. - asset.vcodec, asset.acodec, asset.width, asset.height = probe.resolve_media_facts( - final, info + asset.vcodec, asset.acodec, asset.width, asset.height = ( + vcodec, + acodec, + width, + height, ) asset.container = ext asset.media_kind = mediakind.derive(asset.vcodec, asset.acodec) diff --git a/backend/app/downloads/probe.py b/backend/app/downloads/probe.py index 109339e6..4187d00c 100644 --- a/backend/app/downloads/probe.py +++ b/backend/app/downloads/probe.py @@ -81,10 +81,12 @@ def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | N def resolve_media_facts( path: Path, info: dict -) -> tuple[str | None, str | None, int | None, int | None]: - """(vcodec, acodec, width, height) for the stored download asset: PROBE the finished file on disk - (the ground truth). yt-dlp's `info` omits codec/dimension fields for some sources (Facebook among - them), so deriving media_kind from it there labelled a real H.264 video a download-only 'file'. +) -> tuple[str | None, str | None, int | None, int | None, int | None]: + """(vcodec, acodec, width, height, duration_s) for the stored download asset: PROBE the finished + file on disk (the ground truth). yt-dlp's `info` omits codec/dimension fields for some sources + (Facebook among them), so deriving media_kind from it there labelled a real H.264 video a + download-only 'file'. It omits the duration too (a Facebook reel, an Instagram reel), and a NULL + duration hides the Library row's trim action although the file plays for minutes. Precedence is by whether the probe RAN, not per-field: if ffprobe read the file, its codec verdict is authoritative — INCLUDING a deliberate None (cover-art-only audio, a still image). A per-field @@ -92,16 +94,23 @@ def resolve_media_facts( have (yt-dlp reports a progressive `vcodec` for an audio-mode download whose video it then strips), mislabelling an audio download as a broken in-app 'video'. Only when ffprobe could NOT read the file at all do we fall back to `info` (the pre-probe behaviour, a safety net); the cosmetic - dimensions still gap-fill from `info` in either case.""" + dimensions and the duration still gap-fill from `info` in either case.""" data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True) info_v = info.get("vcodec") if info.get("vcodec") not in (None, "none") else None info_a = info.get("acodec") if info.get("acodec") not in (None, "none") else None info_w = int(info["width"]) if info.get("width") else None info_h = int(info["height"]) if info.get("height") else None + info_d = int(info["duration"]) if info.get("duration") else None if data is None: # ffprobe could not read the file — fall back to the extractor's info entirely. - return info_v, info_a, info_w, info_h - pv, pa, pw, ph, _dur = _facts_from_probe(data) + return info_v, info_a, info_w, info_h, info_d + pv, pa, pw, ph, pd = _facts_from_probe(data) # Probe ran → its codecs win outright (a None means "no such track", not "unknown"); info only - # gap-fills the cosmetic width/height the probe couldn't read. - return pv, pa, pw if pw is not None else info_w, ph if ph is not None else info_h + # gap-fills the width/height/duration the probe couldn't read. + return ( + pv, + pa, + pw if pw is not None else info_w, + ph if ph is not None else info_h, + pd if pd is not None else info_d, + ) diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 6e7a9d01..d1e8c9c2 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -424,10 +424,36 @@ class TestResolveMediaFacts: ], }, ) - v, a, w, h = probemod.resolve_media_facts(Path("x"), {"width": None, "height": None}) + v, a, w, h, _d = probemod.resolve_media_facts(Path("x"), {"width": None, "height": None}) assert (v, a, w, h) == ("h264", "aac", 720, 1280) assert mediakind.derive(v, a) == mediakind.VIDEO # NOT 'file' + def test_the_duration_comes_from_the_file_when_info_has_none(self, monkeypatch): + # Facebook reel 257 (E11.5): yt-dlp gave no duration, the file is 128.4 s. A NULL duration hid + # the Library row's trim action. + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "mov,mp4,m4a", "duration": "128.400000"}, + "streams": [{"codec_type": "video", "codec_name": "h264"}], + }, + ) + assert probemod.resolve_media_facts(Path("x"), {"duration": None})[4] == 128 + # The file wins over a differing info duration, like its codecs do. + assert probemod.resolve_media_facts(Path("x"), {"duration": 300})[4] == 128 + + def test_info_fills_the_duration_the_probe_could_not_read(self, monkeypatch): + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "mov,mp4,m4a"}, + "streams": [{"codec_type": "video", "codec_name": "h264"}], + }, + ) + assert probemod.resolve_media_facts(Path("x"), {"duration": 61.7})[4] == 61 + self._ffprobe(monkeypatch, None) + assert probemod.resolve_media_facts(Path("x"), {"duration": 61.7})[4] == 61 + def test_probe_wins_over_a_DIFFERING_populated_info(self, monkeypatch): # Both the probe AND info are populated but DISAGREE (info carries a pre-remux/pre-merge codec). # The finished file on disk is ground truth, so the probe wins outright — pins the precedence a @@ -442,7 +468,7 @@ class TestResolveMediaFacts: ], }, ) - v, a, w, h = probemod.resolve_media_facts( + v, a, w, h, _d = probemod.resolve_media_facts( Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 640, "height": 480} ) assert (v, a, w, h) == ("h264", "aac", 1920, 1080) # probe, not info @@ -466,7 +492,7 @@ class TestResolveMediaFacts: ], }, ) - v, a, w, h = probemod.resolve_media_facts( + v, a, w, h, _d = probemod.resolve_media_facts( Path("x"), {"vcodec": "avc1", "acodec": "mp4a", "width": 1280, "height": 720} ) assert v is None and a == "mp3" # info's avc1 did NOT win @@ -482,7 +508,7 @@ class TestResolveMediaFacts: "streams": [{"codec_type": "video", "codec_name": "png", "width": 8, "height": 8}], }, ) - v, a, w, h = probemod.resolve_media_facts( + v, a, w, h, _d = probemod.resolve_media_facts( Path("x"), {"vcodec": "png", "width": 8, "height": 8} ) assert v is None and mediakind.derive(v, a) == mediakind.FILE @@ -490,7 +516,7 @@ class TestResolveMediaFacts: def test_info_fills_in_only_when_ffprobe_could_not_read_the_file(self, monkeypatch): # ffprobe failed entirely (None) → fall back to info so we never regress the working path. self._ffprobe(monkeypatch, None) - v, a, w, h = probemod.resolve_media_facts( + v, a, w, h, _d = probemod.resolve_media_facts( Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 1920, "height": 1080} ) assert (v, a, w, h) == ("vp9", "opus", 1920, 1080) @@ -498,6 +524,8 @@ class TestResolveMediaFacts: def test_info_none_codec_string_is_treated_as_absent(self, monkeypatch): # yt-dlp writes the literal "none" for a missing stream — it must not become a codec name. self._ffprobe(monkeypatch, None) # probe failed → info path, where the "none" scrub applies - v, a, w, h = probemod.resolve_media_facts(Path("x"), {"vcodec": "none", "acodec": "aac"}) + v, a, w, h, _d = probemod.resolve_media_facts( + Path("x"), {"vcodec": "none", "acodec": "aac"} + ) assert (v, a) == (None, "aac") assert mediakind.derive(v, a) == mediakind.AUDIO