mirror of
https://gitlab.freedesktop.org/pipewire/pipewire.git
synced 2025-10-29 05:40:27 -04:00
alsa: Fix IEC958 digital output not unmuted on path activation
When selecting an HDMI/DisplayPort (IEC958) output path, the hardware
mute switch remains in kernel default state (muted), causing no audio
output despite correct software routing.
Root cause: pa_alsa_path_select() only sets mute switches when
mute_during_activation is enabled. No mixer paths enable this setting,
making the switch configuration code unreachable for IEC958 paths.
Solution: Always set mute switches to match device mute status after
path activation, regardless of mute_during_activation setting.
Testing: Added test-alsa-path-select tool to verify the fix.
- Loads mixer path and calls pa_alsa_path_select()
- Verifies switch states match expected values
- Tested on AMD Radeon HDMI and Realtek ALC257 analog
Manual verification:
- Before: IEC958 switch OFF, no audio
- After: IEC958 switch set correctly, audio works
This bug was inherited from PulseAudio's ALSA mixer path code where
HDMI path configurations lack IEC958 unmute sections.
Fixes: https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/3261
See-Also: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/562
See-Also: 33be660e4b
See-Also: https://bugs.launchpad.net/hundredpapercuts/+bug/681996
This commit is contained in:
Daniel Nouri
parent
7d781e696f
commit
571ff40704
3 changed files with 374 additions and 7 deletions
|
|
@ -1479,13 +1479,23 @@ int pa_alsa_path_select(pa_alsa_path *p, pa_alsa_setting *s, snd_mixer_t *m, boo
|
|||
if (s)
|
||||
setting_select(s, m);
|
||||
|
||||
/* Finally restore hw mute to the device mute status. */
|
||||
if (p->mute_during_activation) {
|
||||
PA_LLIST_FOREACH(e, p->elements) {
|
||||
if (e->switch_use == PA_ALSA_SWITCH_MUTE) {
|
||||
if (element_set_switch(e, m, !device_is_muted) < 0)
|
||||
return -1;
|
||||
}
|
||||
/* Set hw mute switches to match device mute status.
|
||||
*
|
||||
* When mute_during_activation is enabled, switches are temporarily muted
|
||||
* during path setup to avoid pops/clicks, and this restores them.
|
||||
*
|
||||
* When mute_during_activation is disabled (default), switches may still be
|
||||
* in their kernel default state (often muted for digital outputs like IEC958).
|
||||
* We must explicitly unmute them if the device isn't muted.
|
||||
*
|
||||
* This ensures HDMI/DisplayPort audio (IEC958) works correctly, as these
|
||||
* digital output switches default to muted in ALSA drivers but are not
|
||||
* automatically enabled when the path is selected.
|
||||
*/
|
||||
PA_LLIST_FOREACH(e, p->elements) {
|
||||
if (e->switch_use == PA_ALSA_SWITCH_MUTE) {
|
||||
if (element_set_switch(e, m, !device_is_muted) < 0)
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -20,3 +20,12 @@ acp_lib = static_library(
|
|||
dependencies : [ spa_dep, alsa_dep, mathlib, ]
|
||||
)
|
||||
acp_dep = declare_dependency(link_with: acp_lib)
|
||||
|
||||
executable('test-alsa-path-select',
|
||||
[ 'test-alsa-path-select.c' ],
|
||||
c_args : acp_c_args,
|
||||
include_directories : [configinc, includes_inc ],
|
||||
dependencies : [ spa_dep, alsa_dep, mathlib, ],
|
||||
link_with : [ acp_lib ],
|
||||
install : false,
|
||||
)
|
||||
|
|
|
|||
348
spa/plugins/alsa/acp/test-alsa-path-select.c
Normal file
348
spa/plugins/alsa/acp/test-alsa-path-select.c
Normal file
|
|
@ -0,0 +1,348 @@
|
|||
/* ALSA path select test tool */
|
||||
/* SPDX-FileCopyrightText: Copyright © 2025 Daniel Nouri */
|
||||
/* SPDX-License-Identifier: MIT */
|
||||
|
||||
/*
|
||||
* Tests pa_alsa_path_select() to verify mute switches are correctly set
|
||||
* based on device mute status, regardless of mute_during_activation setting.
|
||||
*
|
||||
* Regression test for HDMI/DisplayPort audio bug where IEC958 switches
|
||||
* remain in kernel default state (OFF) because pa_alsa_path_select() only
|
||||
* set switches when mute_during_activation=true, but no paths enable this.
|
||||
*
|
||||
* Test verification:
|
||||
* Before fix: IEC958 switch stays OFF after path select → test FAILS
|
||||
* After fix: IEC958 switch set correctly based on device state → test PASSES
|
||||
*
|
||||
* Usage:
|
||||
* test-alsa-path-select -c <card> [-p <path>] [-m] [-v]
|
||||
*
|
||||
* Examples:
|
||||
* test-alsa-path-select -c 1 # IEC958 unmuted (expect ON)
|
||||
* test-alsa-path-select -c 1 -m # IEC958 muted (expect OFF)
|
||||
* test-alsa-path-select -c 0 -p analog-output # Regression test
|
||||
*
|
||||
* Exit codes:
|
||||
* 0 = Test passed
|
||||
* 1 = Test failed
|
||||
* 2 = Setup error
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <stdbool.h>
|
||||
#include <string.h>
|
||||
#include <errno.h>
|
||||
#include <getopt.h>
|
||||
#include <alsa/asoundlib.h>
|
||||
|
||||
#include "alsa-mixer.h"
|
||||
#include "alsa-util.h"
|
||||
|
||||
#define DEFAULT_PATHS_DIR "/usr/share/alsa-card-profile/mixer/paths"
|
||||
|
||||
struct test_config {
|
||||
int card;
|
||||
const char *path_name;
|
||||
const char *paths_dir;
|
||||
bool device_muted;
|
||||
bool verbose;
|
||||
};
|
||||
|
||||
static void show_help(const char *name)
|
||||
{
|
||||
printf("Usage: %s [options]\n", name);
|
||||
printf("\n");
|
||||
printf("Test tool for pa_alsa_path_select() - verifies IEC958 unmute fix\n");
|
||||
printf("\n");
|
||||
printf("Options:\n");
|
||||
printf(" -c, --card <N> ALSA card number (required)\n");
|
||||
printf(" -p, --path <name> Mixer path name (default: iec958-stereo-output)\n");
|
||||
printf(" -d, --paths-dir <dir> Path to mixer path configs (default: %s)\n", DEFAULT_PATHS_DIR);
|
||||
printf(" -m, --muted Test with device muted (default: unmuted)\n");
|
||||
printf(" -v, --verbose Verbose output\n");
|
||||
printf(" -h, --help Show this help\n");
|
||||
printf("\n");
|
||||
printf("Examples:\n");
|
||||
printf(" # Test IEC958 unmute on card 1\n");
|
||||
printf(" %s -c 1\n", name);
|
||||
printf("\n");
|
||||
printf(" # Test analog output\n");
|
||||
printf(" %s -c 0 -p analog-output\n", name);
|
||||
printf("\n");
|
||||
printf(" # Test with device muted\n");
|
||||
printf(" %s -c 1 -m\n", name);
|
||||
printf("\n");
|
||||
printf("Exit codes:\n");
|
||||
printf(" 0 - Test passed (switches set correctly)\n");
|
||||
printf(" 1 - Test failed (switches not set as expected)\n");
|
||||
printf(" 2 - Setup error (path not found, hardware error, etc.)\n");
|
||||
}
|
||||
|
||||
static int parse_args(int argc, char *argv[], struct test_config *config)
|
||||
{
|
||||
static const struct option long_options[] = {
|
||||
{ "card", required_argument, NULL, 'c' },
|
||||
{ "path", required_argument, NULL, 'p' },
|
||||
{ "paths-dir", required_argument, NULL, 'd' },
|
||||
{ "muted", no_argument, NULL, 'm' },
|
||||
{ "verbose", no_argument, NULL, 'v' },
|
||||
{ "help", no_argument, NULL, 'h' },
|
||||
{ NULL, 0, NULL, 0 }
|
||||
};
|
||||
|
||||
int c;
|
||||
config->card = -1;
|
||||
config->path_name = "iec958-stereo-output.conf";
|
||||
config->paths_dir = DEFAULT_PATHS_DIR;
|
||||
config->device_muted = false;
|
||||
config->verbose = false;
|
||||
|
||||
while ((c = getopt_long(argc, argv, "c:p:d:mvh", long_options, NULL)) != -1) {
|
||||
switch (c) {
|
||||
case 'c':
|
||||
config->card = atoi(optarg);
|
||||
break;
|
||||
case 'p':
|
||||
config->path_name = optarg;
|
||||
break;
|
||||
case 'd':
|
||||
config->paths_dir = optarg;
|
||||
break;
|
||||
case 'm':
|
||||
config->device_muted = true;
|
||||
break;
|
||||
case 'v':
|
||||
config->verbose = true;
|
||||
break;
|
||||
case 'h':
|
||||
show_help(argv[0]);
|
||||
return -1;
|
||||
default:
|
||||
show_help(argv[0]);
|
||||
return -2;
|
||||
}
|
||||
}
|
||||
|
||||
if (config->card < 0) {
|
||||
fprintf(stderr, "Error: Card number is required (-c <N>)\n\n");
|
||||
show_help(argv[0]);
|
||||
return -2;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int get_switch_state(snd_mixer_t *mixer, const char *name, int index, bool *state)
|
||||
{
|
||||
snd_mixer_elem_t *elem;
|
||||
snd_mixer_selem_id_t *sid;
|
||||
int val;
|
||||
|
||||
snd_mixer_selem_id_alloca(&sid);
|
||||
snd_mixer_selem_id_set_name(sid, name);
|
||||
snd_mixer_selem_id_set_index(sid, index);
|
||||
|
||||
elem = snd_mixer_find_selem(mixer, sid);
|
||||
if (!elem) {
|
||||
fprintf(stderr, "Warning: Element '%s',%d not found\n", name, index);
|
||||
return -ENOENT;
|
||||
}
|
||||
|
||||
if (!snd_mixer_selem_has_playback_switch(elem)) {
|
||||
fprintf(stderr, "Warning: Element '%s',%d has no playback switch\n", name, index);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_MONO, &val) < 0) {
|
||||
if (snd_mixer_selem_get_playback_switch(elem, SND_MIXER_SCHN_FRONT_LEFT, &val) < 0) {
|
||||
fprintf(stderr, "Error: Cannot read switch state for '%s',%d\n", name, index);
|
||||
return -EIO;
|
||||
}
|
||||
}
|
||||
|
||||
*state = val ? true : false;
|
||||
return 0;
|
||||
}
|
||||
|
||||
int main(int argc, char *argv[])
|
||||
{
|
||||
struct test_config config;
|
||||
pa_alsa_path *path = NULL;
|
||||
snd_mixer_t *mixer = NULL;
|
||||
char card_name[64];
|
||||
int ret = 2; // Setup error by default
|
||||
int err;
|
||||
|
||||
/* Parse arguments */
|
||||
err = parse_args(argc, argv, &config);
|
||||
if (err < 0) {
|
||||
return err == -1 ? 0 : 2; // -1 is help, -2 is error
|
||||
}
|
||||
|
||||
printf("Testing pa_alsa_path_select() with:\n");
|
||||
printf(" Card: %d\n", config.card);
|
||||
printf(" Path: %s\n", config.path_name);
|
||||
printf(" Paths dir: %s\n", config.paths_dir);
|
||||
printf(" Device muted: %s\n", config.device_muted ? "yes" : "no");
|
||||
printf("\n");
|
||||
|
||||
/* Load mixer path */
|
||||
path = pa_alsa_path_new(config.paths_dir, config.path_name, PA_ALSA_DIRECTION_OUTPUT);
|
||||
if (!path) {
|
||||
fprintf(stderr, "Error: Failed to load path '%s' from %s\n",
|
||||
config.path_name, config.paths_dir);
|
||||
fprintf(stderr, "Make sure the path file exists: %s/%s.conf\n",
|
||||
config.paths_dir, config.path_name);
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* Count elements BEFORE probing */
|
||||
int element_count_before = 0;
|
||||
pa_alsa_element *e_temp;
|
||||
PA_LLIST_FOREACH(e_temp, path->elements) {
|
||||
element_count_before++;
|
||||
}
|
||||
printf("Elements in path BEFORE probing: %d\n", element_count_before);
|
||||
|
||||
if (config.verbose && element_count_before > 0) {
|
||||
printf("Loaded path: %s\n", path->name ? path->name : config.path_name);
|
||||
printf(" mute_during_activation: %s\n", path->mute_during_activation ? "yes" : "no");
|
||||
printf(" Elements:\n");
|
||||
|
||||
PA_LLIST_FOREACH(e_temp, path->elements) {
|
||||
const char *switch_use;
|
||||
switch (e_temp->switch_use) {
|
||||
case PA_ALSA_SWITCH_MUTE: switch_use = "mute"; break;
|
||||
case PA_ALSA_SWITCH_ON: switch_use = "on"; break;
|
||||
case PA_ALSA_SWITCH_OFF: switch_use = "off"; break;
|
||||
default: switch_use = "ignore"; break;
|
||||
}
|
||||
printf(" - %s (index %d): switch=%s\n",
|
||||
e_temp->alsa_id.name, e_temp->alsa_id.index, switch_use);
|
||||
}
|
||||
printf("\n");
|
||||
}
|
||||
|
||||
/* Open ALSA mixer */
|
||||
snprintf(card_name, sizeof(card_name), "hw:%d", config.card);
|
||||
|
||||
if (snd_mixer_open(&mixer, 0) < 0) {
|
||||
fprintf(stderr, "Error: Failed to open mixer\n");
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
if (snd_mixer_attach(mixer, card_name) < 0) {
|
||||
fprintf(stderr, "Error: Failed to attach mixer to %s\n", card_name);
|
||||
fprintf(stderr, "Make sure card %d exists (check with: aplay -l)\n", config.card);
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
if (snd_mixer_selem_register(mixer, NULL, NULL) < 0) {
|
||||
fprintf(stderr, "Error: Failed to register mixer\n");
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
if (snd_mixer_load(mixer) < 0) {
|
||||
fprintf(stderr, "Error: Failed to load mixer elements\n");
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* Probe the path to ensure elements are available */
|
||||
printf("Probing path...\n");
|
||||
if (pa_alsa_path_probe(path, NULL, mixer, false) < 0) {
|
||||
fprintf(stderr, "Error: Path probe failed - required elements not found on card\n");
|
||||
goto cleanup;
|
||||
}
|
||||
printf("Path probe: OK\n\n");
|
||||
|
||||
/* Record switch states before activation */
|
||||
printf("Switch states BEFORE pa_alsa_path_select():\n");
|
||||
pa_alsa_element *e;
|
||||
int mute_switch_count = 0;
|
||||
PA_LLIST_FOREACH(e, path->elements) {
|
||||
if (e->switch_use == PA_ALSA_SWITCH_MUTE) {
|
||||
bool state;
|
||||
mute_switch_count++;
|
||||
if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &state) == 0) {
|
||||
printf(" %s,%d = %s\n", e->alsa_id.name, e->alsa_id.index,
|
||||
state ? "ON" : "OFF");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (mute_switch_count == 0) {
|
||||
fprintf(stderr, "Warning: No mute switches found in path\n");
|
||||
printf("This test requires a path with switch=mute elements\n");
|
||||
ret = 2;
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
/* Call pa_alsa_path_select() - THE FUNCTION UNDER TEST */
|
||||
printf("\nCalling pa_alsa_path_select(path, NULL, mixer, device_is_muted=%s)...\n",
|
||||
config.device_muted ? "true" : "false");
|
||||
|
||||
err = pa_alsa_path_select(path, NULL, mixer, config.device_muted);
|
||||
if (err < 0) {
|
||||
fprintf(stderr, "Error: pa_alsa_path_select() failed with error %d\n", err);
|
||||
ret = 2;
|
||||
goto cleanup;
|
||||
}
|
||||
printf("pa_alsa_path_select(): OK\n\n");
|
||||
|
||||
/* Verify switch states after activation */
|
||||
printf("Switch states AFTER pa_alsa_path_select():\n");
|
||||
bool all_correct = true;
|
||||
bool expected_state = !config.device_muted; // unmuted device = switches ON
|
||||
|
||||
PA_LLIST_FOREACH(e, path->elements) {
|
||||
if (e->switch_use == PA_ALSA_SWITCH_MUTE) {
|
||||
bool actual_state;
|
||||
if (get_switch_state(mixer, e->alsa_id.name, e->alsa_id.index, &actual_state) == 0) {
|
||||
printf(" %s,%d = %s", e->alsa_id.name, e->alsa_id.index,
|
||||
actual_state ? "ON" : "OFF");
|
||||
|
||||
if (actual_state == expected_state) {
|
||||
printf(" ✓\n");
|
||||
} else {
|
||||
printf(" ✗ (expected %s)\n", expected_state ? "ON" : "OFF");
|
||||
all_correct = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/* Determine test result */
|
||||
printf("\n");
|
||||
printf("========================================\n");
|
||||
if (all_correct) {
|
||||
printf("TEST PASSED\n");
|
||||
printf("All mute switches correctly set to %s\n", expected_state ? "ON" : "OFF");
|
||||
if (!path->mute_during_activation) {
|
||||
printf("\nThis proves the fix works:\n");
|
||||
printf("- Path has mute_during_activation=false (default)\n");
|
||||
printf("- Switches were still set correctly\n");
|
||||
printf("- Bug would have left them in kernel default state\n");
|
||||
}
|
||||
ret = 0;
|
||||
} else {
|
||||
printf("TEST FAILED\n");
|
||||
printf("One or more switches NOT set correctly\n");
|
||||
printf("Expected: %s, but some switches were %s\n",
|
||||
expected_state ? "ON (unmuted)" : "OFF (muted)",
|
||||
expected_state ? "OFF" : "ON");
|
||||
printf("\nThis indicates the bug is still present:\n");
|
||||
printf("- Switches not set when mute_during_activation=false\n");
|
||||
printf("- IEC958 fix may not be applied\n");
|
||||
ret = 1;
|
||||
}
|
||||
printf("========================================\n");
|
||||
|
||||
cleanup:
|
||||
if (mixer)
|
||||
snd_mixer_close(mixer);
|
||||
if (path)
|
||||
pa_alsa_path_free(path);
|
||||
|
||||
return ret;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue