diff --git a/docs/guide/downloading.md b/docs/guide/downloading.md index c5b3aa20..07b84768 100644 --- a/docs/guide/downloading.md +++ b/docs/guide/downloading.md @@ -499,6 +499,13 @@ unshackle dl --skip-dl --export EXAMPLE 81234567 When `--proxy` is used, the export records the region so an import can reproduce the correct geofence. Without a proxy, no region is stored. +!!! warning "Some DASH and Smooth exports need a title language" + An import re-fetches the DASH or ISM manifest and parses it again. Most manifests label + their own streams, and those import fine. When a manifest labels nothing, the parse falls + back to the title's original language, which comes from `Title.language` on the exporting + service. If a service never sets it, importing that export fails with a message naming + the service. Neither end guesses a language for you, so the fix belongs in the service. + ## Metadata and tagging unshackle looks up metadata automatically, but you can override the identifiers used for diff --git a/tests/core/test_import_tracks.py b/tests/core/test_import_tracks.py new file mode 100644 index 00000000..f503302b --- /dev/null +++ b/tests/core/test_import_tracks.py @@ -0,0 +1,138 @@ +"""Tests for ``ImportService.get_tracks``, which rebuilds tracks from an export. + +Regression: an export whose manifest labels its own streams imports even when the service +never set ``Title.language``. When nothing supplies a language the import fails naming the +service rather than blaming the manifest URL. A direct-URL subtitle anywhere in the export +replaces the manifest's whole subtitle set, since the service side-loaded it precisely +because the manifest's copy was the worse one. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any, Optional +from unittest.mock import patch + +import click +import pytest +import requests + +from unshackle.core.import_service import ImportService +from unshackle.core.manifests import DASH + +MPD_TEMPLATE = """ + + + + + v.mp4 + + + + s.vtt + + + +""" + +# labelled by @lang; unlabelled but with Unified Streaming ids DASH derives a language from; +# and labelled by neither, which is the only shape that needs the title's language +MPD_WITH_LANG = MPD_TEMPLATE.format(lang=' lang="ja"', video_id="v1", sub_id="s1") +MPD_ID_LANG = MPD_TEMPLATE.format(lang="", video_id="video_eng=5000000", sub_id="text_eng=1000") +MPD_NO_LANG = MPD_TEMPLATE.format(lang="", video_id="v1", sub_id="s1") + +URL_SUBTITLE = { + "type": "Subtitle", + "id": "sideloaded-en", + "url": "https://example.invalid/en.srt", + "language": "en", + "descriptor": "URL", + "codec": "SubRip", +} + + +def make_service(tmp_path: Path, language: Optional[str], tracks: Optional[dict[str, Any]] = None) -> ImportService: + export = { + "version": 2, + "service": "EXAMPLE", + "titles": { + "movie-1": { + "meta": {"type": "movie", "id": "movie-1", "name": "Example Movie", "year": 2024, "language": language}, + "manifest_url": "https://example.invalid/m.mpd", + "manifest_type": "DASH", + "tracks": tracks or {}, + } + }, + } + path = tmp_path / "export.json" + path.write_text(json.dumps(export), encoding="utf8") + return ImportService(click.Context(click.Command("dl")), "EXAMPLE", "movie-1", str(path)) + + +def get_tracks(service: ImportService, mpd: str): + with patch.object(DASH, "from_url", side_effect=lambda url, session=None, **kw: DASH.from_text(mpd, url)): + return service.get_tracks(next(iter(service.get_titles()))) + + +@pytest.mark.parametrize("mpd", [MPD_WITH_LANG, MPD_ID_LANG], ids=["lang-attribute", "language-in-rep-id"]) +def test_manifest_supplies_language_without_title_language(tmp_path: Path, mpd: str) -> None: + """A manifest that labels its own streams imports fine with no language in the export.""" + tracks = get_tracks(make_service(tmp_path, language=None), mpd) + assert [str(t.language) for t in tracks.videos] == ["ja" if mpd is MPD_WITH_LANG else "en"] + + +@pytest.mark.parametrize("language", [None, "und"], ids=["missing", "und"]) +def test_unlabelled_manifest_without_usable_language_blames_the_service( + tmp_path: Path, language: Optional[str] +) -> None: + """`und` is truthy and a valid tag, so it must be rejected as firmly as no language at all.""" + with pytest.raises(click.ClickException) as error: + get_tracks(make_service(tmp_path, language=language), MPD_NO_LANG) + assert "Title.language" in error.value.message + assert "may have expired" not in error.value.message + + +def test_exported_url_subtitles_replace_the_manifests(tmp_path: Path) -> None: + """Services drop the manifest's subtitles and side-load their own; importing both duplicates them.""" + tracks = get_tracks(make_service(tmp_path, "ja", {"sideloaded-en": URL_SUBTITLE}), MPD_WITH_LANG) + assert [(t.id, t.descriptor.name) for t in tracks.subtitles] == [("sideloaded-en", "URL")] + assert len(tracks.videos) == 1 + + +def test_direct_url_subtitles_win_over_every_manifest_one(tmp_path: Path) -> None: + """One side-loaded subtitle condemns the whole re-parsed set, including other languages.""" + manifest_sub = { + "type": "Subtitle", + "id": "s1", + "url": "https://example.invalid/m.mpd", + "language": "ja", + "descriptor": "DASH", + "codec": "WebVTT", + } + tracks = get_tracks( + make_service(tmp_path, "ja", {"s1": manifest_sub, "sideloaded-en": URL_SUBTITLE}), MPD_WITH_LANG + ) + assert [(t.id, t.descriptor.name) for t in tracks.subtitles] == [("sideloaded-en", "URL")] + + +def test_unreadable_exported_track_is_skipped(tmp_path: Path) -> None: + """A track dict this build cannot rebuild must not take the whole import down with it.""" + unknown = {**URL_SUBTITLE, "id": "future", "codec": "CODEC_FROM_A_LATER_BUILD"} + tracks = get_tracks(make_service(tmp_path, "ja", {"sideloaded-en": URL_SUBTITLE, "future": unknown}), MPD_WITH_LANG) + assert [t.id for t in tracks.subtitles] == ["sideloaded-en"] + + +@pytest.mark.parametrize( + "failure", + [requests.ConnectionError("dns"), TypeError("Expected 'MPD' document, but received a 'html' document instead.")], + ids=["connection-error", "expired-token-html-page"], +) +def test_fetch_failure_still_reports_a_possible_expiry(tmp_path: Path, failure: Exception) -> None: + """An expired signed URL can 200 with an error page, so document failures are expiries too.""" + service = make_service(tmp_path, "ja") + title = next(iter(service.get_titles())) + with patch.object(DASH, "from_url", side_effect=failure): + with pytest.raises(click.ClickException) as error: + service.get_tracks(title) + assert "may have expired" in error.value.message diff --git a/unshackle/core/import_service.py b/unshackle/core/import_service.py index 3ecaf411..63490245 100644 --- a/unshackle/core/import_service.py +++ b/unshackle/core/import_service.py @@ -9,6 +9,7 @@ from uuid import UUID import click import requests +from langcodes import tag_is_valid from unshackle.core.config import config from unshackle.core.constants import AnyTrack @@ -26,6 +27,18 @@ log = logging.getLogger("import") PARSERS = {"DASH": DASH, "HLS": HLS, "ISM": ISM} +def _fallback_language(language: Any) -> Optional[Any]: + """The title language if the parsers would accept it, else None so they raise. + + DASH drops an und/invalid fallback but ISM does not, and a truthy 'und' there silently + labels every track und instead of failing. + """ + tag = str(language or "").strip() + if not tag or not tag_is_valid(tag) or tag.startswith("und"): + return None + return language + + class ImportService: """Reconstructs a download from an export JSON. @@ -132,9 +145,12 @@ class ImportService: """Reconstruct the title's tracks from the export. DASH/ISM: re-fetch and re-parse the manifest and return the full ladder (the importer - picks quality with normal dl flags; keys are injected by KID later). HLS/URL: rebuild - from the stored per-track dicts, since the variant is re-fetched from track.url at - download time and ATV-style master playlists carry unstable per-fetch tokens. + picks quality with normal dl flags; keys are injected by KID later), then merge in the + exported URL tracks. A service side-loads its own subtitles when the manifest's are the + worse copy, so any direct-URL subtitle in the export means the whole re-parsed subtitle + set is dropped in favour of the exported one. + HLS/URL: rebuild from the stored per-track dicts, since the variant is re-fetched from + track.url at download time and ATV-style master playlists carry unstable per-fetch tokens. """ title_id = str(title.id) if title_id in self.tracks_by_title: @@ -151,21 +167,41 @@ class ImportService: parser = PARSERS.get(manifest_type or "") if manifest_url and parser is not None and manifest_type in ("DASH", "ISM"): try: - parsed = parser.from_url(url=manifest_url, session=self.session).to_tracks(language=title.language) + manifest = parser.from_url(url=manifest_url, session=self.session) except Exception as e: raise click.ClickException( - f"Failed to re-fetch/parse the {manifest_type} manifest for '{title}'. " + f"Failed to fetch the {manifest_type} manifest for '{title}'. " f"The manifest URL may have expired since export. ({e})" ) + try: + parsed = manifest.to_tracks(language=_fallback_language(title.language)) + except ValueError as e: + if "Language information could not be derived" in str(e): + raise click.ClickException( + f"No language for '{title}': the {manifest_type} manifest has none and the " + f"service did not set Title.language." + ) + raise click.ClickException(f"Failed to parse the {manifest_type} manifest for '{title}'. ({e})") + except Exception as e: + raise click.ClickException(f"Failed to parse the {manifest_type} manifest for '{title}'. ({e})") for track in parsed: tracks.add(track) + track_dicts = [t for t in tracks_map.values() if t.get("descriptor", "URL") == "URL"] + if any(t.get("type") == "Subtitle" for t in track_dicts): + tracks.subtitles.clear() else: - for track_dict in tracks_map.values(): + track_dicts = list(tracks_map.values()) + + for track_dict in track_dicts: + try: track = Track.from_dict(track_dict) - drm = self.rebuild_drm(track_dict) - if drm: - track.drm = drm - tracks.add(track) + except Exception as e: + self.log.warning(f"Skipping exported track {track_dict.get('id')!r}: {e}") + continue + drm = self.rebuild_drm(track_dict) + if drm: + track.drm = drm + tracks.add(track, warn_only=True) for attachment in entry.get("attachments") or []: url = attachment.get("url")