From dffb72e41d93ea756ccf0ea9ea99cdd559f8dafa Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Fri, 4 Sep 2026 19:27:39 -0600 Subject: [PATCH] fix(audio): accept immersive channel layouts like 5.1.4 parse_channels rejected a three-figure layout, so a remote download crashed rebuilding the track from JSON. Add channel_total so --channels can compare either form. --- docs/guide/downloading.md | 3 +- tests/tracks/test_audio_channel_layouts.py | 46 ++++++++++++++++++++++ unshackle/commands/dl.py | 4 +- unshackle/core/tracks/audio.py | 28 ++++++++++--- 4 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 tests/tracks/test_audio_channel_layouts.py diff --git a/docs/guide/downloading.md b/docs/guide/downloading.md index de2c9ef9..c218c4e6 100644 --- a/docs/guide/downloading.md +++ b/docs/guide/downloading.md @@ -229,7 +229,8 @@ unshackle dl -a EC3,AAC EXAMPLE 81234567 ### Channels and Atmos - `-c` / `--channels`: desired channel layout, for example `5.1` or `2.0`. Matching is by - ceiling, so `5.1` implicitly matches a `6.0`-reported layout. + ceiling, so `5.1` implicitly matches a `6.0`-reported layout. An immersive layout such as + `5.1.4` matches on its total number of channels, so `-c 10` selects it. - `-naa` / `--noatmos`: exclude Atmos audio tracks from selection. ```shell title="5.1 audio, no Atmos" diff --git a/tests/tracks/test_audio_channel_layouts.py b/tests/tracks/test_audio_channel_layouts.py new file mode 100644 index 00000000..01f25180 --- /dev/null +++ b/tests/tracks/test_audio_channel_layouts.py @@ -0,0 +1,46 @@ +"""Channel values that no float can hold. + +An immersive layout names its height channels in a third figure, e.g. 5.1.4. A service +assigning that straight onto a track keeps it out of validation, but a remote download +sends the track as JSON and rebuilds it through Audio(), which parses the value. +""" + +from __future__ import annotations + +import pytest + +from unshackle.core.tracks import Audio + + +@pytest.mark.parametrize("layout", ["5.1.4", "7.1.2", "9.1.6"]) +def test_an_immersive_layout_survives_construction(layout: str) -> None: + assert Audio(id_="a1", url="https://example.test/a", language="en", channels=layout).channels == layout + + +@pytest.mark.parametrize( + ("value", "expected"), + [("5.1", 5.1), ("2ch", 2.0), ("2", 2.0), ("2.0", 2.0), ("A000", 2.0), ("F801", 5.1), (6, 6.0)], +) +def test_existing_channel_forms_are_unchanged(value: object, expected: float) -> None: + assert Audio.parse_channels(value) == expected + + +@pytest.mark.parametrize("value", ["stereo", "5.1.4.2", "", "..", "5.x.4"]) +def test_a_value_that_is_no_layout_still_raises(value: str) -> None: + with pytest.raises(NotImplementedError): + Audio.parse_channels(value) + + +def test_an_immersive_layout_round_trips_as_a_dict() -> None: + """A remote download rebuilds every track from the dict the server sent.""" + track = Audio(id_="a1", url="https://example.test/a", language="en", channels="5.1.4", bitrate=448000) + assert Audio.from_dict(track.to_dict()).channels == "5.1.4" + + +@pytest.mark.parametrize( + ("channels", "total"), + [("5.1.4", 10.0), ("7.1.2", 10.0), (5.1, 5.1), (6.0, 6.0), ("2.0", 2.0)], +) +def test_channel_total_compares_both_forms(channels: object, total: float) -> None: + """--channels rounds the sub channel up, so both forms have to reach that arithmetic.""" + assert Audio.channel_total(channels) == total diff --git a/unshackle/commands/dl.py b/unshackle/commands/dl.py index 5cd9310b..2473571d 100644 --- a/unshackle/commands/dl.py +++ b/unshackle/commands/dl.py @@ -2703,7 +2703,9 @@ class dl: sys.exit(1) if channels: title.tracks.select_audio( - lambda x: bool(x.channels and math.ceil(x.channels) == math.ceil(channels)) + lambda x: bool( + x.channels and math.ceil(Audio.channel_total(x.channels)) == math.ceil(channels) + ) ) if not title.tracks.audio: self.log.error(f"There's no {channels} Audio Track...") diff --git a/unshackle/core/tracks/audio.py b/unshackle/core/tracks/audio.py index bf02a9e8..ea4e1720 100644 --- a/unshackle/core/tracks/audio.py +++ b/unshackle/core/tracks/audio.py @@ -191,14 +191,17 @@ class Audio(Track): ) @staticmethod - def parse_channels(channels: Union[str, int, float]) -> float: + def parse_channels(channels: Union[str, int, float]) -> Union[float, str]: """ Converts a Channel string to a float representing the audio channel layout. E.g. "3" -> "3.0", "2.1" -> "2.1", ".1" -> "0.1". + An immersive layout names its height channels in a third figure, e.g. "5.1.4", + which no float can hold, so it stays the string it came as. Use channel_total for + arithmetic on either form. + This does not validate channel strings as genuine channel counts or valid layouts. It does not convert the value to assume a sub speaker channel layout, e.g. 5.1->6.0. - It also cannot read expanded surround sound channel layout strings like 7.1.2. """ if isinstance(channels, str): # TODO: Support all possible DASH channel configurations (https://datatracker.ietf.org/doc/html/rfc8216) @@ -206,13 +209,28 @@ class Audio(Track): return 2.0 elif channels.upper() == "F801": return 5.1 - elif channels.replace("ch", "").replace(".", "", 1).isdigit(): - # e.g., '2ch', '2', '2.0', '5.1ch', '5.1' - return float(channels.replace("ch", "")) + layout = channels.replace("ch", "") + if layout.count(".") == 2 and all(part.isdigit() for part in layout.split(".")): + return layout + elif layout.replace(".", "", 1).isdigit(): + return float(layout) raise NotImplementedError(f"Unsupported Channels string value, '{channels}'") return float(channels) + @staticmethod + def channel_total(channels: Union[str, int, float]) -> float: + """ + Total number of channels in either channel form, so a caller can compare both. + + A float layout keeps its own value, because a caller compares those by rounding + the sub channel up, e.g. 5.1 and 6.0 both match. An immersive layout has no such + float, so this adds its figures: "5.1.4" -> 10.0. + """ + if isinstance(channels, str) and channels.count(".") == 2: + return float(sum(int(part) for part in channels.split("."))) + return float(channels) + def to_music_container(self) -> bool: """Remux the track into the container of its codec, as a standalone audio file.