Created attachment 100829 [details] [review]
gstqueue2.c.rate_update.patch

For clarification, the mentioned increase in cur_level.rate_time when queue2 is full, comes from the way rate_time is calculated:

queue->cur_level.rate_time =
    queue->cur_level.bytes / queue->byte_in_rate * GST_SECOND;

To further clarify this issue, here are some steps how this problem might occur:

1. queue2 remains full for a while during playback. (The reason could be e.g. badly demuxed file causes demuxer to block for a while or something else)

2. while the queue2 is full, no data flows in but time elapses -> byte_in_rate drops

3. when byte_in_rate drops, cur_level.rate_time increases, because of the way it is calculated

4. next when data starts flowing out again, cur_level.rate_time might have increased well over max_level.rate_time and so queue2 still refuses to accept more data just because it was full for a while. And because no data is accepted byte_in_rate keeps dropping further. Recovery from this will happen when enough bytes have flown out of queue2.

Another issue is that if byte_in_rate/byte_out_rate are to be used to estimate network bandwidth/stream byterate, then we could exclude these special cases where data is not flowing through the queue "normally" i.e. when queue2 is full (no data flowing in) and while buffering and paused (no data flowing out). Because during these special cases the in/out rates don't actually reflect bandwidth/stream byterate. Using average values makes these problems less significant, but I think the accuracy of the avg values could be improved this way.

It seems that we should not update the timer when we are blocking in the queue because it is filled. This will make sure we only measure the real input rate without the time we spend on blocking.

(In reply to comment #4)
> It seems that we should not update the timer when we are blocking in the queue
> because it is filled. This will make sure we only measure the real input rate
> without the time we spend on blocking.
> 

Hey, that's exactly what I was going to say. Conversely, for output rate, it's the elapsed time minus the time spent blocking for buffers to be available.

Ok, that part I understood. Now for the next part...

        * gst/playback/gstqueue2.c: (gst_queue_chain):
        Pause the timer to measure the input rate when we block because the
        queue is filled. See #503262.

Okay, that should fix the problem with the byte_in_rate. Although, pausing the timer prevents also updating byte_out_rate which we actually should do despite queue is full. On the other hand now there should not be much data (in bytes) flowing out of the queue while full. Unless someone e.g. halves max_level.bytes during playback in which case byte_out_rate measuring will probably get a bit screwed.

Ok, having separate timers makes sense to me. Now whatfor and how would we actually use the output_rate :)

        * gst/playback/gstqueue2.c: (gst_queue_init), (gst_queue_finalize),
        (reset_rate_timer), (update_in_rates), (update_out_rates),
        (gst_queue_locked_enqueue), (gst_queue_locked_dequeue),
        (gst_queue_chain), (gst_queue_loop):
        Use separate timers for input and output rates.
        Pause measuring the output rate when we block for more data.
        See #503262.

Good question :) maybe it could be used to size the queue2 "better" when playing high bitrate content over low bandwidth connection. Or should this kind of scenario be supported at all, should we just error out?

The attached patch seems to be obsolete now, right? Also, this bug itself seems to be fixed since December 2007 or is there still anything to do?

Closing this bug report as no further information has been provided. Please feel free to reopen this bug if you can provide the information asked for.
Thanks!