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

2019-03-26 Thread Hui Wang
On 2019/3/27 上午1:44, Tanu Kaskinen wrote:
> On Tue, 2019-03-26 at 13:17 +0800, Hui Wang wrote:
>> 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?
> I don't think your change is correct. This code is run when the sink
> changes its port, so there are two ports involved: the old port and the
> new one. If the old port caused the sink to be suspended with the
> UNAVAILABLE cause, and the new port doesn't require suspending when
> it's unavailable, then with your change the sink doesn't get
> unsuspended when it should.
>
> In practice your change should be harmless, though, because all ports
> on a Intel HDMI LPE card will have the suspend_when_unavailable flag
> set, and on other cards the flag is never set.
>
> You didn't specify what the exact problem with headphones etc. is. Is
> there an assertion error? I would guess that you're running into this
> bug that was introduced by the HDMI LPE fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=104761
>
> Fixing that bug involved rewriting much of the device suspending and
> state changing code. I don't know if you want to backport those patches
> if your small change seems to make things work well enough, but here's
> a list of relevant commits (oldest first, I'm not 100% that this is a
> complete list):
>
> 3da0de5418b29c90974d0d3e2198c471c39d229f
> 6ed37aeef28708f8da34a16c7035fa1331fa13cb
> d6e39b5e891c767dd42c369d9f118560b8bb24ae
> 7f201b1fd419b91a226d23ee1e216661ae082dcf
> 2dff0d6a6a4df2aab6f36212b705489d5af42835
> 7f09164ed7979210adcdb7674b9d6217fd44ed66
> f6fe411b32c0cf5932fb4f169f5288c76bc6923d
> 0fad369ceb18a8e275e8f74f10f784e0d7476dfb
> 73b8a57078b94033edf84de2fc0cfbe344c10dcd
> b2537a8f38ad71e4dee57263310235abdf2b95a4
> ad0616d4c91de52b7cb69e6222efe96961755482
> ad15e6e50e737fb55a87bb7def22332f774abce9

Ok, got it. I will investigate these commits.

Thanks,

Hui.



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

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

2019-03-26 Thread Tanu Kaskinen
On Tue, 2019-03-26 at 13:17 +0800, Hui Wang wrote:
> 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?

I don't think your change is correct. This code is run when the sink
changes its port, so there are two ports involved: the old port and the
new one. If the old port caused the sink to be suspended with the
UNAVAILABLE cause, and the new port doesn't require suspending when
it's unavailable, then with your change the sink doesn't get
unsuspended when it should.

In practice your change should be harmless, though, because all ports
on a Intel HDMI LPE card will have the suspend_when_unavailable flag
set, and on other cards the flag is never set.

You didn't specify what the exact problem with headphones etc. is. Is
there an assertion error? I would guess that you're running into this
bug that was introduced by the HDMI LPE fixes:
https://bugs.freedesktop.org/show_bug.cgi?id=104761

Fixing that bug involved rewriting much of the device suspending and
state changing code. I don't know if you want to backport those patches
if your small change seems to make things work well enough, but here's
a list of relevant commits (oldest first, I'm not 100% that this is a
complete list):

3da0de5418b29c90974d0d3e2198c471c39d229f
6ed37aeef28708f8da34a16c7035fa1331fa13cb
d6e39b5e891c767dd42c369d9f118560b8bb24ae
7f201b1fd419b91a226d23ee1e216661ae082dcf
2dff0d6a6a4df2aab6f36212b705489d5af42835
7f09164ed7979210adcdb7674b9d6217fd44ed66
f6fe411b32c0cf5932fb4f169f5288c76bc6923d
0fad369ceb18a8e275e8f74f10f784e0d7476dfb
73b8a57078b94033edf84de2fc0cfbe344c10dcd
b2537a8f38ad71e4dee57263310235abdf2b95a4
ad0616d4c91de52b7cb69e6222efe96961755482
ad15e6e50e737fb55a87bb7def22332f774abce9

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

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

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

2019-03-25 Thread Hui Wang
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, tryi

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

