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
commit 17bff49262489950e40f6775e9ba0d2b574537bf Author: Branko Subasic <branko@axis.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. https://bugzilla.gnome.org/show_bug.cgi?id=690557
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 queue more. 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.