flvmux: Correct breaks in gst_flv_mux_find_best_pad

The code seems to use `continue` and `break` as if both refer to the
surrounding `while` loop. But because `break` breaks out of the
`switch`, they actually have the same effect.

This may have caused the loop not to terminate when it should. E.g. when
`skip_backwards_streams` drops a buffer we should abort the aggregation
and wait for all pads to be filled again. Instead, we might have just
selected a subsequent pad as our new "best".

Replace `break` with `done = TRUE; break`, and `continue` with `break`.
Then simplify the code a bit.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/710>
This commit is contained in:
Jan Alexander Steffens (heftig) 2020-08-31 13:43:42 +02:00
parent e695991508
commit 01594d19b8

View file

@ -1853,8 +1853,8 @@ static GstFlvMuxPad *
gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts, gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
gboolean timeout) gboolean timeout)
{ {
GstFlvMux *mux = GST_FLV_MUX (aggregator);
GstFlvMuxPad *best = NULL; GstFlvMuxPad *best = NULL;
GstBuffer *buffer;
GstClockTime best_ts = GST_CLOCK_TIME_NONE; GstClockTime best_ts = GST_CLOCK_TIME_NONE;
GstIterator *pads; GstIterator *pads;
GValue padptr = { 0, }; GValue padptr = { 0, };
@ -1865,22 +1865,19 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
while (!done) { while (!done) {
switch (gst_iterator_next (pads, &padptr)) { switch (gst_iterator_next (pads, &padptr)) {
case GST_ITERATOR_OK:{ case GST_ITERATOR_OK:{
GstFlvMux *mux = GST_FLV_MUX (aggregator);
GstAggregatorPad *apad = g_value_get_object (&padptr); GstAggregatorPad *apad = g_value_get_object (&padptr);
GstFlvMuxPad *fpad = GST_FLV_MUX_PAD (apad); GstFlvMuxPad *fpad = GST_FLV_MUX_PAD (apad);
gint64 t = GST_CLOCK_TIME_NONE; GstClockTime t = GST_CLOCK_TIME_NONE;
GstBuffer *buffer;
buffer = gst_aggregator_pad_peek_buffer (apad); buffer = gst_aggregator_pad_peek_buffer (apad);
if (!buffer) { if (!buffer) {
if (timeout || GST_PAD_IS_EOS (apad)) { if (!timeout && !GST_PAD_IS_EOS (apad)) {
continue; gst_object_replace ((GstObject **) & best, NULL);
} else {
if (best)
gst_object_replace ((GstObject **) & best, NULL);
best_ts = GST_CLOCK_TIME_NONE; best_ts = GST_CLOCK_TIME_NONE;
break; done = TRUE;
} }
break;
} }
if (fpad->drop_deltas) { if (fpad->drop_deltas) {
@ -1890,14 +1887,12 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
"Dropped buffer %" GST_PTR_FORMAT " until keyframe", buffer); "Dropped buffer %" GST_PTR_FORMAT " until keyframe", buffer);
gst_buffer_unref (buffer); gst_buffer_unref (buffer);
gst_aggregator_pad_drop_buffer (apad); gst_aggregator_pad_drop_buffer (apad);
if (timeout) { if (!timeout) {
continue; gst_object_replace ((GstObject **) & best, NULL);
} else {
if (best)
gst_object_replace ((GstObject **) & best, NULL);
best_ts = GST_CLOCK_TIME_NONE; best_ts = GST_CLOCK_TIME_NONE;
break; done = TRUE;
} }
break;
} else { } else {
/* drop-deltas is set and the buffer isn't delta, drop flag */ /* drop-deltas is set and the buffer isn't delta, drop flag */
fpad->drop_deltas = FALSE; fpad->drop_deltas = FALSE;
@ -1918,14 +1913,12 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
gst_aggregator_pad_drop_buffer (apad); gst_aggregator_pad_drop_buffer (apad);
/* Look for non-delta buffer */ /* Look for non-delta buffer */
fpad->drop_deltas = TRUE; fpad->drop_deltas = TRUE;
if (timeout) { if (!timeout) {
continue; gst_object_replace ((GstObject **) & best, NULL);
} else {
if (best)
gst_object_replace ((GstObject **) & best, NULL);
best_ts = GST_CLOCK_TIME_NONE; best_ts = GST_CLOCK_TIME_NONE;
break; done = TRUE;
} }
break;
} }
} }
@ -1935,7 +1928,6 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
best_ts = t; best_ts = t;
} }
gst_buffer_unref (buffer); gst_buffer_unref (buffer);
g_value_reset (&padptr);
break; break;
} }
case GST_ITERATOR_DONE: case GST_ITERATOR_DONE:
@ -1952,13 +1944,19 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
g_assert_not_reached (); g_assert_not_reached ();
break; break;
} }
g_value_reset (&padptr);
} }
g_value_unset (&padptr); g_value_unset (&padptr);
gst_iterator_free (pads); gst_iterator_free (pads);
GST_DEBUG_OBJECT (aggregator, if (best) {
"Best pad found with %" GST_TIME_FORMAT ": %" GST_PTR_FORMAT, GST_DEBUG_OBJECT (aggregator,
GST_TIME_ARGS (best_ts), best); "Best pad found with TS %" GST_TIME_FORMAT ": %" GST_PTR_FORMAT,
GST_TIME_ARGS (best_ts), best);
} else {
GST_DEBUG_OBJECT (aggregator, "Best pad not found");
}
if (ts) if (ts)
*ts = best_ts; *ts = best_ts;
return best; return best;