[pulseaudio-discuss] [PATCH 1/3] alsa: fix infinite loop with Intel HDMI LPE

[Tanu Kaskinen]

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
+        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;
 
-- 
2.14.2