[pulseaudio-discuss] [PATCH 1/3] alsa: fix infinite loop with Intel HDMI LPE
Hui Wang
hui.wang at canonical.com
Tue Mar 26 05:17:37 UTC 2019
On 2017/12/28 下午6:09, Tanu Kaskinen wrote:
> The Intel HDMI LPE driver works in a peculiar way when the HDMI cable is
> not plugged in: any written audio is immediately discarded and underrun
> is reported. That resulted in an infinite loop, because PulseAudio tried
> to keep the buffer filled, which was futile since the written audio was
> immediately consumed/discarded.
>
> This patch adds special handling for the LPE driver: if the active port
> of the sink is unavailable, the sink suspends itself. A new suspend
> cause is added: PA_SUSPEND_UNAVAILABLE.
>
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100488
> ---
> src/modules/alsa/alsa-mixer.h | 1 +
> src/modules/alsa/alsa-sink.c | 22 ++++++++++++++++++++++
> src/modules/alsa/module-alsa-card.c | 34 ++++++++++++++++++++++++++++++++++
> src/pulsecore/core.h | 1 +
> 4 files changed, 58 insertions(+)
>
> diff --git a/src/modules/alsa/alsa-mixer.h b/src/modules/alsa/alsa-mixer.h
> index 4ebf1922b..3577f435f 100644
> --- a/src/modules/alsa/alsa-mixer.h
> +++ b/src/modules/alsa/alsa-mixer.h
> @@ -364,6 +364,7 @@ int pa_alsa_set_mixer_rtpoll(struct pa_alsa_mixer_pdata *pd, snd_mixer_t *mixer,
> struct pa_alsa_port_data {
> pa_alsa_path *path;
> pa_alsa_setting *setting;
> + bool suspend_when_unavailable;
> };
>
> void pa_alsa_add_ports(void *sink_or_source_new_data, pa_alsa_path_set *ps, pa_card *card);
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index 7936cfaca..a80caab2e 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -1527,6 +1527,11 @@ static int sink_set_port_cb(pa_sink *s, pa_device_port *p) {
> s->set_volume(s);
> }
>
> + if (data->suspend_when_unavailable && p->available == PA_AVAILABLE_NO)
> + pa_sink_suspend(s, true, PA_SUSPEND_UNAVAILABLE);
> + else
Hi Tanu,
We tried to backport this patch to pulseaudio-8.0 (for ubuntu linux
16.04), but after applying this patch, all audio jacks (like headphone,
line-in/line-out) can't work anymore. If we change the above line as
below, the problem will disappear.
else (data->suspend_when_unavailable)
In theory, if a port is not set suspend_when_unavailable, the port
should not be changed by this patch. I don't know if it is correct or
not, could you please take a look at it?
Thanks,
Hui.
> + pa_sink_suspend(s, false, PA_SUSPEND_UNAVAILABLE);
> +
> return 0;
> }
>
> @@ -2460,6 +2465,23 @@ pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
> if (profile_set)
> pa_alsa_profile_set_free(profile_set);
>
> + /* Suspend if necessary. FIXME: It would be better to start suspended, but
> + * that would require some core changes. It's possible to set
> + * pa_sink_new_data.suspend_cause, but that has to be done before the
> + * pa_sink_new() call, and we know if we need to suspend only after the
> + * pa_sink_new() call when the initial port has been chosen. Calling
> + * pa_sink_suspend() between pa_sink_new() and pa_sink_put() would
> + * otherwise work, but currently pa_sink_suspend() will crash if
> + * pa_sink_put() hasn't been called. */
> + if (u->sink->active_port) {
> + pa_alsa_port_data *port_data;
> +
> + port_data = PA_DEVICE_PORT_DATA(u->sink->active_port);
> +
> + if (port_data->suspend_when_unavailable && u->sink->active_port->available == PA_AVAILABLE_NO)
> + pa_sink_suspend(u->sink, true, PA_SUSPEND_UNAVAILABLE);
> + }
> +
> return u->sink;
>
> fail:
> diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
> index 804b4f872..b193d40cc 100644
> --- a/src/modules/alsa/module-alsa-card.c
> +++ b/src/modules/alsa/module-alsa-card.c
> @@ -426,6 +426,22 @@ static int report_jack_state(snd_mixer_elem_t *melem, unsigned int mask) {
> if (tp->avail == PA_AVAILABLE_NO)
> pa_device_port_set_available(tp->port, tp->avail);
>
> + for (tp = tports; tp->port; tp++) {
> + pa_alsa_port_data *data;
> + pa_sink *sink;
> + uint32_t idx;
> +
> + data = PA_DEVICE_PORT_DATA(tp->port);
> +
> + if (!data->suspend_when_unavailable)
> + continue;
> +
> + PA_IDXSET_FOREACH(sink, u->core->sinks, idx) {
> + if (sink->active_port == tp->port)
> + pa_sink_suspend(sink, tp->avail == PA_AVAILABLE_NO, PA_SUSPEND_UNAVAILABLE);
> + }
> + }
> +
> /* Update profile availabilities. The logic could be improved; for now we
> * only set obviously unavailable profiles (those that contain only
> * unavailable ports) to PA_AVAILABLE_NO and all others to
> @@ -836,6 +852,24 @@ int pa__init(pa_module *m) {
> goto fail;
> }
>
> + /* The Intel HDMI LPE driver needs some special handling. When the HDMI
> + * cable is not plugged in, trying to play audio doesn't work. Any written
> + * audio is immediately discarded and an underrun is reported, and that
> + * results in an infinite loop of "fill buffer, handle underrun". To work
> + * around this issue, the suspend_when_unavailable flag is used to stop
> + * playback when the HDMI cable is unplugged. */
> + if (pa_safe_streq(pa_proplist_gets(data.proplist, "alsa.driver_name"), "snd_hdmi_lpe_audio")) {
> + pa_device_port *port;
> + void *state;
> +
> + PA_HASHMAP_FOREACH(port, data.ports, state) {
> + pa_alsa_port_data *port_data;
> +
> + port_data = PA_DEVICE_PORT_DATA(port);
> + port_data->suspend_when_unavailable = true;
> + }
> + }
> +
> u->card = pa_card_new(m->core, &data);
> pa_card_new_data_done(&data);
>
> diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
> index 79a095d24..afe6c25ed 100644
> --- a/src/pulsecore/core.h
> +++ b/src/pulsecore/core.h
> @@ -34,6 +34,7 @@ typedef enum pa_suspend_cause {
> PA_SUSPEND_SESSION = 8, /* Used by module-hal for mark inactive sessions */
> PA_SUSPEND_PASSTHROUGH = 16, /* Used to suspend monitor sources when the sink is in passthrough mode */
> PA_SUSPEND_INTERNAL = 32, /* This is used for short period server-internal suspends, such as for sample rate updates */
> + PA_SUSPEND_UNAVAILABLE = 64, /* Used by device implementations that have to suspend when the device is unavailable */
> PA_SUSPEND_ALL = 0xFFFF /* Magic cause that can be used to resume forcibly */
> } pa_suspend_cause_t;
>
More information about the pulseaudio-discuss
mailing list