Fix HDMI audio detection on Pi4 with dual HDMI ports#2811
Merged
Conversation
Pi4 has two HDMI ports (HDMI-A-1 and HDMI-A-2), but the existing code always hardcodes `default:CARD=vc4hdmi0` regardless of which port has a display connected. This causes no audio when using HDMI-A-2. Changes: - Add `_detect_hdmi_audio_device()` that reads `/sys/class/drm/cardN-HDMI-A-N/status` to find which HDMI port is connected and returns the correct ALSA device - Use `sysdefault:CARD=` instead of `default:CARD=` for more reliable audio output (avoids PulseAudio/dmix interference) - Apply detection for Pi4 and Pi5 HDMI output - Keep headphones and Pi1-3 paths unchanged Tested on Pi4 Model B with single HDMI connected to HDMI-A-1.
…ings Update existing tests for the default: -> sysdefault: switch and add coverage for _detect_hdmi_audio_device(): both port preferences, missing status files, and OSError fall-through. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
There was a problem hiding this comment.
Pull request overview
Fixes HDMI audio device selection on Raspberry Pi 4/5 by auto-detecting which HDMI connector is physically connected and selecting the corresponding ALSA card, improving reliability when default: is affected by PulseAudio/dmix.
Changes:
- Add
_detect_hdmi_audio_device()that reads/sys/class/drm/*/statusfor HDMI connector connection state and returns a matchingsysdefault:CARD=...device string. - Update
get_alsa_audio_device()to use HDMI auto-detection on Pi4/Pi5 and switch several ALSA device strings fromdefault:tosysdefault:. - Update and extend unit tests to cover the new detection helper and the updated device selection behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| viewer/media_player.py | Introduces HDMI connector detection helper and routes Pi4/Pi5 HDMI output through it; switches ALSA device strings to sysdefault:. |
| tests/test_media_player.py | Updates expectations for sysdefault: and adds direct tests for HDMI detection/fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Error Address PR feedback: - Docstring previously conflated DRM card directories (/sys/class/drm/cardN-HDMI-A-N/) with ALSA card names (vc4hdmiN). Rewrite to describe the two layers separately and document the card1 / vc4hdmi0|1 mapping explicitly. - The except OSError branch silently swallowed read failures. Log them at debug level with the path and exception so field debugging has something to grep for; behavior (fallback to vc4hdmi0) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # tests/test_media_player.py
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When audio_output=local on Pi5 the helper hardcoded sysdefault:CARD=vc4hdmi0, which has the same dual-HDMI bug we fix elsewhere — a Pi5 with only HDMI-A-2 connected would still target vc4hdmi0 and produce no audio. Pi5 has no 3.5mm jack, so 'local' is effectively HDMI; route it through the same _detect_hdmi_audio_device() helper used by the 'hdmi' branch. Reviewed-by: copilot-pull-request-reviewer Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… card1
Two follow-ups from Copilot review:
1. _detect_hdmi_audio_device hardcoded /sys/class/drm/card1-HDMI-A-{1,2}.
The vc4 DRM card index isn't stable across images/kernels — on a
layout where it probes as card0 or card2, auto-detection would never
see a connected port and always fall back to vc4hdmi0, reintroducing
the very bug this PR fixes. Discover directories by scanning
/sys/class/drm and matching the HDMI-A-N suffix; the HDMI-A-N to
vc4hdmiN mapping is what's actually stable.
2. get_alsa_audio_device() called get_device_type() up to three times
per dispatch (each re-reading /proc/device-tree/model). Cache it in a
local.
Tests updated to mock os.scandir and add coverage for non-card1 DRM
layouts.
Reviewed-by: copilot-pull-request-reviewer
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sorting the discovered DRM connector list by the full entry name (card0-HDMI-A-2 < card1-HDMI-A-1) could prefer HDMI-A-2 over HDMI-A-1 on multi-card layouts. Build a (suffix, entry) list and sort on the HDMI-A-N suffix instead, so HDMI-A-1 always wins when both connectors report "connected" — regardless of the DRM card prefix. Adds two precedence tests covering same-card and mixed-card layouts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
get_alsa_audio_device() runs on every MPV play() and every VLC set_asset(), so the "No connected HDMI detected" fallback warning spammed the viewer log whenever a Pi4/Pi5 ran with no display attached (e.g. cold boot before the TV powers on, transient EDID loss). Keep the first miss at WARNING — that's the useful signal — and drop subsequent misses to DEBUG via a module-level flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tests/test_media_player.py:1
- This test patches
_detect_hdmi_audio_devicebut doesn’t assert it was called, so it wouldn’t fail if MPV switched back to a hard-codedsysdefault:CARD=vc4hdmi0. Add an assertion (e.g.,_mock_detect.assert_called_once()) aftermpv.player.play()to explicitly verify that the MPV path is using the HDMI auto-detection helper.
import logging
- Replace the WARN-once flag with a single _last_detected_device cache. The first call always logs at the higher level; repeated identical results drop to DEBUG; transitions (HDMI-A-1 ⇄ HDMI-A-2, or detected ⇄ fallback) re-log loudly. Addresses log spam on both the success path (INFO ran on every play()) and the fallback path, and keeps state changes visible. - Quote the os.DirEntry[str] annotation so it isn't evaluated at import time, sidestepping any PEP-585 runtime concern on older interpreters while keeping mypy happy. - Patch logging via string paths in the new test (fixes mypy attr-defined on patch.object(mp.logging, ...)). Extend the test to cover the three transition shapes: fallback→fallback (DEBUG), fallback→connected (INFO), connected→fallback (WARN re-fires). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes #2667 by @Alex1981-tech, rebased onto current
masterwith conflicts resolved against the recentmedia_player.pyrefactor (top-levelget_alsa_audio_device()shared across MPV and VLC) and tests updated.Original work and the fix itself are by @Alex1981-tech; Alex's commit is preserved as-is.
Changes (from @Alex1981-tech)
default:CARD=vc4hdmi0regardless of which port had a display connected — no audio when using HDMI-A-2.default:CARD=ALSA device name is unreliable when PulseAudio or dmix is involved.The fix:
_detect_hdmi_audio_device()that reads/sys/class/drm/cardN-HDMI-A-N/statusto find which HDMI port is connected and returns the matching ALSA device.default:CARD=tosysdefault:CARD=for direct hardware access.get_alsa_audio_device()to a top-level helper used by both players.default:tosysdefault:for consistency.Conflict resolution notes
get_alsa_audio_deviceinto a top-level function shared byMPVMediaPlayerandVLCMediaPlayer. Alex's PR was based on a version where it lived as a method onVLCMediaPlayer. Alex's intent has been ported onto the current top-level helper.style: apply ruff format) was redundant on the resolved structure and was dropped during rebase; an equivalent format pass is already folded into his fix commit.Tests
tests/test_media_player.pycases updated fordefault:→sysdefault:._detect_hdmi_audio_deviceand asserts it's called._detect_hdmi_audio_deviceitself: first-port-connected, second-port-only-connected, no status files (fallback), and OSError on read (fallback).Test plan
uv run ruff check viewer/media_player.py tests/test_media_player.py— cleanuv run ruff format --check viewer/media_player.py tests/test_media_player.py— cleanuv run pytest -m "not integration"— 413 passed🤖 Generated with Claude Code