queue2: don't change global buffering state from within query handler

When a buffering query is handled it uses the get_buffering_percent()
function to get some statitics. Unfortunately this function also
calculates whether the queue should be buffering and adapts the
global queue2 state in case of state transitions from/to buffering
(including whether a buffering message was posted on the bus!).

This means that there is a race which can cause buffering messages
to never posted if the global state changes happen as a result of aa
query instead of resulting from bytes flowing in/out.

Spotted by Sjoerd Simons.

Change to only query state in get_buffering_percent() and update
state only in update_buffering().

https://bugzilla.gnome.org/show_bug.cgi?id=705332
This commit is contained in:
Tim-Philipp Müller 2013-08-16 16:28:12 +01:00
parent d7a98f9802
commit c02081ca30

View file

@ -783,7 +783,6 @@ static gboolean
get_buffering_percent (GstQueue2 * queue, gboolean * is_buffering, get_buffering_percent (GstQueue2 * queue, gboolean * is_buffering,
gint * percent) gint * percent)
{ {
gboolean post = FALSE;
gint perc; gint perc;
if (queue->high_percent <= 0) { if (queue->high_percent <= 0) {
@ -817,21 +816,6 @@ get_buffering_percent (GstQueue2 * queue, gboolean * is_buffering,
} }
#undef GET_PERCENT #undef GET_PERCENT
if (queue->is_buffering) {
post = TRUE;
/* if we were buffering see if we reached the high watermark */
if (perc >= queue->high_percent)
queue->is_buffering = FALSE;
} else {
/* we were not buffering, check if we need to start buffering if we drop
* below the low threshold */
if (perc < queue->low_percent) {
queue->is_buffering = TRUE;
queue->buffering_iteration++;
post = TRUE;
}
}
if (is_buffering) if (is_buffering)
*is_buffering = queue->is_buffering; *is_buffering = queue->is_buffering;
@ -841,19 +825,13 @@ get_buffering_percent (GstQueue2 * queue, gboolean * is_buffering,
if (perc > 100) if (perc > 100)
perc = 100; perc = 100;
if (post) {
if (perc == queue->buffering_percent)
post = FALSE;
else
queue->buffering_percent = perc;
}
if (percent) if (percent)
*percent = perc; *percent = perc;
GST_DEBUG_OBJECT (queue, "buffering %d, percent %d", queue->is_buffering, GST_DEBUG_OBJECT (queue, "buffering %d, percent %d", queue->is_buffering,
perc); perc);
return post; return TRUE;
} }
static void static void
@ -897,7 +875,30 @@ update_buffering (GstQueue2 * queue)
gint percent; gint percent;
gboolean post = FALSE; gboolean post = FALSE;
post = get_buffering_percent (queue, NULL, &percent); if (!get_buffering_percent (queue, NULL, &percent))
return;
if (queue->is_buffering) {
post = TRUE;
/* if we were buffering see if we reached the high watermark */
if (percent >= queue->high_percent)
queue->is_buffering = FALSE;
} else {
/* we were not buffering, check if we need to start buffering if we drop
* below the low threshold */
if (percent < queue->low_percent) {
queue->is_buffering = TRUE;
queue->buffering_iteration++;
post = TRUE;
}
}
if (post) {
if (percent == queue->buffering_percent)
post = FALSE;
else
queue->buffering_percent = percent;
}
if (post) { if (post) {
GstMessage *message; GstMessage *message;