diff --git a/docs/guide/output-and-naming.md b/docs/guide/output-and-naming.md index a8cca915..71f4aa01 100644 --- a/docs/guide/output-and-naming.md +++ b/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. diff --git a/tests/core/test_music_container.py b/tests/core/test_music_container.py new file mode 100644 index 00000000..6fc82ca6 --- /dev/null +++ b/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" diff --git a/tests/orchestration/test_music_remux_flow.py b/tests/orchestration/test_music_remux_flow.py new file mode 100644 index 00000000..fa125ded --- /dev/null +++ b/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"] diff --git a/unshackle/commands/dl.py b/unshackle/commands/dl.py index 66f909e3..15da8124 100644 --- a/unshackle/commands/dl.py +++ b/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..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() diff --git a/unshackle/core/music/tagger.py b/unshackle/core/music/tagger.py index ca8b6237..771dd9f8 100644 --- a/unshackle/core/music/tagger.py +++ b/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) diff --git a/unshackle/core/tracks/audio.py b/unshackle/core/tracks/audio.py index 3d10d300..70f25975 100644 --- a/unshackle/core/tracks/audio.py +++ b/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 ""