[pulseaudio-discuss] [RFC][PATCH] improve missing handling in memblockq (was Re: stream wedged in non-playing state)
arun at arunraghavan.net
Tue May 31 10:05:26 UTC 2016
On Thu, 19 May 2016, at 08:02 PM, Pierre Ossman wrote:
> On Wed, 18 May 2016 15:40:55 +0200
> Pierre Ossman <ossman at cendio.se> wrote:
> > I suspect the bug is in the memblockq code somewhere. It seems like it
> > wedges itself somehow. The "missing" state variable is something I'm
> > looking at with suspicion.
> This wasn't the problem. The trace revealed that we do not handle
> changes to minreq properly. The scenario is:
> 1. Large latency, large buffer, large target fill, large minimum
> request. Silence in queue (i.e. buffer is full).
> 2. Buffer drains slightly, making it fall below target fill. It is
> however still below the minimum request, so nothing is sent to the
> 3. The client requests a reduced latency, buffer is reduced, target
> fill is reduced, minimum request is reduced. The buffer now greatly
> exceeds target fill as it was almost up to the previous target fill
> level. This means that the server will not be asking the client for
> more data for a while.
> 4. Some time later we've drained most of the excess and are almost
> back down to the target fill level. However the data requested in 2 is
> sufficiently large that we never fall back down below target fill.
> Hence we never start requesting for more data. And we already decided
> in 2 not to send a request for the first portion.
> I've attached a patch that moves the minreq handling in to memblockq,
> which should avoid this and any similar issues as there is now a single
> place where things are throttled for minreq.
> This will affect all callers of pa_memblockq_pop_missing() and perturb
> things a bit. I can't see it being fundamentally worse or better
> though, just different. Besides fixing the bugs of course.
Has this been tested with different prebuf values in buffer_attr? That
seems to be one change that might affect other callers. Ideally we
should also just add a test to the check-daemon suite to cover this and
maybe a long-running case for the bug you found.
> > Why do we even have that? Can't we derive that from the other
> > variables? Having redundant state is just asking for things to get out
> > of sync and for bugs to appear.
> Patch added for this as well. The memblockq code makes my head spin,
> and killing of "missing" and santising "requested" makes it a bit
> easier to comprehend.
> Please review as I'd really like this fix to start trickling out to the
> distributions sooner rather than later.
I've not been able to look through this, will try to do that a bit
More information about the pulseaudio-discuss