GNOME Bugzilla – Bug 690557
Corrected the control of whether a SingleQueue is full or not
Last modified: 2012-12-20 16:27:16 UTC
The code in single_queue_overrun_cb(), which checks whether a SingleQueue is to be considered full or not, is confusing and does not behave as one would expect.
When checking for fullness it should compare with it's own max sizes, not with the other queues in the same MultiQueue. Furthermore checking for the hard limit ought to be done first, before the loop. After all, the queue is considered full if the hard limits are reached, no matter if another SingleQueue is empty.
If the hard limits have not been reached yet then it checks whether any of the other SingleQueues are empty, and if that's the case the allowed buffer limit is increased with one. That should only be done once for each call.
There is already a comment in the code pointing out that these things seems to be wrong.
Created attachment 231979 [details] [review]
Patch for correcting the SingleQueue full callback
Author: Branko Subasic <email@example.com>
Date: Thu Dec 20 13:31:02 2012 +0100
multiqueue: correct overrun handling
The control of wheteher a SingleQueue is full is not correct.
Rewrote single_queue_overrun_cb() so it checks the correct variables
when checking if the queue has reached the hard limits, and to
increase the max buffer limit once for each call.
I haven't found a case where the MultiQueue issue causes a crash or it actually
goes wrong. The problem is that in each call to single_queue_overrun_cb() the
max_size.visible for the full SingleQueue is incremented once per empty
SingleQueue, and the queue size will be retained throughout the session, which
means that an unnecessarily high amount of memory is used.
By just incrementing it once in each call, we give the demuxer a chance to
produce data for the other streams, thereby not having to enlarge the full
This may not be an issue on a PC, but it may be in an embedded system with
limited memory resources and when the hard limit is checked, the values from
one queue is compared to limits from another, which is confusing and makes it
harder to understand and debug
And lastly, the code is now easier to follow imho.