From d8a4950b9919c27a2556f7cec22154ed69a23f8e Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Mon, 2 Nov 2026 20:58:49 -0700 Subject: [PATCH] fix(import): keep only exported dash content periods on a re-fetch The export writes mediaexport content_periods per manifest; an import that re-fetches the MPD (no saved snapshot) drops the periods a service filtered. --- docs/guide/downloading.md | 8 +++ tests/core/test_export.py | 104 ++++++++++++++++++++++++++++++- unshackle/commands/dl.py | 43 ++++++++++++- unshackle/core/import_service.py | 31 ++++++++- uv.lock | 2 +- 5 files changed, 181 insertions(+), 7 deletions(-) diff --git a/docs/guide/downloading.md b/docs/guide/downloading.md index 2974aa9d..f54340bd 100644 --- a/docs/guide/downloading.md +++ b/docs/guide/downloading.md @@ -990,6 +990,14 @@ The [mediaexport](https://github.com/unshackle-dl/mediaexport) package specifies title language, the video track takes the language of the default audio track. When the master playlist gives no audio language either, the import stops with a message. +!!! note "Ad periods a service skipped stay skipped" + Some services skip DASH periods, such as ads, by a rule of their own. The export also + records the periods the service kept as `content_periods` on each manifest. An import that + must fetch the manifest again, because the file has no saved manifest or another tool wrote + it, downloads only those periods, also when the new manifest gives an ad period a new ID. + When the new manifest has none of the recorded periods, the import logs a warning and keeps + every period. + ## Metadata and tagging unshackle looks up metadata automatically, but you can override the identifiers used for diff --git a/tests/core/test_export.py b/tests/core/test_export.py index 89f0d350..e20918d0 100644 --- a/tests/core/test_export.py +++ b/tests/core/test_export.py @@ -12,7 +12,7 @@ import json import logging from pathlib import Path from types import SimpleNamespace -from typing import Any, cast +from typing import Any, Optional, cast from uuid import UUID import click @@ -374,3 +374,105 @@ def test_malformed_content_key_skips_the_key_not_the_download( assert "keys" not in entry(export) assert f"Not exporting the key for KID {KID.hex}" in caplog.text + + +AD_MPD_URL = "https://example.test/ads/m.mpd" + + +def ad_mpd(ad_id: str) -> str: + """An MPD whose pre-roll carries only a marker that a service filter, not core, knows.""" + + def period(pid: str, marker: str) -> str: + prop = f'' if marker else "" + return f"""{prop} + + + + + """ + + return f""" + +{period(ad_id, "PRE_ROLL")}{period("main", "")}""" + + +def export_service_filtered_title(tmp_path: Path, manifest_url: Optional[str]) -> Path: + """Export a DASH title as a service would, its own period_filter dropping the pre-roll, with no manifest snapshot. + + Without a snapshot the import re-fetches the manifest, the path ``content_periods`` serves. + """ + from unshackle.core.manifests import DASH + + title = Movie(id_="movie-1", service=StubService, name="Example Movie", year=2024, language="en") + parsed = DASH.from_text(ad_mpd("ad-1"), AD_MPD_URL).to_tracks( + language="en", + period_filter=lambda p: any(x.get("value") == "PRE_ROLL" for x in p.findall("SupplementalProperty")), + ) + video = parsed.videos[0] + assert video.data["dash"]["filtered_period_ids"] == ["ad-1"] + title.tracks.add(video) + title.tracks.manifest_url = manifest_url + export = tmp_path / "export.json" + make_dl().write_export(export, title, video) + doc = read_export(export) + doc["titles"][0]["x-unshackle"].pop("snapshot", None) + export.write_text(json.dumps(doc), encoding="utf8") + return export + + +@pytest.mark.parametrize("manifest_url", [AD_MPD_URL, None]) +@pytest.mark.parametrize("ad_id_on_refetch", ["ad-1", "ad-9"]) +def test_import_keeps_out_the_periods_the_service_filtered( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, manifest_url: Optional[str], ad_id_on_refetch: str +) -> None: + """The import re-fetches the MPD without the service, so its filter must travel in the export. + + A re-fetch can name the ad period anew, so only the exported content periods are kept. + """ + from unshackle.core.manifests import DASH + + export = export_service_filtered_title(tmp_path, manifest_url) + assert [m.get("content_periods") for m in entry(export)["manifests"]] == [["main"]] + monkeypatch.setattr(DASH, "from_url", lambda url, session=None, **kw: DASH.from_text(ad_mpd(ad_id_on_refetch), url)) + + svc = ImportService(import_ctx(), "EXAMPLE", "movie-1", str(export)) + video = svc.get_tracks(next(iter(svc.get_titles()))).videos[0] + + assert video.data["dash"]["filtered_period_ids"] == [ad_id_on_refetch] + + +def test_an_export_without_content_periods_imports_every_period( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An export written before content_period_ids, or from a service with no filter, keeps every period.""" + from unshackle.core.manifests import DASH + + export = export_service_filtered_title(tmp_path, AD_MPD_URL) + doc = read_export(export) + for manifest in doc["titles"][0]["manifests"]: + del manifest["content_periods"] + export.write_text(json.dumps(doc), encoding="utf8") + monkeypatch.setattr(DASH, "from_url", lambda url, session=None, **kw: DASH.from_text(ad_mpd("ad-1"), url)) + + svc = ImportService(import_ctx(), "EXAMPLE", "movie-1", str(export)) + video = svc.get_tracks(next(iter(svc.get_titles()))).videos[0] + + assert video.data["dash"]["filtered_period_ids"] == [] + + +def test_import_keeps_every_period_when_no_exported_period_is_left( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A re-fetch that names every period anew has nothing to match, so the import warns and filters nothing.""" + from unshackle.core.manifests import DASH + + export = export_service_filtered_title(tmp_path, AD_MPD_URL) + renamed = ad_mpd("ad-1").replace('id="main"', 'id="main-2"') + monkeypatch.setattr(DASH, "from_url", lambda url, session=None, **kw: DASH.from_text(renamed, url)) + + svc = ImportService(import_ctx(), "EXAMPLE", "movie-1", str(export)) + with caplog.at_level(logging.WARNING): + video = svc.get_tracks(next(iter(svc.get_titles()))).videos[0] + + assert video.data["dash"]["filtered_period_ids"] == [] + assert "keeping every period" in caplog.text diff --git a/unshackle/commands/dl.py b/unshackle/commands/dl.py index 457fe0a4..5df0b800 100644 --- a/unshackle/commands/dl.py +++ b/unshackle/commands/dl.py @@ -100,7 +100,7 @@ from unshackle.core.tracks import Audio, Subtitle, Tracks, Video from unshackle.core.tracks.attachment import Attachment from unshackle.core.tracks.dv_fixup import apply_dv_fixup from unshackle.core.tracks.hybrid import Hybrid -from unshackle.core.tracks.track import assert_fragments_decrypted, has_encrypted_sample_entry +from unshackle.core.tracks.track import Track, assert_fragments_decrypted, has_encrypted_sample_entry from unshackle.core.tracks.tracks import at_resolution from unshackle.core.utilities import ( apply_original_language, @@ -4371,14 +4371,33 @@ class dl: session = getattr(getattr(self, "export_service", None), "session", None) # mediaexport drops Cookie and Authorization itself, on write and on read headers = dict(getattr(session, "headers", None) or {}) - manifests = [mediaexport.Manifest(primary_url, manifest_type, headers, role="primary")] if primary_url else [] + manifests = ( + [ + mediaexport.Manifest( + primary_url, + manifest_type, + headers, + role="primary", + content_periods=self.content_periods(all_tracks, primary_url), + ) + ] + if primary_url + else [] + ) # the whole URL identifies a manifest: two profiles on one endpoint are two manifests seen = {primary_url} for t in all_tracks: url = str(t.url) if t.descriptor in (Video.Descriptor.DASH, Video.Descriptor.ISM) and url not in seen: seen.add(url) - manifests.append(mediaexport.Manifest(url, t.descriptor.name.lower(), role="extra")) + manifests.append( + mediaexport.Manifest( + url, + t.descriptor.name.lower(), + role="extra", + content_periods=self.content_periods(all_tracks, url), + ) + ) rows = [] for t in all_tracks: @@ -4428,6 +4447,24 @@ class dl: }, ) + @staticmethod + def content_periods(tracks: Sequence[Track], url: str) -> list[str]: + """The ids of the DASH periods a service kept in the manifest at ``url``, empty when it filtered none. + + An import re-fetches the manifest without the service, so its period filter must travel in the export. + Empty also when a kept period has no id, since an id allow-list cannot name it. + """ + dash = next( + (d for t in tracks if (d := t.data.get("dash") or {}).get("filtered_period_ids") and str(t.url) == url), + None, + ) + if dash is None: + return [] + kept = [ + p.get("id") for p in dash["manifest"].findall("Period") if p.get("id") not in dash["filtered_period_ids"] + ] + return kept if all(kept) else [] + @staticmethod def failure_lines(e: BaseException) -> list[str]: """Return the header and the one line that report a failed download. diff --git a/unshackle/core/import_service.py b/unshackle/core/import_service.py index aa391ace..d3312969 100644 --- a/unshackle/core/import_service.py +++ b/unshackle/core/import_service.py @@ -88,6 +88,20 @@ def hls_playlist_kids(track: Any, session: Optional[requests.Session] = None) -> return None +def content_period_filter(manifest: Any, kept: Any, title: Any) -> Optional[Callable[[Any], bool]]: + """A DASH period_filter that drops every period not in ``kept``, the export's ``content_periods`` for this manifest. + + None when there is nothing to filter, or when no kept id is in the re-fetched manifest: the server can name its + periods again on each fetch, so the import then warns and keeps every period. + """ + if not kept or not isinstance(manifest, DASH): + return None + if not set(kept) & {p.get("id") for p in manifest.manifest.findall("Period")}: + log.warning(f"No exported content period of '{title}' is in its re-fetched DASH manifest, keeping every period") + return None + return lambda period: period.get("id") not in kept + + def resolve_import_manifest_data( tracks: Tracks, manifest_type: Optional[str], @@ -96,6 +110,7 @@ def resolve_import_manifest_data( language: Any, title: Title_T, saved: Optional[dict[str, Any]] = None, + content_periods: Optional[dict[str, list[str]]] = None, ) -> None: """Fill ``track.data`` for DASH/ISM tracks rebuilt from export dicts. @@ -124,7 +139,12 @@ def resolve_import_manifest_data( for url in {str(track.url) for track in pending if track.url}: try: manifest = parser.from_url(url=url, session=session) - parsed = manifest.to_tracks(language=fallback_lang) + kept = content_period_filter(manifest, (content_periods or {}).get(url), title) + parsed = ( + manifest.to_tracks(language=fallback_lang, period_filter=kept) + if kept and isinstance(manifest, DASH) + else manifest.to_tracks(language=fallback_lang) + ) except MissingLanguageError: raise click.ClickException( f"No language for '{title}': the {manifest_type} manifest has none and Title.language is unset" @@ -273,6 +293,7 @@ class ImportService: "snapshot": x.get("snapshot"), "chapters": entry.chapters, "attachments": x.get("attachments") or [], + "content_periods": {m.url: m.content_periods for m in entry.manifests if m.content_periods}, } @staticmethod @@ -404,8 +425,13 @@ class ImportService: raise click.ClickException( f"No language for '{title}': the export and the HLS manifest have none" ) + kept = content_period_filter(manifest, entry.get("content_periods", {}).get(manifest_url), title) try: - parsed = manifest.to_tracks(language=language) + parsed = ( + manifest.to_tracks(language=language, period_filter=kept) + if kept and isinstance(manifest, DASH) + else manifest.to_tracks(language=language) + ) except MissingLanguageError: raise click.ClickException(no_language) except Exception as e: @@ -466,6 +492,7 @@ class ImportService: language=title.language, title=title, saved=entry.get("snapshot"), + content_periods=entry.get("content_periods"), ) for attachment in entry.get("attachments") or []: diff --git a/uv.lock b/uv.lock index 8c64635b..ade1c5a0 100644 --- a/uv.lock +++ b/uv.lock @@ -1086,7 +1086,7 @@ wheels = [ [[package]] name = "mediaexport" version = "0.1.0" -source = { git = "https://github.com/unshackle-dl/mediaexport?rev=main#fa9357e961d050450b59b1e4a839761121b2bcbd" } +source = { git = "https://github.com/unshackle-dl/mediaexport?rev=main#0ed846cb4d40d514c3b9f55e1814ea2f3090b884" } [[package]] name = "more-itertools"