diff --git a/backend/app/downloads/probe.py b/backend/app/downloads/probe.py index dfcb0ffa..109339e6 100644 --- a/backend/app/downloads/probe.py +++ b/backend/app/downloads/probe.py @@ -34,21 +34,16 @@ _IMAGE_FORMATS = { } -def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | None, int | None]: - """(vcodec, acodec, width, height, duration_s) from one ffprobe call; all None if ffprobe fails - or the file isn't playable media (an unprobeable file — or a still image — is treated as a generic - non-media file, so it becomes a download-only 'file' rather than a broken 'video'). - - A video stream flagged `attached_pic` (embedded cover art in an mp3/m4a) is skipped — it's a - thumbnail, not a real video track, so an audio file with album art still classifies as audio.""" - data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True) - if data is None: - return None, None, None, None, None - +def _facts_from_probe( + data: dict, +) -> tuple[str | None, str | None, int | None, int | None, int | None]: + """(vcodec, acodec, width, height, duration_s) parsed from a NON-None ffprobe JSON dict. A still + image / image-sequence container yields no vcodec (→ generic 'file'); a video stream flagged + `attached_pic` (embedded cover art in an mp3/m4a) is skipped — it's a thumbnail, not a real video + track, so an audio file with album art still classifies as audio.""" fmt = data.get("format") or {} fmt_names = {n.strip() for n in (fmt.get("format_name") or "").split(",")} if fmt_names & _IMAGE_FORMATS: - # A still image / image sequence is a generic file, not video — no vcodec means media_kind='file'. return None, None, None, None, None vcodec = acodec = None @@ -74,22 +69,39 @@ def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | N return vcodec, acodec, width, height, duration +def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | None, int | None]: + """(vcodec, acodec, width, height, duration_s) from one ffprobe call; all None if ffprobe fails + or the file isn't playable media (an unprobeable file — or a still image — is treated as a generic + non-media file, so it becomes a download-only 'file' rather than a broken 'video').""" + data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True) + if data is None: + return None, None, None, None, None + return _facts_from_probe(data) + + 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), and fall back to the extractor's `info` dict only for a field the probe could - not read. 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'. The file is - already on disk, so the probe is authoritative; `info` only fills a gap the probe left.""" - pv, pa, pw, ph, _dur = probe_media(path) + (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'. + + 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 + `probe or info` fallback would let `info`'s stale vcodec resurrect a video track the file doesn't + 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.""" + 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 - return ( - pv or info_v, - pa or info_a, - pw if pw is not None else info_w, - ph if ph is not None else info_h, - ) + 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) + # 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 diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index fc83527f..6e7a9d01 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -405,16 +405,91 @@ class TestProbeClassifier: class TestResolveMediaFacts: + # These stub run_ffprobe_json (one level below probe_media) so the REAL probe parsing + the + # probe-vs-info precedence are exercised together — the precedence keys off whether ffprobe RAN. + @staticmethod + def _ffprobe(monkeypatch, data): + monkeypatch.setattr(probemod.editmod, "run_ffprobe_json", lambda *a, **k: data) + def test_probe_wins_when_info_lacks_codecs(self, monkeypatch): - # THE regression: info has no vcodec/acodec/dims (Facebook), but the file probes as h264/aac. - monkeypatch.setattr(probemod, "probe_media", lambda _p: ("h264", "aac", 720, 1280, 24)) + # THE original regression: info has no vcodec/acodec/dims (Facebook), but the file probes as + # h264/aac. + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "mov,mp4,m4a"}, + "streams": [ + {"codec_type": "video", "codec_name": "h264", "width": 720, "height": 1280}, + {"codec_type": "audio", "codec_name": "aac"}, + ], + }, + ) v, a, w, h = 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_info_fills_only_fields_the_probe_could_not_read(self, monkeypatch): - # Probe failed / not media (all None) → fall back to info so we never regress the working path. - monkeypatch.setattr(probemod, "probe_media", lambda _p: (None, None, None, None, None)) + 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 + # `info or probe` inversion would silently break. + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "mov,mp4,m4a"}, + "streams": [ + {"codec_type": "video", "codec_name": "h264", "width": 1920, "height": 1080}, + {"codec_type": "audio", "codec_name": "aac"}, + ], + }, + ) + v, a, w, h = 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 + + def test_cover_art_audio_download_is_not_resurrected_to_video_by_info(self, monkeypatch): + # THE bug this fix targets: an audio-mode download from a progressive source embeds cover art, + # and yt-dlp's info still carries the source's video codec. The probe correctly finds only audio + # (the attached_pic is skipped); info's vcodec must NOT resurrect a video track → media_kind + # stays audio, not a broken in-app 'video'. + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "mp3", "duration": "180.0"}, + "streams": [ + { + "codec_type": "video", + "codec_name": "mjpeg", + "disposition": {"attached_pic": 1}, + }, + {"codec_type": "audio", "codec_name": "mp3"}, + ], + }, + ) + v, a, w, h = 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 + assert mediakind.derive(v, a) == mediakind.AUDIO # NOT VIDEO + + def test_still_image_download_is_not_resurrected_to_video_by_info(self, monkeypatch): + # A downloaded still image: the probe returns no vcodec (image-format guard). info's image + # "vcodec" must not turn it into a broken 'video' → stays a generic file. + self._ffprobe( + monkeypatch, + { + "format": {"format_name": "png_pipe"}, + "streams": [{"codec_type": "video", "codec_name": "png", "width": 8, "height": 8}], + }, + ) + v, a, w, h = probemod.resolve_media_facts( + Path("x"), {"vcodec": "png", "width": 8, "height": 8} + ) + assert v is None and mediakind.derive(v, a) == mediakind.FILE + + 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( Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 1920, "height": 1080} ) @@ -422,7 +497,7 @@ 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. - monkeypatch.setattr(probemod, "probe_media", lambda _p: (None, None, None, None, None)) + 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"}) assert (v, a) == (None, "aac") assert mediakind.derive(v, a) == mediakind.AUDIO