From 01594d19b8dfac38226910d1b10231c26133c7f9 Mon Sep 17 00:00:00 2001
From: "Jan Alexander Steffens (heftig)" <jan.steffens@ltnglobal.com>
Date: Mon, 31 Aug 2020 13:43:42 +0200
Subject: [PATCH] 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>
---
 gst/flv/gstflvmux.c | 50 ++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/gst/flv/gstflvmux.c b/gst/flv/gstflvmux.c
index 71ecd7aeb0..8b9e732838 100644
--- a/gst/flv/gstflvmux.c
+++ b/gst/flv/gstflvmux.c
@@ -1853,8 +1853,8 @@ static GstFlvMuxPad *
 gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
     gboolean timeout)
 {
+  GstFlvMux *mux = GST_FLV_MUX (aggregator);
   GstFlvMuxPad *best = NULL;
-  GstBuffer *buffer;
   GstClockTime best_ts = GST_CLOCK_TIME_NONE;
   GstIterator *pads;
   GValue padptr = { 0, };
@@ -1865,22 +1865,19 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
   while (!done) {
     switch (gst_iterator_next (pads, &padptr)) {
       case GST_ITERATOR_OK:{
-
-        GstFlvMux *mux = GST_FLV_MUX (aggregator);
         GstAggregatorPad *apad = g_value_get_object (&padptr);
         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);
         if (!buffer) {
-          if (timeout || GST_PAD_IS_EOS (apad)) {
-            continue;
-          } else {
-            if (best)
-              gst_object_replace ((GstObject **) & best, NULL);
+          if (!timeout && !GST_PAD_IS_EOS (apad)) {
+            gst_object_replace ((GstObject **) & best, NULL);
             best_ts = GST_CLOCK_TIME_NONE;
-            break;
+            done = TRUE;
           }
+          break;
         }
 
         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);
             gst_buffer_unref (buffer);
             gst_aggregator_pad_drop_buffer (apad);
-            if (timeout) {
-              continue;
-            } else {
-              if (best)
-                gst_object_replace ((GstObject **) & best, NULL);
+            if (!timeout) {
+              gst_object_replace ((GstObject **) & best, NULL);
               best_ts = GST_CLOCK_TIME_NONE;
-              break;
+              done = TRUE;
             }
+            break;
           } else {
             /* drop-deltas is set and the buffer isn't delta, drop flag */
             fpad->drop_deltas = FALSE;
@@ -1918,14 +1913,12 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
             gst_aggregator_pad_drop_buffer (apad);
             /* Look for non-delta buffer */
             fpad->drop_deltas = TRUE;
-            if (timeout) {
-              continue;
-            } else {
-              if (best)
-                gst_object_replace ((GstObject **) & best, NULL);
+            if (!timeout) {
+              gst_object_replace ((GstObject **) & best, NULL);
               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;
         }
         gst_buffer_unref (buffer);
-        g_value_reset (&padptr);
         break;
       }
       case GST_ITERATOR_DONE:
@@ -1952,13 +1944,19 @@ gst_flv_mux_find_best_pad (GstAggregator * aggregator, GstClockTime * ts,
         g_assert_not_reached ();
         break;
     }
+    g_value_reset (&padptr);
   }
   g_value_unset (&padptr);
   gst_iterator_free (pads);
 
-  GST_DEBUG_OBJECT (aggregator,
-      "Best pad found with %" GST_TIME_FORMAT ": %" GST_PTR_FORMAT,
-      GST_TIME_ARGS (best_ts), best);
+  if (best) {
+    GST_DEBUG_OBJECT (aggregator,
+        "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)
     *ts = best_ts;
   return best;