fix(viewer): route audio_output to a live PulseAudio sink on x86/arm64

src/anthias_viewer/media_player.py

@@ -184,79 +184,148 @@ def _detect_hdmi_audio_device() -> str:
     return device
 
 
-# Once-per-process flag for _log_arm64_alsa_default_once() — we don't
-# want every play() call repeating the same INFO line.
-_arm64_alsa_logged = False
-
-
-def _log_arm64_alsa_default_once() -> None:
-    """Log the kernel's ALSA card listing so silent-HDMI reports are
-    debuggable from journalctl alone. Reads /proc/asound/cards (always
-    present when sound is registered) rather than shelling to
-    `aplay -l` — the viewer image deliberately doesn't ship
-    alsa-utils, so the subprocess form would always fall through to
-    a "not found" error and surface no useful info.
-    """
-    global _arm64_alsa_logged
-    if _arm64_alsa_logged:
-        return
-    _arm64_alsa_logged = True
-    try:
-        with open('/proc/asound/cards') as f:
-            listing = f.read().strip() or '<no cards registered>'
-    except OSError as exc:
-        listing = f'<could not read /proc/asound/cards: {exc}>'
-    logging.info(
-        'arm64: using the default audio device for HDMI audio '
-        '(resolves to the PulseAudio default sink — see '
-        'start_pulseaudio in bin/start_viewer.sh). If audio is '
-        'silent, check `pactl list short sinks` in the viewer '
-        'container. Registered ALSA cards:\n%s',
-        listing,
-    )
+# Last sink `_resolve_pulse_sink()` returned in this process, so a
+# stable resolution doesn't repeat the same INFO/WARNING line on every
+# play()/set_asset() call — only transitions (a different sink, or the
+# fallback kicking in) are logged loudly.
+_last_pulse_sink: str | None = None
 
 
-def get_alsa_audio_device() -> str:
-    # device_helper.get_device_type() reads /proc/device-tree/model and
-    # falls back to 'pi1' for any aarch64 host whose model line isn't a
-    # Pi regex match (Rock Pi, Orange Pi, Banana Pi, …). The Pi-firmware
-    # ALSA card names below (vc4hdmi*, "Headphones") don't exist on
-    # those boards, so route via DEVICE_TYPE env first and only fall
-    # through to the Pi-name dispatch when we're actually on a Pi.
-    if os.environ.get('DEVICE_TYPE') in ARM64_DEVICE_TYPES:
-        # No portable per-SoC HDMI card name across Rockchip /
-        # Allwinner / Amlogic, so defer to the `default` device —
-        # which VideoView::resolveAlsaDevice resolves to the
-        # PulseAudio default sink (the daemon start_viewer.sh runs;
-        # Debian's Qt 6 Multimedia only has a PulseAudio backend).
-        # Log the chosen device at INFO once per process so a
-        # silent-HDMI report carries enough breadcrumbs to debug
-        # from journalctl alone.
-        _log_arm64_alsa_default_once()
-        return 'default'
-
-    device_type = get_device_type()
-    if settings['audio_output'] == 'local':
+def _pi_alsa_device(device_type: str, audio_output: str) -> str | None:
+    """Resolve the fixed ALSA card name for a Raspberry Pi board.
+
+    Pi firmware exposes stable card names (``vc4hdmi*`` for HDMI,
+    ``Headphones`` for the 3.5 mm jack), so the audio_output setting
+    maps straight onto them. Returns ``None`` for any non-Pi board
+    (x86, arm64 SBCs) whose card names aren't portable — the caller
+    then falls through to runtime PulseAudio sink discovery.
+    """
+    if audio_output == 'local':
         if device_type == 'pi5':
             return _detect_hdmi_audio_device()