2018-01-03 Thread Tanu Kaskinen
On Sat, 2017-12-30 at 13:23 +0100, David Henningsson wrote:
> 
> On 2017-12-30 13:03, Georg Chini wrote:
> > On 30.12.2017 10:54, Alexander E. Patrakov wrote:
> > > 2017-12-29 20:37 GMT+08:00 Tanu Kaskinen :
> > > > On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:
> > > > > 2017-12-28 18:09 GMT+08:00 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.
> > > > > 
> > > > > I think this is not a complete fix. There was a case in the past where
> > > > > some other card started eating samples too quickly (some Radeon?
> > > > > unfortunately, can't find it now). While blacklisting one known bad
> > > > > driver helps, I think it would be better to detect the misbehavior at
> > > > > runtime, based on the number of samples written and the wall-clock
> > > > > time. If the card stalls or eats samples too quickly, set a flag that
> > > > > it misbehaves, accept the xrun, and then set it to off when
> > > > > convenient.
> > > > > 
> > > > > Sorry, I have no time to help with the code :(
> > > > 
> > > > Did that other sample-eating sound card exhibit the behaviour only when
> > > > unplugged? With the LPE driver we know that we can resume once the
> > > > cable is plugged in again. If we don't take the jack state into
> > > > consideration, then we don't know when we should try resuming.
> > > 
> > > Yes, it was only when unplugged. The thread (found it!) starts here:
> > > 
> > > http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081365.html
> > >  
> > > 
> > > 
> > > David: do you know whether it has been fixed in the kernel in that case?
> > > 
> > > > I can change the patch so that the sink is suspended when both of the
> > > > conditions are fulfilled: too fast sample consumption and jack
> > > > unplugged.
> > > 
> > > I think an "or" condition would be more appropriate here, for
> > > robustness. The real bug here is the CPU overuse (in the worst case,
> > > infinite loop) due to too-fast sample consumption by a misbehaving
> > > soundcard driver (but we accept that this misbehavior happens and we
> > > have to deal with it). The fact that the cable is unplugged is just
> > > one known trigger.
> > 
> > If the sample rate significantly deviates from the nominal value when
> > the jack is plugged, would that not mean that the card (or the driver)
> > is broken and the card is therefore unusable?
> > I think the only situation where the misbehavior is acceptable is when
> > the jack is unplugged, so force-suspending the sink on unplug seems
> > sufficient to me.
> 
> Btw, if there are buffers that need to be filled up beyond our own ALSA 
> buffer, it would be possible that the card fills those buffers first, 
> which might look to us like a "too high sample rate".
> There is already a workaround for this problem in alsa-sink.c (see 
> comment "USB devices on ALSA seem to hit a buffer
> underrun during the first iterations much quicker then we calculate 
> here, probably due to the transport latency.").

Since adding automatic "misbehaviour" detection seems complex and prone
to false positives, I prefer to apply my original patch that is
arguably simpler but more limited. Georg acked the patch, so I pushed
it now.

-- 
Tanu

https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


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

2017-12-30 Thread David Henningsson



On 2017-12-30 13:03, Georg Chini wrote:

On 30.12.2017 10:54, Alexander E. Patrakov wrote:

2017-12-29 20:37 GMT+08:00 Tanu Kaskinen :

On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:

2017-12-28 18:09 GMT+08:00 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.

I think this is not a complete fix. There was a case in the past where
some other card started eating samples too quickly (some Radeon?
unfortunately, can't find it now). While blacklisting one known bad
driver helps, I think it would be better to detect the misbehavior at
runtime, based on the number of samples written and the wall-clock
time. If the card stalls or eats samples too quickly, set a flag that
it misbehaves, accept the xrun, and then set it to off when
convenient.

Sorry, I have no time to help with the code :(

Did that other sample-eating sound card exhibit the behaviour only when
unplugged? With the LPE driver we know that we can resume once the
cable is plugged in again. If we don't take the jack state into
consideration, then we don't know when we should try resuming.

Yes, it was only when unplugged. The thread (found it!) starts here:

http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081365.html 



David: do you know whether it has been fixed in the kernel in that case?


I can change the patch so that the sink is suspended when both of the
conditions are fulfilled: too fast sample consumption and jack
unplugged.

I think an "or" condition would be more appropriate here, for
robustness. The real bug here is the CPU overuse (in the worst case,
infinite loop) due to too-fast sample consumption by a misbehaving
soundcard driver (but we accept that this misbehavior happens and we
have to deal with it). The fact that the cable is unplugged is just
one known trigger.


If the sample rate significantly deviates from the nominal value when
the jack is plugged, would that not mean that the card (or the driver)
is broken and the card is therefore unusable?
I think the only situation where the misbehavior is acceptable is when
the jack is unplugged, so force-suspending the sink on unplug seems
sufficient to me.


Btw, if there are buffers that need to be filled up beyond our own ALSA 
buffer, it would be possible that the card fills those buffers first, 
which might look to us like a "too high sample rate".
There is already a workaround for this problem in alsa-sink.c (see 
comment "USB devices on ALSA seem to hit a buffer
underrun during the first iterations much quicker then we calculate 
here, probably due to the transport latency.").


// David
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


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

2017-12-30 Thread Georg Chini

On 30.12.2017 10:54, Alexander E. Patrakov wrote:

2017-12-29 20:37 GMT+08:00 Tanu Kaskinen :

On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:

2017-12-28 18:09 GMT+08:00 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.

I think this is not a complete fix. There was a case in the past where
some other card started eating samples too quickly (some Radeon?
unfortunately, can't find it now). While blacklisting one known bad
driver helps, I think it would be better to detect the misbehavior at
runtime, based on the number of samples written and the wall-clock
time. If the card stalls or eats samples too quickly, set a flag that
it misbehaves, accept the xrun, and then set it to off when
convenient.

Sorry, I have no time to help with the code :(

Did that other sample-eating sound card exhibit the behaviour only when
unplugged? With the LPE driver we know that we can resume once the
cable is plugged in again. If we don't take the jack state into
consideration, then we don't know when we should try resuming.

Yes, it was only when unplugged. The thread (found it!) starts here:

http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081365.html

David: do you know whether it has been fixed in the kernel in that case?


I can change the patch so that the sink is suspended when both of the
conditions are fulfilled: too fast sample consumption and jack
unplugged.

I think an "or" condition would be more appropriate here, for
robustness. The real bug here is the CPU overuse (in the worst case,
infinite loop) due to too-fast sample consumption by a misbehaving
soundcard driver (but we accept that this misbehavior happens and we
have to deal with it). The fact that the cable is unplugged is just
one known trigger.


If the sample rate significantly deviates from the nominal value when
the jack is plugged, would that not mean that the card (or the driver)
is broken and the card is therefore unusable?
I think the only situation where the misbehavior is acceptable is when
the jack is unplugged, so force-suspending the sink on unplug seems
sufficient to me.


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


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

2017-12-30 Thread David Henningsson



On 2017-12-30 10:54, Alexander E. Patrakov wrote:

2017-12-29 20:37 GMT+08:00 Tanu Kaskinen :

On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:

2017-12-28 18:09 GMT+08:00 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.

I think this is not a complete fix. There was a case in the past where
some other card started eating samples too quickly (some Radeon?
unfortunately, can't find it now). While blacklisting one known bad
driver helps, I think it would be better to detect the misbehavior at
runtime, based on the number of samples written and the wall-clock
time. If the card stalls or eats samples too quickly, set a flag that
it misbehaves, accept the xrun, and then set it to off when
convenient.

Sorry, I have no time to help with the code :(

Did that other sample-eating sound card exhibit the behaviour only when
unplugged? With the LPE driver we know that we can resume once the
cable is plugged in again. If we don't take the jack state into
consideration, then we don't know when we should try resuming.

Yes, it was only when unplugged. The thread (found it!) starts here:

http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081365.html

David: do you know whether it has been fixed in the kernel in that case?


No idea. And I've switched graphics card since, so I can't test either.

// David
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


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

2017-12-30 Thread Alexander E. Patrakov
2017-12-29 20:37 GMT+08:00 Tanu Kaskinen :
> On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:
>> 2017-12-28 18:09 GMT+08:00 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.
>>
>> I think this is not a complete fix. There was a case in the past where
>> some other card started eating samples too quickly (some Radeon?
>> unfortunately, can't find it now). While blacklisting one known bad
>> driver helps, I think it would be better to detect the misbehavior at
>> runtime, based on the number of samples written and the wall-clock
>> time. If the card stalls or eats samples too quickly, set a flag that
>> it misbehaves, accept the xrun, and then set it to off when
>> convenient.
>>
>> Sorry, I have no time to help with the code :(
>
> Did that other sample-eating sound card exhibit the behaviour only when
> unplugged? With the LPE driver we know that we can resume once the
> cable is plugged in again. If we don't take the jack state into
> consideration, then we don't know when we should try resuming.

Yes, it was only when unplugged. The thread (found it!) starts here:

http://mailman.alsa-project.org/pipermail/alsa-devel/2014-September/081365.html

David: do you know whether it has been fixed in the kernel in that case?

> I can change the patch so that the sink is suspended when both of the
> conditions are fulfilled: too fast sample consumption and jack
> unplugged.

I think an "or" condition would be more appropriate here, for
robustness. The real bug here is the CPU overuse (in the worst case,
infinite loop) due to too-fast sample consumption by a misbehaving
soundcard driver (but we accept that this misbehavior happens and we
have to deal with it). The fact that the cable is unplugged is just
one known trigger.

But, it's up to you to decide.

-- 
Alexander E. Patrakov
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


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

2017-12-29 Thread Tanu Kaskinen
On Fri, 2017-12-29 at 11:46 +0800, Alexander E. Patrakov wrote:
> 2017-12-28 18:09 GMT+08:00 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.
> 
> I think this is not a complete fix. There was a case in the past where
> some other card started eating samples too quickly (some Radeon?
> unfortunately, can't find it now). While blacklisting one known bad
> driver helps, I think it would be better to detect the misbehavior at
> runtime, based on the number of samples written and the wall-clock
> time. If the card stalls or eats samples too quickly, set a flag that
> it misbehaves, accept the xrun, and then set it to off when
> convenient.
> 
> Sorry, I have no time to help with the code :(

Did that other sample-eating sound card exhibit the behaviour only when
unplugged? With the LPE driver we know that we can resume once the
cable is plugged in again. If we don't take the jack state into
consideration, then we don't know when we should try resuming.

I can change the patch so that the sink is suspended when both of the
conditions are fulfilled: too fast sample consumption and jack
unplugged.

-- 
Tanu

https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss


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

2017-12-28 Thread Alexander E. Patrakov
2017-12-28 18:09 GMT+08:00 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.

I think this is not a complete fix. There was a case in the past where
some other card started eating samples too quickly (some Radeon?
unfortunately, can't find it now). While blacklisting one known bad
driver helps, I think it would be better to detect the misbehavior at
runtime, based on the number of samples written and the wall-clock
time. If the card stalls or eats samples too quickly, set a flag that
it misbehaves, accept the xrun, and then set it to off when
convenient.

Sorry, I have no time to help with the code :(

-- 
Alexander E. Patrakov
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss