fix(music): remux songs into the container of their codec

A song skips the muxer, so the decrypted fragmented MP4 was the delivered file: its header states a length of zero and FLAC in an MP4 is not a FLAC stream, so playback stopped early and tagging failed.

docs/guide/output-and-naming.md

@@ -15,7 +15,7 @@ When a download finishes, unshackle builds the final path from three pieces:
 - **Output directory**: `directories.downloads` from your config, or whatever you pass to `-o/--output` on the command line for a single run.
 - **Folder template**: an optional per-title subfolder. unshackle **always** puts TV episodes and songs in a folder. Movies only get one if you define a `movies` folder template, or a single-string `folder` template that applies to all title kinds (see [Folder templates](#folder-templates)).
 - **Filename template**: the `output_template` for the title kind (movie, series, or song).
-- **Extension**: chosen by the muxer for a movie or an episode: `.mkv` for video, `.mka` for audio-only, `.mks` for subtitle-only. unshackle does not mux a song. A song keeps the container the service delivered, such as `.flac`, `.m4a`, or `.mp3`. Read [Music output files](#music-output-files).
+- **Extension**: chosen by the muxer for a movie or an episode: `.mkv` for video, `.mka` for audio-only, `.mks` for subtitle-only. unshackle does not mux a song. It remuxes a song into the container of its codec, such as `.flac`, `.opus`, or `.m4a`. Read [Music output files](#music-output-files).
 
 !!! example "A typical episode path"
     ```text
@@ -225,10 +225,24 @@ The per-kind folder config keys are `movies`, `series`, `songs`, and `albums`. A
 
 ## Music output files
 
-A song does not go through the muxer. unshackle moves the downloaded audio file to
-its final path. It keeps the container that the service delivered. A lossless download
-arrives as a `.flac` file. An AAC download arrives as a `.m4a` file. unshackle writes no
-`.mkv` file for music, and the [muxing options](#muxing-options) below do not apply.
+A song does not go through the muxer. Music services deliver audio in a fragmented MP4, which is
+not a music file. Its header states a length of zero, so a player stops after the first fragment.
+FLAC inside an MP4 is also not a FLAC stream. A rename cannot correct either fault, so unshackle
+remuxes each downloaded audio track into the container of its codec, then moves the file to its
+final path.
+
+| Codec | Extension |
+|---|---|
+| FLAC | `.flac` |
+| Opus | `.opus` |
+| Vorbis | `.ogg` |
+| AAC, ALAC, and all other codecs | `.m4a` |
+
+unshackle re-encodes FLAC in this step. The audio does not change, but the new file states its
+true length. A plain copy cannot, because it keeps the source header and the zero in it.
+
+unshackle writes no `.mkv` file for music, and the [muxing options](#muxing-options) below do not
+apply.
 
 The `songs` output template names the file. The `albums` folder template names the folder
 around it.

tests/core/test_music_container.py

@@ -0,0 +1,144 @@
+"""``Audio.to_music_container`` must turn a fragmented MP4 into a playable, taggable file.
+
+Music titles never reach the muxer, so the downloaded fragmented MP4 is what the user gets.
+Its ``mvhd`` states a length of zero, so a player stops after the first fragment. FLAC
+inside an MP4 is also not a FLAC stream. Each test builds a real MP4 with FFmpeg and reads
+the result back with mutagen, which reads the header only, exactly as a player does.
+FFprobe cannot prove this, because it scans every fragment and reports the true length
+either way.
+"""
+
+from __future__ import annotations
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import mutagen
+import pytest
+
+from tests.core.test_music_tagger_roundtrip import make_song
+from unshackle.core.music.tagger import write_music_metadata
+from unshackle.core.tracks import Audio
+
+pytestmark = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="requires ffmpeg")
+
+
+def encode(path: Path, encoder: str, *extra_args: str) -> Path:
+    subprocess.run(
+        [
+            "ffmpeg",
+            "-nostdin",
+            "-hide_banner",
+            "-loglevel",
+            "error",
+            "-y",
+            "-f",
+            "lavfi",
+            "-i",
+            "sine=f=440:r=44100:d=6",
+            "-ac",
+            "2",
+            "-c:a",
+            encoder,
+            "-strict",
+            "-2",  # flac and opus in mp4 are flagged experimental by the muxer
+            *extra_args,
+            str(path),
+        ],
+        check=True,
+    )
+    return path
+
+
+def fragmented_mp4(path: Path, encoder: str) -> Path:
+    return encode(path, encoder, "-movflags", "frag_keyframe+empty_moov+default_base_moof", "-frag_duration", "1000000")
+
+
+def header_length(path: Path) -> float:
+    """The length the file states in its header, which is what a player and the tagger read."""
+    return mutagen.File(path).info.length
+
+
+@pytest.mark.parametrize(
+    ("codec", "encoder", "extension"),
+    [
+        (Audio.Codec.AAC, "aac", ".m4a"),
+        (Audio.Codec.ALAC, "alac", ".m4a"),
+        (Audio.Codec.FLAC, "flac", ".flac"),
+        (Audio.Codec.OPUS, "libopus", ".opus"),
+    ],
+)
+def test_remux_states_the_full_length_in_the_codec_container(
+    tmp_path: Path, codec: Audio.Codec, encoder: str, extension: str
+) -> None:
+    source = fragmented_mp4(tmp_path / "track.mp4", encoder)
+    assert header_length(source) == 0.0, "the fixture must reproduce the fault the remux corrects"
+
+    track = Audio(id_="a1", url="https://example.test/a1.mp4", language="en", codec=codec)
+    track.path = source
+
+    assert track.to_music_container() is True
+    assert track.path.suffix == extension
+    assert not source.exists()
+    assert header_length(track.path) == pytest.approx(6.0, abs=0.5)
+    assert write_music_metadata(track.path, make_song()).written, "the remuxed file must accept tags"
+
+
+def test_flac_is_re_encoded_because_a_copy_states_no_length(tmp_path: Path) -> None:
+    """A copy carries the source STREAMINFO over, whose sample count a fragmenting writer zeroes."""
+    source = fragmented_mp4(tmp_path / "track.mp4", "flac")
+    copied = tmp_path / "copied.flac"
+    subprocess.run(
+        [
+            "ffmpeg",
+            "-nostdin",
+            "-hide_banner",
+            "-loglevel",
+            "error",
+            "-y",
+            "-i",
+            str(source),
+            "-map",
+            "0:a:0",
+            "-c",
+            "copy",
+            str(copied),
+        ],
+        check=True,
+    )
+    assert header_length(copied) == 0.0
+
+    track = Audio(id_="a1", url="https://example.test/a1.mp4", language="en", codec=Audio.Codec.FLAC)
+    track.path = source
+    track.to_music_container()
+
+    assert header_length(track.path) == pytest.approx(6.0, abs=0.5)
+    assert mutagen.File(track.path).info.bits_per_sample == 16, "the re-encode must not change the sample format"
+
+
+def test_dolby_digital_plus_falls_back_to_the_mp4_muxer(tmp_path: Path) -> None:
+    """A .m4a name picks FFmpeg's ipod muxer, which refuses Dolby Digital Plus; the remux must not fail on it."""
+    source = encode(tmp_path / "track.mp4", "eac3", "-ar", "48000")
+
+    track = Audio(id_="a1", url="https://example.test/a1.mp4", language="en", codec=Audio.Codec.EC3)
+    track.path = source
+
+    assert track.to_music_container() is True
+    assert track.path.suffix == ".m4a"
+    assert header_length(track.path) == pytest.approx(6.0, abs=0.5)
+    assert write_music_metadata(track.path, make_song()).written
+
+
+def test_a_failed_remux_keeps_the_source_and_removes_its_partial_file(tmp_path: Path) -> None:
+    """FFmpeg creates the output before it fails, and nothing else sweeps the temp directory."""
+    source = encode(tmp_path / "track.mp4", "pcm_s16le", "-f", "mov")  # no MP4 muxer takes PCM
+
+    track = Audio(id_="a1", url="https://example.test/a1.mp4", language="en", codec=None)
+    track.path = source
+
+    with pytest.raises(subprocess.CalledProcessError):
+        track.to_music_container()
+
+    assert track.path == source and source.exists(), "a failed remux must keep the downloaded file"
+    assert not list(tmp_path.glob("track_music*")), "a failed remux must not leave a partial file"

tests/orchestration/test_music_remux_flow.py

@@ -0,0 +1,122 @@
+"""A Song must end as a playable, correctly named, correctly tagged music file.
+
+``remux_music_tracks`` (``unshackle/commands/dl.py``) is the hook the download loop runs
+on every title after repack and before naming. These tests drive the real hook, then the
+real naming, move, and tagging seams in the order ``dl.result()`` composes them. Only the
+surrounding Click command does not run, because it needs a service, a CDM, and a network.
+Every music-specific decision it makes lives in the seams driven here.
+"""
+
+from __future__ import annotations
+
+import logging
+import shutil
+from pathlib import Path
+
+import mutagen
+import pytest
+from mutagen.flac import FLAC
+from pymediainfo import MediaInfo
+
+from tests.core.test_music_container import fragmented_mp4, header_length
+from tests.core.test_music_tagger_roundtrip import make_song
+from unshackle.commands.dl import remux_music_tracks
+from unshackle.core.config import config
+from unshackle.core.tracks import Audio, Tracks
+from unshackle.core.utils import tags
+
+pytestmark = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="requires ffmpeg")
+
+log = logging.getLogger("test_music_remux_flow")
+
+
+@pytest.fixture
+def default_templates():
+    """Pin the naming templates, because the suite shares the live global config."""
+    saved = (config.output_template, config.folder_templates, config.folder_template, config.tag_imdb_tmdb)
+    config.output_template = {"songs": "{track_number}. {title}"}
+    config.folder_templates = {}
+    config.folder_template = ""
+    config.tag_imdb_tmdb = False  # keep provider lookups out of the test
+    yield
+    config.output_template, config.folder_templates, config.folder_template, config.tag_imdb_tmdb = saved
+
+
+def make_audio(path: Path, codec: Audio.Codec, track_id: str) -> Audio:
+    track = Audio(id_=track_id, url=f"https://example.test/{track_id}.mp4", language="en", codec=codec)
+    track.path = path
+    return track
+
+
+def test_the_hook_remuxes_every_audio_track_of_a_song(tmp_path: Path) -> None:
+    song = make_song()
+    song.tracks = Tracks(
+        [
+            make_audio(fragmented_mp4(tmp_path / "Audio_a1.mp4", "flac"), Audio.Codec.FLAC, "a1"),
+            make_audio(fragmented_mp4(tmp_path / "Audio_a2.mp4", "aac"), Audio.Codec.AAC, "a2"),
+        ]
+    )
+
+    remux_music_tracks(song, log)
+
+    assert [t.path.suffix for t in song.tracks.audio] == [".flac", ".m4a"]
+    for track in song.tracks.audio:
+        assert header_length(track.path) == pytest.approx(6.0, abs=0.5)
+
+
+def test_one_broken_track_warns_and_does_not_stop_the_rest(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None:
+    broken = tmp_path / "Audio_a1.mp4"
+    broken.write_bytes(b"")  # ffmpeg cannot read this, so its remux must fail
+    song = make_song()
+    song.tracks = Tracks(
+        [
+            make_audio(broken, Audio.Codec.AAC, "a1"),
+            make_audio(fragmented_mp4(tmp_path / "Audio_a2.mp4", "aac"), Audio.Codec.AAC, "a2"),
+        ]
+    )
+
+    with caplog.at_level(logging.WARNING, logger=log.name):
+        remux_music_tracks(song, log)
+
+    assert "Could not remux audio track a1" in caplog.text
+    assert song.tracks.audio[0].path == broken, "a failed remux must keep the downloaded file"
+    assert song.tracks.audio[1].path.suffix == ".m4a", "the failure must not stop the other tracks"
+
+
+def test_a_non_song_title_is_left_alone(tmp_path: Path) -> None:
+    source = fragmented_mp4(tmp_path / "Audio_a1.mp4", "aac")
+    track = make_audio(source, Audio.Codec.AAC, "a1")
+
+    class NotASong:
+        tracks = Tracks([track])
+
+    remux_music_tracks(NotASong(), log)
+
+    assert track.path == source
+
+
+def test_a_fragmented_flac_song_ends_playable_named_and_tagged(tmp_path: Path, default_templates: None) -> None:
+    """The full post-download flow: hook, MediaInfo naming, final move, music tagging."""
+    song = make_song()
+    source = fragmented_mp4(tmp_path / "Audio_track-0001.mp4", "flac")  # the downloader's naming scheme
+    track = make_audio(source, Audio.Codec.FLAC, "track-0001")
+    song.tracks = Tracks([track])
+    assert header_length(source) == 0.0, "the fixture must reproduce the fault the remux corrects"
+
+    remux_music_tracks(song, log)
+
+    # the naming and move seams, with the arguments dl.result() hands them
+    media_info = MediaInfo.parse(track.path)
+    final_dir = tmp_path / "downloads" / song.get_filename(media_info, show_service=False, folder=True)
+    final_dir.mkdir(parents=True)
+    final_path = final_dir / f"{song.get_filename(media_info, show_service=False)}{track.path.suffix}"
+    shutil.move(track.path, final_path)
+
+    tags.tag_file(final_path, song)
+
+    assert final_path == final_dir / "04.Example.Track.flac"
+    assert not source.exists(), "the fragmented source must be gone"
+    assert mutagen.File(final_path).info.length == pytest.approx(6.0, abs=0.5)
+    audio = FLAC(final_path)
+    assert audio["TITLE"] == ["Example Track"]
+    assert audio["ARTIST"] == ["Example Artist"]

unshackle/commands/dl.py

@@ -210,6 +210,22 @@ def post_script_group(candidate: Any) -> Any:
     return ("title", id(candidate))
 
 
+def remux_music_tracks(title: Any, log: logging.Logger) -> None:
+    """Remux each audio track of a Song into the container of its codec.
+
+    A Song never reaches the muxer, so each downloaded audio track is itself a delivered
+    file. A failure warns, keeps that downloaded track, and moves on to the next one: the
+    user still gets the audio, in a container a player cuts short.
+    """
+    if not isinstance(title, Song):
+        return
+    for track in title.tracks.audio:
+        try:
+            track.to_music_container()
+        except Exception as e:  # noqa: BLE001
+            log.warning(f"Could not remux audio track {track.id} for music: {e}")
+
+
 def apply_service_dl_overrides(ctx: click.Context, service_dl_config: dict[str, Any], log: logging.Logger) -> None:
     """Apply ``services.<TAG>.dl`` config onto ``ctx.params``. Explicit CLI/env values win.
     The config replaces defaults and global ``dl:`` default_map values."""
@@ -3093,6 +3109,8 @@ class dl:
                         except Exception as e:  # noqa: BLE001
                             self.log.warning(f"VUI normalization skipped for {track.id}: {e}")
 
+                remux_music_tracks(title, self.log)
+
                 muxed_paths = []
                 muxed_audio_codecs: dict[Path, Optional[Audio.Codec]] = {}
                 tagged_paths: set[Path] = set()

unshackle/core/music/tagger.py

@@ -36,10 +36,11 @@ try:
     )
     from mutagen.mp3 import MP3
     from mutagen.mp4 import MP4, MP4Cover
+    from mutagen.oggopus import OggOpus
     from mutagen.oggvorbis import OggVorbis
 except ImportError:  # pragma: no cover - optional tagging dependency
     FLAC = Picture = None
-    MP3 = MP4 = MP4Cover = OggVorbis = None
+    MP3 = MP4 = MP4Cover = OggOpus = OggVorbis = None  # type: ignore[assignment]
     ID3NoHeaderError = Exception
     APIC = COMM = TALB = TCOM = TCON = TCOP = TDRC = TIT2 = TPE1 = TPE2 = TPOS = TRCK = TSRC = TXXX = None
     USLT = None
@@ -65,10 +66,10 @@ class MusicMetadataResult:
 
 
 def write_music_metadata(path: Path, song: Any, *, session: Any = None, source_md5: str = "") -> MusicMetadataResult:
-    """Write normalized music metadata for FLAC, Ogg, MP3, and M4A/MP4 audio files."""
+    """Write normalized music metadata for FLAC, Ogg, Opus, MP3, and M4A/MP4 audio files."""
     path = Path(path)
     extension = path.suffix.lower()
-    if extension not in {".flac", ".mp3", ".m4a", ".mp4", ".ogg"}:
+    if extension not in {".flac", ".mp3", ".m4a", ".mp4", ".ogg", ".opus"}:
         return MusicMetadataResult(skipped=True, reason=f"Unsupported music metadata container: {extension}")
 
     if extension == ".flac" and FLAC is None:
@@ -77,7 +78,7 @@ def write_music_metadata(path: Path, song: Any, *, session: Any = None, source_m
         return MusicMetadataResult(skipped=True, reason="install mutagen to write MP3 tags")
     if extension in {".m4a", ".mp4"} and MP4 is None:
         return MusicMetadataResult(skipped=True, reason="install mutagen to write MP4/M4A tags")
-    if extension == ".ogg" and OggVorbis is None:
+    if extension in {".ogg", ".opus"} and OggVorbis is None:
         return MusicMetadataResult(skipped=True, reason="install mutagen to write Ogg tags")
 
     tags = build_tag_values(song, source_md5=source_md5)
@@ -86,8 +87,8 @@ def write_music_metadata(path: Path, song: Any, *, session: Any = None, source_m
 
     if extension == ".flac":
         write_flac_tags(path, tags, cover_data, mime_type)
-    elif extension == ".ogg":
-        write_ogg_tags(path, tags, cover_data, mime_type)
+    elif extension in {".ogg", ".opus"}:
+        write_ogg_tags(path, tags, cover_data, mime_type, OggOpus if extension == ".opus" else OggVorbis)
     elif extension == ".mp3":
         write_mp3_tags(path, tags, cover_data, mime_type)
     else:
@@ -180,8 +181,10 @@ def build_picture(cover_data: Optional[bytes], mime_type: str) -> Optional[Any]:
     return picture
 
 
-def write_ogg_tags(path: Path, tags: dict[str, str], cover_data: Optional[bytes], mime_type: str) -> None:
-    audio = OggVorbis(path)
+def write_ogg_tags(
+    path: Path, tags: dict[str, str], cover_data: Optional[bytes], mime_type: str, reader: Any = None
+) -> None:
+    audio = (reader or OggVorbis)(path)
     for key, value in tags.items():
         audio[key] = [value]
     picture = build_picture(cover_data, mime_type)

unshackle/core/tracks/audio.py

@@ -1,9 +1,11 @@
 from __future__ import annotations
 
 import math
+import subprocess
 from enum import Enum
 from typing import Any, Optional, Union
 
+from unshackle.core import binaries
 from unshackle.core.tracks.track import Track
 
 
@@ -208,6 +210,77 @@ class Audio(Track):
 
         return float(channels)
 
+    def to_music_container(self) -> bool:
+        """Remux the track into the container of its codec, as a standalone audio file.
+
+        Music titles never reach the muxer, so the downloaded file is the delivered file, and
+        after decryption that is still a fragmented MP4. Its header states a length of zero, so
+        a player stops after the first fragment. FLAC inside an MP4 is also not a FLAC stream.
+        A rename cannot correct either fault. Returns True once the new file replaces the
+        downloaded one; every failure raises.
+        """
+        if not self.path or not self.path.exists():
+            raise ValueError("Cannot remux a Track that has not been downloaded.")
+
+        if not binaries.FFMPEG:
+            raise EnvironmentError('FFmpeg executable "ffmpeg" was not found but is required for this call.')
+
+        containers: dict[Optional[Audio.Codec], str] = {
+            Audio.Codec.FLAC: ".flac",
+            Audio.Codec.OPUS: ".opus",
+            Audio.Codec.OGG: ".ogg",
+        }
+        extension = containers.get(self.codec, ".m4a")
+        original_path = self.path
+        output_path = original_path.with_name(f"{original_path.stem}_music{extension}")
+
+        def ffmpeg(*extra_args: str) -> None:
+            subprocess.run(
+                [
+                    str(binaries.FFMPEG),
+                    "-nostdin",
+                    "-hide_banner",
+                    "-loglevel",
+                    "error",
+                    "-y",
+                    "-i",
+                    str(original_path),
+                    "-map",
+                    "0:a:0",
+                    "-map_metadata",
+                    "-1",
+                    # a copy keeps the source STREAMINFO, whose sample count a fragmenting writer
+                    # leaves at zero, so the FLAC states no length; the encoder writes a real one
+                    *(["-c:a", "flac"] if extension == ".flac" else ["-c", "copy"]),
+                    # the fragmented moov is what cuts playback short, so ask for one contiguous header
+                    *(["-movflags", "+faststart"] if extension == ".m4a" else []),
+                    *extra_args,
+                    str(output_path),
+                ],
+                check=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+            )
+
+        try:
+            try:
+                ffmpeg()
+            except subprocess.CalledProcessError as e:
+                if b"not currently supported in container" not in e.stderr:
+                    raise
+                # a .m4a name picks FFmpeg's ipod muxer, which refuses Dolby Digital Plus and the
+                # other codecs Apple never put in an M4A; the plain mp4 muxer accepts them
+                ffmpeg("-f", "mp4")
+        except subprocess.CalledProcessError:
+            # FFmpeg creates the output before it fails, and nothing sweeps the shared temp
+            # directory for it, so a failed run must remove its own partial file
+            output_path.unlink(missing_ok=True)
+            raise
+
+        original_path.unlink()
+        self.path = output_path
+        return True
+
     def get_track_name(self) -> Optional[str]:
         """Return the base Track Name."""
         track_name = super().get_track_name() or ""