-
-        return 'plughw:CARD=Headphones'
+        if device_type in ('pi1', 'pi2', 'pi3', 'pi4'):
+            return 'plughw:CARD=Headphones'
+        return None
+
+    # 'hdmi' (the default for any other value)
+    if device_type in ('pi4', 'pi5'):
+        return _detect_hdmi_audio_device()
+    if device_type in ('pi1', 'pi2', 'pi3'):
+        return 'sysdefault:CARD=vc4hdmi'
+    return None
+
+
+def _list_pulse_sinks() -> list[str]:
+    """Return the sink names the running PulseAudio daemon exposes.
+
+    Uses ``pactl`` (pulseaudio-utils, which the viewer image ships —
+    unlike alsa-utils/``aplay``). The viewer process already runs with
+    ``XDG_RUNTIME_DIR`` pointed at the pulse socket that
+    start_pulseaudio() created, so the child inherits it. Returns an
+    empty list if pulse is unreachable.
+    """
+    try:
+        completed = subprocess.run(
+            ['pactl', 'list', 'short', 'sinks'],
+            capture_output=True,
+            text=True,
+            timeout=5,
+            check=True,
+        )
+    except (OSError, subprocess.SubprocessError) as exc:
+        logging.debug('Could not list PulseAudio sinks: %s', exc)
+        return []
+
+    sinks = []
+    for line in completed.stdout.splitlines():
+        # `pactl list short sinks` is tab-separated:
+        # index<TAB>name<TAB>module<TAB>sample-spec<TAB>state
+        fields = line.split('\t')
+        if len(fields) >= 2 and fields[1]:
+            sinks.append(fields[1])
+    return sinks
+
+
+def _select_pulse_sink(sinks: list[str], audio_output: str) -> str | None:
+    """Pick the sink matching the requested output, or ``None``.
+
+    module-alsa-card names its sinks ``alsa_output.<card>.<profile>``
+    where <profile> is a standardised token (``hdmi-stereo``,
+    ``analog-stereo``, ``iec958-stereo``, …). That profile token — not
+    the card name — is the reliable HDMI-vs-analog discriminator: the
+    Dell Latitude in forum #6749 puts both an ``analog`` and an
+    ``hdmi`` sink on the *same* HDA card, so matching on the card name
+    alone can't tell them apart.
+    """
+    if audio_output == 'local':
+        keywords = ('analog', 'headphone', 'speaker', 'lineout')
     else:
-        if device_type in ['pi4', 'pi5']:
-            return _detect_hdmi_audio_device()
-        elif device_type in ['pi1', 'pi2', 'pi3']:
-            return 'sysdefault:CARD=vc4hdmi'
+        keywords = ('hdmi', 'displayport', 'iec958')
+
+    for sink in sinks:
+        name = sink.lower()
+        if any(keyword in name for keyword in keywords):
+            return sink
+    return None
+
+
+def _resolve_pulse_sink(audio_output: str) -> str:
+    """Map the audio_output setting onto a live PulseAudio sink.
+
+    x86 and arm64 SBCs run Qt 6 Multimedia, whose only backend on
+    Debian is PulseAudio, and have no portable per-SoC ALSA card name.
+    Rather than hard-code one (the old code returned ``default`` and so
+    always followed whatever sink pulse defaulted to — issue #3208), we
+    ask the running daemon which sinks exist and route the setting to
+    the matching one. The bare sink name is returned;
+    ``_build_video_options`` prepends ``alsa/`` and
+    ``VideoView::resolveAlsaDevice`` matches it against
+    ``QAudioDevice::id()``.
+
+    Falls back to ``default`` — the PulseAudio default sink — when
+    pulse is unreachable or no sink matches, preserving the previous
+    behaviour rather than silencing audio.
+    """
+    sinks = _list_pulse_sinks()
+    chosen = _select_pulse_sink(sinks, audio_output)
+    result = chosen if chosen is not None else 'default'
+
+    global _last_pulse_sink
+    if result != _last_pulse_sink:
+        if chosen is not None:
+            logging.info(
+                'audio_output=%r routed to PulseAudio sink %r',
+                audio_output,
+                chosen,
+            )
         else:
