diff --git a/docs/dev/creating-a-service.md b/docs/dev/creating-a-service.md index f1e9196f..caa3921a 100644 --- a/docs/dev/creating-a-service.md +++ b/docs/dev/creating-a-service.md @@ -491,6 +491,25 @@ def get_tracks(self, title: Title_T) -> Tracks: return self.get_tracks_for_variants(title, fetch_variant) ``` +!!! note "What the helper merges" + The helper adds the video, audio and subtitle tracks of every manifest. + From a later manifest, it skips a track that has the ID of a held track. It + also skips a track whose `Tracks.signature` matches a held track, because + the same track can have a different ID in each manifest. Chapters and + attachments come from the first manifest only. + + - `signature` holds the attributes that the track list and selection + read: codec, range, resolution, frame rate, bitrate and name for video; + language, codec, channels, JOC, descriptive, bitrate and name for audio; + language, codec, flags and name for subtitles. + - The helper does not remove copies inside one manifest. To remove them, + call `tracks.dedupe()`. Two tracks that differ only in a value outside + the signature, such as a dialogue-boost mix, become one track, so remove the + track you do not want before you call it. + - The audio that `dl` selects can come from a different manifest than the + video. Keep licence data on each track (`track.data`), not in one value + per title that the next fetch overwrites. + After parsing, you may **correct** track metadata the manifest gets wrong: stamp the real `video.range`, fix odd audio channel counts, mark descriptive audio, add hand-built subtitles or an `Attachment` (e.g. cover art). Do *not* @@ -595,7 +614,7 @@ def get_widevine_license(self, *, challenge: bytes, title, track): `get_clearkey_license` to do the POST yourself. 3. **Keys arrive obfuscated or through a bespoke channel**: fetch and unwrap the content key in `get_tracks` and pre-populate - `drm.content_keys[kid] = key_hex` on the track's `ClearKeyCENC`. The + `drm.signatures[kid] = key_hex` on the track's `ClearKeyCENC`. The non-obvious payoff: when every KID is already keyed, the framework skips the license round-trip entirely. diff --git a/tests/core/test_hybrid_hdr10plus.py b/tests/core/test_hybrid_hdr10plus.py index 6f74266e..e6587bcc 100644 --- a/tests/core/test_hybrid_hdr10plus.py +++ b/tests/core/test_hybrid_hdr10plus.py @@ -31,7 +31,7 @@ from unshackle.core.api.handlers import validate_download_parameters from unshackle.core.config import config from unshackle.core.service import Service, TrackRequest from unshackle.core.services import Services -from unshackle.core.tracks import Tracks, Video +from unshackle.core.tracks import Audio, Chapter, Subtitle, Tracks, Video from unshackle.core.tracks import hybrid as hybrid_mod from unshackle.core.tracks.hybrid import Hybrid, hdr10plus_mean_pq, hdr10plus_shots, nits_to_pq @@ -731,3 +731,50 @@ def test_a_native_hybrid_passes_the_gate_and_takes_no_base_at_its_resolution() - assert [v.id for v in bases] == ["h1080"] assert dv is not None and dv.id == "dvp2160" + + +@pytest.mark.parametrize( + "ranges, codecs, surround_on", + [ + ([Video.Range.SDR, Video.Range.HDR10], [], Video.Range.HDR10), + ([Video.Range.HYBRID], [Video.Codec.AVC, Video.Codec.HEVC], Video.Range.DV), + ], +) +def test_variants_merge_every_manifest_but_skip_a_subtitle_copy_and_later_chapters( + ranges: list[Video.Range], codecs: list[Video.Codec], surround_on: Video.Range +) -> None: + """Audio and a subtitle language that only a later manifest offers are kept. A track whose ID + differs per manifest (a signed URL) is not duplicated, and later chapters do not clash.""" + fetches = 0 + + def fetch(title: Any, codec: Any, range_: Video.Range) -> Tracks: + nonlocal fetches + fetches += 1 + tracks = Tracks() + tracks.add( + Video( + url="https://example.invalid/v", language="en", id_=f"{range_.name}{codec}", codec=codec, range_=range_ + ) + ) + tracks.add(Audio(url="https://example.invalid/a2", language="en", id_=f"stereo{fetches}", channels=2)) + if range_ == surround_on: + tracks.add(Audio(url="https://example.invalid/a6", language="en", id_="surround", channels=6)) + tracks.add(Subtitle(url=f"https://example.invalid/s?tok={fetches}", language="en", id_=f"sub{fetches}")) + if range_ == surround_on: + tracks.add(Subtitle(url=f"https://example.invalid/j?tok={fetches}", language="ja", id_=f"ja{fetches}")) + tracks.add(Chapter(0)) + return tracks + + service = SimpleNamespace( + track_request=TrackRequest(codecs=codecs, ranges=ranges), + log=logging.getLogger("variants-test"), + ) + + tracks = Service.get_tracks_for_variants(service, None, fetch) # type: ignore[arg-type] + + assert fetches > 1 + assert sorted(a.id for a in tracks.audio) == ["stereo1", "surround"] + assert [str(s.language) for s in tracks.subtitles] == ["en", "ja"] + assert tracks.subtitles[0].id == "sub1" + assert len(tracks.chapters) == 1 + assert len(tracks.videos) == fetches diff --git a/tests/tracks/test_track_selection_fixes.py b/tests/tracks/test_track_selection_fixes.py index c3d8023a..8020fce7 100644 --- a/tests/tracks/test_track_selection_fixes.py +++ b/tests/tracks/test_track_selection_fixes.py @@ -147,3 +147,25 @@ def test_sort_audio_codec_priority_beats_bitrate() -> None: best = Tracks.by_language(tracks.audio, ["en"])[0] # EC3 is prioritized even though the AC3 track has the higher bitrate assert best.id == "ec3-320" + + +def test_dedupe_keeps_the_first_copy_and_every_track_that_differs() -> None: + tracks = Tracks() + tracks.audio = [ + make_audio("ec3-a", codec=Audio.Codec.EC3, bitrate=128_000), + make_audio("ec3-b", codec=Audio.Codec.EC3, bitrate=128_000), + make_audio("aac", codec=Audio.Codec.AAC, bitrate=128_000), + make_audio("ec3-desc", codec=Audio.Codec.EC3, bitrate=128_000, descriptive=True), + make_audio("ec3-atmos", codec=Audio.Codec.EC3, bitrate=128_000, joc=16), + make_audio("ec3-de", codec=Audio.Codec.EC3, bitrate=128_000, language="de"), + ] + tracks.videos = [ + make_video("hevc-a", height=1080, bitrate=5_000_000, width=1920), + make_video("hevc-b", height=1080, bitrate=5_000_000, width=1920), + make_video("hevc-hi", height=1080, bitrate=8_000_000, width=1920), + ] + + tracks.dedupe() + + assert [a.id for a in tracks.audio] == ["ec3-a", "aac", "ec3-desc", "ec3-atmos", "ec3-de"] + assert [v.id for v in tracks.videos] == ["hevc-a", "hevc-hi"] diff --git a/unshackle/core/service.py b/unshackle/core/service.py index 082e2718..506dc56f 100644 --- a/unshackle/core/service.py +++ b/unshackle/core/service.py @@ -319,14 +319,19 @@ class Service(metaclass=ABCMeta): ) -> Tracks: """Call fetch_fn for each codec/range combo in track_request, merge results. + Video, audio and subtitles merge from every combo, so a track that only one manifest + offers is kept. The helper skips a later track whose ``Tracks.signature`` matches a held + one, because its ID can differ between manifests. Chapters and attachments come from the + first combo only. + Services that need separate API calls per codec/range combo can use this helper from their get_tracks() implementation. The fetch_fn signature should be: (title, codec, range_) -> Tracks For HYBRID range, this helper calls fetch_fn with HDR10 and DV separately, - then merges the DV video tracks into the HDR10 result. With no DV and - ``generate_dv`` requested, it merges the HDR10P video tracks instead. + then merges the DV tracks into the HDR10 result. With no DV video and + ``generate_dv`` requested, it merges the HDR10P tracks instead. Args: title: The title to process. @@ -335,12 +340,21 @@ class Service(metaclass=ABCMeta): all_tracks = Tracks() first = True + def merge(tracks: Tracks) -> None: + nonlocal first + if first: + all_tracks.add(tracks, warn_only=True) + first = False + return + held = {Tracks.signature(track) for track in all_tracks} + all_tracks.add([track for track in tracks if Tracks.signature(track) not in held], warn_only=True) + codecs = self.track_request.codecs or [None] ranges = self.track_request.ranges or [Video.Range.SDR] for range_val in ranges: if range_val == Video.Range.HYBRID: - # HYBRID: fetch HDR10 first (full tracks), then DV (video only) + # HYBRID: fetch HDR10 first, then DV for codec_val in codecs: try: hdr_tracks = fetch_fn(title, codec=codec_val, range_=Video.Range.HDR10) @@ -349,29 +363,23 @@ class Service(metaclass=ABCMeta): self.log.warning(f" - HDR10 not available for HYBRID, skipping ({e})") continue raise - if first: - all_tracks.add(hdr_tracks, warn_only=True) - if hdr_tracks.manifest_url and not all_tracks.manifest_url: - all_tracks.manifest_url = hdr_tracks.manifest_url - first = False - else: - for video in hdr_tracks.videos: - all_tracks.add(video, warn_only=True) + merge(hdr_tracks) try: - dv_videos = fetch_fn(title, codec=codec_val, range_=Video.Range.DV).videos + dv_tracks = fetch_fn(title, codec=codec_val, range_=Video.Range.DV) except (ValueError, SystemExit): self.log.info(" - No DolbyVision manifest available for HYBRID") - dv_videos = [] + dv_tracks = Tracks() + merge(dv_tracks) has_hdr10p = any(v.range == Video.Range.HDR10P for v in all_tracks.videos) - if not dv_videos and self.track_request.generate_dv and not has_hdr10p: + if not dv_tracks.videos and self.track_request.generate_dv and not has_hdr10p: # the HDR10 request may not return HDR10+, which the conversion needs as its base try: - dv_videos = fetch_fn(title, codec=codec_val, range_=Video.Range.HDR10P).videos + hdr10p_tracks = fetch_fn(title, codec=codec_val, range_=Video.Range.HDR10P) except (ValueError, SystemExit): self.log.info(" - No HDR10+ manifest available for HYBRID") - for video in dv_videos: - all_tracks.add(video, warn_only=True) + else: + merge(hdr10p_tracks) else: for codec_val in codecs: try: @@ -382,14 +390,7 @@ class Service(metaclass=ABCMeta): self.log.warning(f" - {range_val.name}/{codec_name} not available, skipping ({e})") continue raise - if first: - all_tracks.add(tracks, warn_only=True) - if tracks.manifest_url and not all_tracks.manifest_url: - all_tracks.manifest_url = tracks.manifest_url - first = False - else: - for video in tracks.videos: - all_tracks.add(video, warn_only=True) + merge(tracks) return all_tracks @@ -584,7 +585,7 @@ class Service(metaclass=ABCMeta): :return: The JWK Set license as a dict, JSON str, or raw bytes. Return None (the default) to let the framework POST the challenge to the manifest-provided Laurl, if any. Services with no license server can instead pre-populate the DRM object's - `content_keys` in get_tracks. + `signatures` in get_tracks. """ return None diff --git a/unshackle/core/tracks/tracks.py b/unshackle/core/tracks/tracks.py index 0f75362e..dd2d9cf1 100644 --- a/unshackle/core/tracks/tracks.py +++ b/unshackle/core/tracks/tracks.py @@ -276,6 +276,36 @@ class Tracks: if duplicates: log.debug(f" - Found and skipped {duplicates} duplicate tracks...") + @staticmethod + def signature(track: AnyTrack) -> tuple: + """The attributes that listing and selection read. Two tracks with one signature look and select the same.""" + if isinstance(track, Video): + return Video, track.codec, track.range, track.width, track.height, track.fps, track.bitrate, track.name + if isinstance(track, Audio): + return ( + Audio, + str(track.language), + track.codec, + track.channels, + track.joc, + track.descriptive, + track.bitrate, + track.name, + ) + return type(track), str(track.language), track.codec, track.forced, track.sdh, track.cc, track.name + + def dedupe(self) -> None: + """Drop each video, audio or subtitle track whose signature matches an earlier track.""" + seen: set[tuple] = set() + for kind in ("videos", "audio", "subtitles"): + kept = [] + for track in getattr(self, kind): + key = self.signature(track) + if key not in seen: + seen.add(key) + kept.append(track) + setattr(self, kind, kept) + def sort_videos( self, by_language: Optional[Sequence[Union[str, Language]]] = None, exact_match: bool = False ) -> None: