fix(mux): repoint sample entries with a missing data reference for MP4Box

MP4Box refused 3D titles at the credits with "Bad Parameter"; an exit with no output now fails the mux.

tests/core/test_mp4_container_routing.py

@@ -19,6 +19,7 @@ import pytest
 from unshackle.core import binaries
 from unshackle.core.tracks import Audio, Subtitle, Tracks, Video
 from unshackle.core.tracks.track import (
+    fix_data_reference_indexes,
     has_mp4_only_audio_entry,
     iter_top_level_boxes,
     strip_duplicate_init_boxes,
@@ -190,10 +191,27 @@ def test_mp4box_errors_come_back_to_the_caller(tmp_path: Path, monkeypatch: pyte
 
     tracks = build_tracks(tmp_path, b"dtsx")
     _, returncode, errors = tracks.mux("Title", delete=False, output_path=tmp_path / "out.muxed.mkv")
-    assert returncode == 1
+    assert returncode == 2, "MP4Box exits 1 on a failed import, and an exit with no output is no warning"
     assert errors == ["Error importing track: not supported"]
 
 
+def test_an_mp4box_warning_with_output_stays_a_warning(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    class WarningPopen(FakePopen):
+        def __init__(self, args, **kwargs):
+            super().__init__(args, **kwargs)
+            Path(args[-1]).write_bytes(b"muxed")
+
+        def wait(self) -> int:
+            return 1
+
+    monkeypatch.setattr(binaries, "MP4Box", Path("MP4Box"), raising=False)
+    monkeypatch.setattr(subprocess, "Popen", WarningPopen)
+
+    tracks = build_tracks(tmp_path, b"dtsx")
+    _, returncode, _ = tracks.mux("Title", delete=False, output_path=tmp_path / "out.muxed.mkv")
+    assert returncode == 1
+
+
 def init_pair(sample_entry: bytes, stamp: bytes = b"\x00\x00\x00\x01") -> bytes:
     """An ftyp+moov pair; the stamp stands in for the fetch time a packager writes."""
     stsd = box(b"stsd", box(sample_entry, b"\x00" * 20))
@@ -268,3 +286,77 @@ def test_an_unreadable_input_aborts_instead_of_shipping_a_placeholder(
 
     assert returncode >= 2, "a refused input must abort the run, not pass as a warning"
     assert errors
+
+
+def two_entry_video(path: Path, dref_count: int = 1, entry_body: int = 78, decoy: bytes = b"") -> Path:
+    """A video MP4 whose second sample entry names data reference 2; ``decoy`` goes in the trak before mdia."""
+
+    def entry(index: int) -> bytes:
+        return box(b"dvh1", (b"\x00" * 6 + struct.pack(">H", index) + b"\x00" * 70)[:entry_body])
+
+    dref = box(b"dref", struct.pack(">II", 0, dref_count) + box(b"url ", b"\x00\x00\x00\x01"))
+    stsd = box(b"stsd", struct.pack(">II", 0, 2) + entry(1) + entry(2))
+    minf = box(b"minf", box(b"dinf", dref) + box(b"stbl", stsd))
+    path.write_bytes(box(b"ftyp", b"isom") + box(b"moov", box(b"trak", decoy + box(b"mdia", minf))) + fragment(b"x"))
+    return path
+
+
+def repointed(data: bytes) -> bytes:
+    """``data`` with its second dvh1 entry pointed at data reference 1."""
+    expected = bytearray(data)
+    index = expected.rindex(b"dvh1") + 4 + 6
+    expected[index : index + 2] = b"\x00\x01"
+    return bytes(expected)
+
+
+@pytest.mark.parametrize(
+    "decoy",
+    [
+        pytest.param(b"", id="plain"),
+        pytest.param(box(b"meta", box(b"dinf", box(b"dref", struct.pack(">II", 0, 0)))), id="meta-dref-before-mdia"),
+    ],
+)
+def test_a_sample_entry_with_a_missing_data_reference_is_pointed_at_the_first(tmp_path: Path, decoy: bytes) -> None:
+    """A packager can give a second sample entry data_reference_index 2 with one dref entry; MP4Box refuses it.
+
+    Only the media's own dref counts, not one under a meta box before mdia.
+    """
+    path = two_entry_video(tmp_path / "v.mp4", decoy=decoy)
+    expected = repointed(path.read_bytes())
+
+    assert fix_data_reference_indexes(path) == 1
+    assert path.read_bytes() == expected, "only the second entry's index may change"
+    assert fix_data_reference_indexes(path) == 0
+
+
+@pytest.mark.parametrize(
+    ("dref_count", "entry_body"),
+    [pytest.param(0, 78, id="empty-dref"), pytest.param(1, 4, id="short-entry"), pytest.param(1, 0, id="empty-entry")],
+)
+def test_malformed_entries_are_left_alone(tmp_path: Path, dref_count: int, entry_body: int) -> None:
+    """With no dref entry to point at, or no room for the index, the file must not change."""
+    path = two_entry_video(tmp_path / "v.mp4", dref_count=dref_count, entry_body=entry_body)
+    before = path.read_bytes()
+
+    assert fix_data_reference_indexes(path) == 0
+    assert path.read_bytes() == before
+
+
+def test_the_mp4_mux_hands_mp4box_the_repointed_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    seen: dict[str, bytes] = {}
+
+    def fake_popen(args: list[str], **kwargs: object) -> FakePopen:
+        source = next(arg for arg in args if arg.startswith(str(tmp_path / "v.mp4")))
+        seen["video"] = Path(source.split(":lang=")[0]).read_bytes()
+        Path(args[-1]).write_bytes(b"muxed")
+        return FakePopen(args, **kwargs)
+
+    monkeypatch.setattr(binaries, "MP4Box", Path("MP4Box"), raising=False)
+    monkeypatch.setattr(subprocess, "Popen", fake_popen)
+
+    tracks = build_tracks(tmp_path, b"dtsx")
+    original = two_entry_video(tracks.videos[0].path).read_bytes()
+    _, returncode, _ = tracks.mux("Title", delete=False, output_path=tmp_path / "out.muxed.mkv")
+
+    assert returncode == 0
+    assert seen["video"] == repointed(original)

unshackle/commands/dl.py

@@ -104,7 +104,12 @@ 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.subtitle_convert import PYCAPTION_WRITERS, SUBSTATION, build_ass_style
-from unshackle.core.tracks.track import Track, assert_fragments_decrypted, has_encrypted_sample_entry
+from unshackle.core.tracks.track import (
+    Track,
+    assert_fragments_decrypted,
+    fix_data_reference_indexes,
+    has_encrypted_sample_entry,
+)
 from unshackle.core.tracks.tracks import at_resolution
 from unshackle.core.utilities import (
     apply_original_language,
@@ -3959,6 +3964,7 @@ class dl:
                     final_dir.mkdir(parents=True, exist_ok=True)
 
                     for track_path in muxed_paths:
+                        fix_data_reference_indexes(track_path)
                         media_info = MediaInfo.parse(track_path)
                         base_filename = title.get_filename(media_info, show_service=not no_source)
                         if sample:

unshackle/core/tracks/track.py

@@ -29,11 +29,13 @@ from unshackle.core.constants import DOWNLOAD_ALL_DRM, DOWNLOAD_CANCELLED, DOWNL
 from unshackle.core.downloaders.requests import requests
 from unshackle.core.drm import DRM_T, BlowfishStripe, ClearKeyCENC, PlayReady, Widevine
 from unshackle.core.drm.verify import (
+    children,
     decrypt_track,
     find_child_box,
     has_encrypted_sample_entry,
     read_top_level_box,
     senc_protects_samples,
+    walk,
 )
 from unshackle.core.events import events
 from unshackle.core.session import MAX_BACKOFF, AnySession, RnetSession, utf8_text
@@ -166,6 +168,38 @@ def strip_duplicate_init_boxes(src: Path, dst: Path) -> int:
     return dropped
 
 
+def fix_data_reference_indexes(path: Path) -> int:
+    """Point each sample entry whose data_reference_index has no dref entry at entry 1, in place.
+
+    MP4Box refuses a sample of such an entry with "Bad Parameter", while mkvmerge ignores the index.
+    Returns how many entries it changed.
+    """
+    moov_box = next(((offset, size) for kind, offset, size in iter_top_level_boxes(path) if kind == b"moov"), None)
+    if moov_box is None:
+        return 0
+    offset, size = moov_box
+    fixed = 0
+    with path.open("r+b") as f:
+        f.seek(offset)
+        moov = f.read(size)
+        for kind, body, end in walk(moov):
+            if kind != b"minf":
+                continue
+            dref = find_child_box(moov[body:end], b"dref")
+            refs = int.from_bytes(dref[12:16], "big") if dref and len(dref) >= 16 else 1
+            if not refs:
+                continue
+            for box, stsd, stsd_end in walk(moov, body, end):
+                if box != b"stsd":
+                    continue
+                for _, entry, entry_end in children(moov, stsd + 8, stsd_end):
+                    if entry_end - entry >= 8 and not 1 <= int.from_bytes(moov[entry + 6 : entry + 8], "big") <= refs:
+                        f.seek(offset + entry + 6)
+                        f.write(b"\x00\x01")
+                        fixed += 1
+    return fixed
+
+
 MPEGH_SAMPLE_ENTRIES = ("mha1", "mha2", "mhm1", "mhm2")
 
 

unshackle/core/tracks/tracks.py

@@ -27,7 +27,12 @@ from unshackle.core.tracks.attachment import Attachment
 from unshackle.core.tracks.audio import Audio
 from unshackle.core.tracks.chapters import Chapter, Chapters
 from unshackle.core.tracks.subtitle import Subtitle
-from unshackle.core.tracks.track import Track, has_mp4_only_audio_entry, strip_duplicate_init_boxes
+from unshackle.core.tracks.track import (
+    Track,
+    fix_data_reference_indexes,
+    has_mp4_only_audio_entry,
+    strip_duplicate_init_boxes,
+)
 from unshackle.core.tracks.video import Video
 from unshackle.core.utilities import (
     FPS,
@@ -722,6 +727,14 @@ class Tracks:
                 )
             elif candidate.exists():
                 candidate.unlink()
+            repointed = fix_data_reference_indexes(source)
+            if repointed:
+                log_event(
+                    "mux_mp4_dref_fixed",
+                    level="DEBUG",
+                    message=f"Pointed {repointed} sample entry(s) of {track.path.name} at data reference 1 for MP4Box",
+                    context={"track_id": track.id, "entries": repointed},
+                )
 
             delay = round((track.sample_start - sample_base) * 1000) if config.sample else 0
             moov = read_top_level_box(source, b"moov")
@@ -804,6 +817,8 @@ class Tracks:
             returncode = p.wait()
             if returncode < 2 and any(MP4BOX_OPEN_FAILED.search(line) for line in errors):
                 returncode = 2
+            if returncode and not output_path.exists():
+                returncode = 2
             if config.sample and returncode == 0 and not stereo:
                 start, end = (seconds - sample_base for seconds in config.sample)
                 cut_path = output_path.with_suffix(".cut.mp4")