Re: [PATCH 1/3] alsa: fix infinite loop with Intel HDMI LPE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss




[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux