On Tue, 2016-12-27 at 11:25 +0100, Pierre Ossman wrote:
> On 26/12/16 06:31, Ahmed S. Darwish wrote:
> >
> > But bq->requested has different semantics upon write index change
> > before and after the same commit:
> >
>
> More cleanup and comments of that code is probably needed to make it
> manageable by mere mortals. :)
Yes, this code is painful...
> > After the cleanup commit:
> >
> > static void write_index_changed(pa_memblockq *bq, int64_t old_write_index,
> > bool account) {
> > int64_t delta;
> >
> > delta = bq->write_index - old_write_index;
> >
> > if (account) {
> > if (delta > (int64_t)bq->requested)
> > bq->requested = 0; <== Here is the trigger; bq 'requested'
> > <== only adjusted in the positive case
> > else if (delta > 0)
> > bq->requested -= delta;
> > }
> > ...
> > }
>
> Allow bq->requested to be negative is a bad idea IMO as it conceptually
> makes little to no sense.
I started to investigate this bug myself today. I didn't get so far
that I'd understand what the "requested"Â variable is supposed to
represent. I'll try to figure that out tomorrow, but feel free to
explain it to me before that if you already have an explanation.
"missing" seems to mean, at least in the current code that doesn't
work, the amount of data that the client needs to send in order to
reach tlength. That seems logical. Currently the server sends REQUEST
commands only when "missing" becomes positive, however, which doesn't
work when the buffer is filled beyond tlength, because the client
expects REQUEST commands also during the time when the server already
has enough data.
Maybe the REQUEST commands shouldn't be so directly based on the
"missing" variable?
> Is it sufficient to remove the second if? What would that mean though?
> An alternative way of increasing the requested size? Why is this
> happening? What's the call chain?
This happens, because the client sends a chunk of data that makes the
buffered amount larger than tlength. If "requested" means the amount
that we've requested from the client, and if it is decremented when the
client writes more data, then the extra data has to be accounted
somehow. One way to do that is to make "requested" negative, and
interpret a negative "requested" value as "the client has sent this
much more data than we have requested it to send".
--
Tanu
https://www.patreon.com/tanuk
