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

@@ -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

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'<SupplementalProperty schemeIdUri="urn:example:ad" value="{marker}"/>' if marker else ""
+        return f"""<Period id="{pid}" duration="PT30S">{prop}
+          <AdaptationSet contentType="video" mimeType="video/mp4" lang="en">
+            <Representation id="v1" bandwidth="5000000" width="1920" height="1080" codecs="avc1.640028">
+              <SegmentTemplate media="{pid}/$Number$.m4s" initialization="{pid}/i.mp4" duration="10" startNumber="1"/>
+            </Representation>
+          </AdaptationSet></Period>"""
+
+    return f"""<?xml version="1.0"?>
+<MPD xmlns="urn:mpeg:dash:schema:mpd:2011" type="static" mediaPresentationDuration="PT60S">
+{period(ad_id, "PRE_ROLL")}{period("main", "")}</MPD>"""
+
+
+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

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.

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 []:

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"