[pulseaudio-discuss] [PATCH 2/2] memblockq-test: fix incorrect assumption of pa_memblockq_pop_missing() behaviour

[Pierre Ossman]

On 31/12/16 17:26, Ahmed S. Darwish wrote:
>On Fri, Dec 30, 2016 at 05:52:36PM +0200, Tanu Kaskinen wrote:
>> diff --git a/src/tests/memblockq-test.c b/src/tests/memblockq-test.c
>> index fc83d99..2a9b88a 100644
>> --- a/src/tests/memblockq-test.c
>> +++ b/src/tests/memblockq-test.c
>> @@ -421,74 +421,129 @@ START_TEST (memblockq_test_pop_missing) {
>>      bq = pa_memblockq_new("test memblockq", idx, maxlength, tlength, &ss, prebuf, minreq, maxrewind, &silence);
>>      fail_unless(bq != NULL);
>>
>> -    /* initially, the whole target length of bytes is missing */
>> +    /* The following equation regarding the internal variables of a memblockq
>> +     * is always true:
>> +     *
>> +     *   length + missing + requested = tlength
>> +     *
>> +     * "length" is the current memblockq length (write index minus read index)
>> +     * and "tlength" is the target length. The intuitive meaning of "missing"
>> +     * would be the difference between tlength and length, but actually
>> +     * "missing" and "requested" together constitute the amount that is missing
>> +     * from the queue. Writing to the queue decrements "requested" and reading
>> +     * from the queue increments "missing". pa_memblockq_pop_missing() resets
>> +     * "missing" to zero, returns the old "missing" value and adds the
>> +     * equivalent amount to "requested".
>> +     *
>> +     * This test has comments between each step documenting the assumed state
>> +     * of those internal variables. */
>> +
>

How about a rename to fit their use instead? I find negative requested 
to be absolutely horrible.

> What really bothers me is that memblockq.requested is actually
> referenced in just _one_ place: a MEMBLOCKQ_DEBUG printf()! :-(
>

Great, so that would nuke it. And it means that we only have "missing" 
to deal with and the fact that there can be more "missing" than there is 
room in the buffer.

How about calling it "consumed"? Because that seems to be what it is, 
the amount of data consumed from the buffer. And in that case it is more 
natural that it might exceed available space if you fail to pop it 
regularly.

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

That sounds like a bug to be honest. Why can't we wait until the buffer 
drains a bit?

Regards
-- 
Pierre Ossman           Software Development
Cendio AB               https://cendio.com
Teknikringen 8          https://twitter.com/ThinLinc
583 30 Linköping        https://facebook.com/ThinLinc
Phone: +46-13-214600    https://plus.google.com/+CendioThinLinc

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?