-            # x86 fallback: ALSA card names vary across Intel / AMD /
-            # Nvidia HDA chipsets, so there's no portable per-SoC name
-            # to hard-code (the old 'sysdefault:CARD=HID' matched no
-            # real card on standard HDA setups). Defer to the `default`
-            # device — which VideoView::resolveAlsaDevice resolves to
-            # the PulseAudio default sink (x86 is a Qt 6 board and
-            # Debian's Qt 6 Multimedia only has a PulseAudio backend;
-            # the daemon is started by start_viewer.sh). Mirrors the
-            # ARM64 path above.
-            return 'default'
+            logging.warning(
+                'audio_output=%r: no matching PulseAudio sink among %s; '
+                'falling back to the default sink. Check '
+                '`pactl list short sinks` in the viewer container.',
+                audio_output,
+                sinks or '<none>',
+            )
+        _last_pulse_sink = result
+    return result
+
+
+def get_alsa_audio_device() -> str:
+    audio_output = settings['audio_output']
+
+    # Raspberry Pi boards have stable, portable ALSA card names, so the
+    # setting maps directly onto them. get_device_type() reads
+    # /proc/device-tree/model; the ARM64_DEVICE_TYPES env gate keeps
+    # non-Pi aarch64 boards (Rock Pi, Orange Pi, …) — which
+    # get_device_type() would misclassify as 'pi1' — out of the Pi
+    # dispatch and on the PulseAudio path below.
+    if os.environ.get('DEVICE_TYPE') not in ARM64_DEVICE_TYPES:
+        pi_device = _pi_alsa_device(get_device_type(), audio_output)
+        if pi_device is not None:
+            return pi_device
+
+    # x86 + arm64 SBCs: resolve the setting against a live pulse sink.
+    return _resolve_pulse_sink(audio_output)
 
 
 class MediaPlayer:

tests/test_media_player.py

@@ -125,13 +125,40 @@ def test_play_omits_libmpv_era_options(mpv: _MPVFixtures) -> None:
         assert legacy not in options, legacy
 
 
-def test_play_uses_default_alsa_device_on_arm64(mpv: _MPVFixtures) -> None:
-    # No portable per-SoC HDMI card name exists across Rockchip /
-    # Allwinner / Amlogic, so arm64 defers to ALSA's `default`
-    # device rather than the Pi-firmware vc4hdmi* / HID cards the
-    # regular dispatch would otherwise pick.
+def test_play_routes_to_hdmi_sink_on_arm64(mpv: _MPVFixtures) -> None:
+    # arm64 SBCs have no portable per-SoC ALSA card name, so the
+    # audio_output setting is resolved against a live PulseAudio sink
+    # (issue #3208). With HDMI selected, playback routes to the HDMI
+    # sink rather than whatever pulse happens to default to.
     mpv.player.set_asset('file:///test/video.mp4', 30)
-    with patch.dict('os.environ', {'DEVICE_TYPE': 'arm64'}):
+    with (
+        patch.dict('os.environ', {'DEVICE_TYPE': 'arm64'}),
+        patch(
+            'anthias_viewer.media_player._list_pulse_sinks',
+            return_value=[
+                'alsa_output.hdmisound.stereo-fallback',
+                'alsa_output.Analog.stereo-fallback',
+            ],
+        ),
+    ):
+        mpv.player.play()
+
+    options = _last_play_options(mpv.mock_bus)
+    assert (
+        options['audio-device'] == 'alsa/alsa_output.hdmisound.stereo-fallback'
+    )
+
+
+def test_play_falls_back_to_default_sink_on_arm64(mpv: _MPVFixtures) -> None:
+    # When pulse is unreachable (no sinks listed), fall back to the
+    # default sink rather than silencing audio.
+    mpv.player.set_asset('file:///test/video.mp4', 30)
+    with (
+        patch.dict('os.environ', {'DEVICE_TYPE': 'arm64'}),
+        patch(
+            'anthias_viewer.media_player._list_pulse_sinks', return_value=[]
+        ),
+    ):
         mpv.player.play()
 
     options = _last_play_options(mpv.mock_bus)
@@ -341,17 +368,123 @@ def test_hdmi_on_pi1_pi2_pi3_uses_vc4hdmi(
         assert get_alsa_audio_device() == 'sysdefault:CARD=vc4hdmi'
 
 
-def test_hdmi_on_x86_uses_alsa_default(alsa_settings: Any) -> None:
-    # No portable per-SoC ALSA card name across Intel / AMD / Nvidia
-    # HDA chipsets, so x86 defers to `default`, which the C++ side
-    # resolves to the PulseAudio default sink.
+# Real-world `pactl list short sinks` layouts from the testbeds:
+# an HDA x86 box (analog + HDMI on separate cards) and a Rock Pi 4B.
+_X86_SINKS = [
+    'alsa_output.HID.analog-stereo',
+    'alsa_output.PCH.hdmi-stereo',
+]
+_ROCKPI_SINKS = [
+    'alsa_output.hdmisound.stereo-fallback',
+    'alsa_output.Analog.stereo-fallback',
+]
+
+
+def test_hdmi_on_x86_routes_to_hdmi_sink(alsa_settings: Any) -> None:
+    # x86 resolves HDMI to the live HDMI sink instead of the pulse
+    # default (issue #3208).
     alsa_settings.__getitem__.return_value = 'hdmi'
-    with patch(
-        'anthias_viewer.media_player.get_device_type', return_value='x86'
+    with (
+        patch(
+            'anthias_viewer.media_player.get_device_type', return_value='x86'
+        ),
+        patch(
+            'anthias_viewer.media_player._list_pulse_sinks',
+            return_value=_X86_SINKS,
+        ),
+    ):
+        assert get_alsa_audio_device() == 'alsa_output.PCH.hdmi-stereo'
+
+
+def test_local_on_x86_routes_to_analog_sink(alsa_settings: Any) -> None:
+    alsa_settings.__getitem__.return_value = 'local'
+    with (
+        patch(
+            'anthias_viewer.media_player.get_device_type', return_value='x86'
+        ),
+        patch(
+            'anthias_viewer.media_player._list_pulse_sinks',
+            return_value=_X86_SINKS,
+        ),
+    ):
+        assert get_alsa_audio_device() == 'alsa_output.HID.analog-stereo'
+
+
+def test_x86_falls_back_to_default_when_no_matching_sink(
+    alsa_settings: Any,
+) -> None:
+    # HDMI requested but only an analog sink exists -> default sink.
+    alsa_settings.__getitem__.return_value = 'hdmi'
+    with (
+        patch(
+            'anthias_viewer.media_player.get_device_type', return_value='x86'
+        ),
+        patch(
+            'anthias_viewer.media_player._list_pulse_sinks',
+            return_value=['alsa_output.HID.analog-stereo'],
+        ),
     ):
         assert get_alsa_audio_device() == 'default'
 
 
+@pytest.mark.parametrize(
+    'audio_output,sinks,expected',
+    [
+        ('hdmi', _X86_SINKS, 'alsa_output.PCH.hdmi-stereo'),
+        ('local', _X86_SINKS, 'alsa_output.HID.analog-stereo'),
+        ('hdmi', _ROCKPI_SINKS, 'alsa_output.hdmisound.stereo-fallback'),
+        ('local', _ROCKPI_SINKS, 'alsa_output.Analog.stereo-fallback'),
+        # Same HDA card exposes both profiles (forum #6749 Dell): the
+        # profile token, not the card name, disambiguates.
+        (
+            'hdmi',
+            [
+                'alsa_output.pci-0000_00_1f.3.analog-stereo',
+                'alsa_output.pci-0000_00_1f.3.hdmi-stereo',
+            ],
+            'alsa_output.pci-0000_00_1f.3.hdmi-stereo',
+        ),
+        ('hdmi', ['alsa_output.HID.analog-stereo'], None),
+        ('local', [], None),
+    ],
+)
+def test_select_pulse_sink(
+    audio_output: str, sinks: list[str], expected: str | None
+) -> None:
+    from anthias_viewer.media_player import _select_pulse_sink
+
+    assert _select_pulse_sink(sinks, audio_output) == expected
+
+
+def test_list_pulse_sinks_parses_pactl_output() -> None:
+    from anthias_viewer.media_player import _list_pulse_sinks
+
+    stdout = (
+        '0\talsa_output.HID.analog-stereo\tmodule-alsa-card.c\t'
+        's16le 2ch 48000Hz\tSUSPENDED\n'
+        '1\talsa_output.PCH.hdmi-stereo\tmodule-alsa-card.c\t'
+        's16le 2ch 44100Hz\tSUSPENDED\n'
+    )
+    with patch(
+        'anthias_viewer.media_player.subprocess.run',
+        return_value=MagicMock(stdout=stdout),
+    ):
+        assert _list_pulse_sinks() == [
+            'alsa_output.HID.analog-stereo',
+            'alsa_output.PCH.hdmi-stereo',
+        ]
+
+
+def test_list_pulse_sinks_returns_empty_when_pactl_missing() -> None:
+    from anthias_viewer.media_player import _list_pulse_sinks
+
+    with patch(
+        'anthias_viewer.media_player.subprocess.run',
+        side_effect=FileNotFoundError('pactl'),
+    ):
+        assert _list_pulse_sinks() == []
+
+
 class _FakeDirEntry:
     """Minimal os.DirEntry stand-in for scandir tests."""