videodecoder: Re-work reverse playback handling

Move processing of the gather list into the flush_parse function.

Add a last ditch attempt to apply timestamps to outgoing buffers
when walking backwards through decoded frames. Requires that each
gathered region has at least one timestamp.

Make sure to remove decoded packets from the decode list when
they are sent - otherwise the list just grows on each cycle, with
more and more frames being decoded and then clipped away.

Break out of the processing loop early on a bad flow return to make
seeking more responsive.

Use the gst_video_decoder_clip_and_push_buf function in reverse
mode, instead of pushing all buffers arbitrarily.

A couple of small efficiency gains in the list handling, by moving
list elements directly and not reallocating, and by reversing
and concatenating the gather list instead of moving it one node
at a time.

Rename the gst_video_decoder_do_finish_frame function to
gst_video_decoder_release_frame.

gst-libs/gst/video/gstvideodecoder.c

@@ -420,6 +420,8 @@ static GstFlowReturn gst_video_decoder_decode_frame (GstVideoDecoder * decoder,
     GstVideoCodecFrame * frame);
 static gboolean gst_video_decoder_set_src_caps (GstVideoDecoder * decoder);
 
+static void gst_video_decoder_release_frame (GstVideoDecoder * dec,
+    GstVideoCodecFrame * frame);
 static guint64
 gst_video_decoder_get_timestamp (GstVideoDecoder * decoder, int picture_number);
 static guint64 gst_video_decoder_get_frame_duration (GstVideoDecoder * decoder,
@@ -428,6 +430,8 @@ static GstVideoCodecFrame *gst_video_decoder_new_frame (GstVideoDecoder *
     decoder);
 static GstFlowReturn gst_video_decoder_clip_and_push_buf (GstVideoDecoder *
     decoder, GstBuffer * buf);
+static GstFlowReturn gst_video_decoder_flush_parse (GstVideoDecoder * dec);
+
 static void gst_video_decoder_clear_queues (GstVideoDecoder * dec);
 
 static gboolean gst_video_decoder_sink_event_default (GstVideoDecoder * decoder,
@@ -1617,13 +1621,12 @@ gst_video_decoder_flush_decode (GstVideoDecoder * dec)
   GstFlowReturn res = GST_FLOW_OK;
   GList *walk;
 
-  walk = priv->decode;
-
   GST_DEBUG_OBJECT (dec, "flushing buffers to decode");
 
   /* clear buffer and decoder state */
   gst_video_decoder_flush (dec, FALSE);
 
+  walk = priv->decode;
   while (walk) {
     GList *next;
     GstVideoCodecFrame *frame = (GstVideoCodecFrame *) (walk->data);
@@ -1634,9 +1637,13 @@ gst_video_decoder_flush_decode (GstVideoDecoder * dec)
 
     next = walk->next;
 
+    priv->decode = g_list_delete_link (priv->decode, walk);
+
     /* decode buffer, resulting data prepended to queue */
     gst_video_codec_frame_ref (frame);
     res = gst_video_decoder_decode_frame (dec, frame);
+    if (res != GST_FLOW_OK)
+      break;
 
     walk = next;
   }
@@ -1644,6 +1651,13 @@ gst_video_decoder_flush_decode (GstVideoDecoder * dec)
   return res;
 }
 
+/* gst_video_decoder_flush_parse is called from the
+ * chain_reverse() function when a buffer containing
+ * a DISCONT - indicating that reverse playback
+ * looped back to the next data block, and therefore
+ * all available data should be fed through the
+ * decoder and frames gathered for reversed output
+ */
 static GstFlowReturn
 gst_video_decoder_flush_parse (GstVideoDecoder * dec)
 {
@@ -1651,21 +1665,25 @@ gst_video_decoder_flush_parse (GstVideoDecoder * dec)
   GstFlowReturn res = GST_FLOW_OK;
   GList *walk;
 
-  walk = priv->parse;
-
   GST_DEBUG_OBJECT (dec, "flushing buffers to parsing");
 
+  /* Reverse the gather list, and prepend it to the parse list,
+   * then flush to parse whatever we can */
+  priv->gather = g_list_reverse (priv->gather);
+  priv->parse = g_list_concat (priv->gather, priv->parse);
+  priv->gather = NULL;
+
   /* clear buffer and decoder state */
   gst_video_decoder_flush (dec, FALSE);
 
+  walk = priv->parse;
   while (walk) {
-    GList *next;
     GstBuffer *buf = GST_BUFFER_CAST (walk->data);
+    GList *next = walk->next;
 
     GST_DEBUG_OBJECT (dec, "parsing buffer %p, ts %" GST_TIME_FORMAT,
         buf, GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (buf)));
 
-    next = walk->next;
     /* parse buffer, resulting frames prepended to parse_gather queue */
     gst_buffer_ref (buf);
     res = gst_video_decoder_chain_forward (dec, buf);
@@ -1682,23 +1700,30 @@ gst_video_decoder_flush_parse (GstVideoDecoder * dec)
     walk = next;
   }
 
-  /* now we can process frames */
+  /* now we can process frames. Start by moving each frame from the parse_gather
-  GST_DEBUG_OBJECT (dec, "checking frames");
+   * to the decode list, reverse the order as we go, and stopping when/if we
-  while (priv->parse_gather) {
+   * copy a keyframe. */
-    GstVideoCodecFrame *frame;
+  GST_DEBUG_OBJECT (dec, "checking parsed frames for a keyframe to decode");
+  walk = priv->parse_gather;
+  while (walk) {
+    GstVideoCodecFrame *frame = (GstVideoCodecFrame *) (walk->data);
 
-    frame = (GstVideoCodecFrame *) (priv->parse_gather->data);
     /* remove from the gather list */
-    priv->parse_gather =
+    priv->parse_gather = g_list_remove_link (priv->parse_gather, walk);
-        g_list_delete_link (priv->parse_gather, priv->parse_gather);
+
-    /* copy to decode queue */
+    /* move it to the front of the decode queue */
-    priv->decode = g_list_prepend (priv->decode, frame);
+    priv->decode = g_list_concat (walk, priv->decode);
 
     /* if we copied a keyframe, flush and decode the decode queue */
     if (GST_VIDEO_CODEC_FRAME_IS_SYNC_POINT (frame)) {
-      GST_DEBUG_OBJECT (dec, "copied keyframe");
+      GST_DEBUG_OBJECT (dec, "found keyframe %p with PTS %" GST_TIME_FORMAT,
+          frame, GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (frame->input_buffer)));
       res = gst_video_decoder_flush_decode (dec);
+      if (res != GST_FLOW_OK)
+        goto done;
     }
+
+    walk = priv->parse_gather;
   }
 
   /* now send queued data downstream */
@@ -1710,7 +1735,22 @@ gst_video_decoder_flush_parse (GstVideoDecoder * dec)
       /* avoid stray DISCONT from forward processing,
        * which have no meaning in reverse pushing */
       GST_BUFFER_FLAG_UNSET (buf, GST_BUFFER_FLAG_DISCONT);
-      res = gst_pad_push (dec->srcpad, buf);
+
+      /* Last chance to calculate a timestamp as we loop backwards
+       * through the list */
+      if (GST_BUFFER_TIMESTAMP (buf) != GST_CLOCK_TIME_NONE)
+        priv->last_timestamp = GST_BUFFER_TIMESTAMP (buf);
+      else if (priv->last_timestamp != GST_CLOCK_TIME_NONE &&
+          GST_BUFFER_DURATION (buf) != GST_CLOCK_TIME_NONE) {
+        GST_BUFFER_TIMESTAMP (buf) =
+            priv->last_timestamp - GST_BUFFER_DURATION (buf);
+        priv->last_timestamp = GST_BUFFER_TIMESTAMP (buf);
+        GST_LOG_OBJECT (dec,
+            "Calculated TS %" GST_TIME_FORMAT " working backwards",
+            GST_TIME_ARGS (priv->last_timestamp));
+      }
+
+      res = gst_video_decoder_clip_and_push_buf (dec, buf);
     } else {
       gst_buffer_unref (buf);
     }
@@ -1720,6 +1760,7 @@ gst_video_decoder_flush_parse (GstVideoDecoder * dec)
     walk = priv->output_queued;
   }
 
+done:
   return res;
 }
 
@@ -1732,16 +1773,8 @@ gst_video_decoder_chain_reverse (GstVideoDecoder * dec, GstBuffer * buf)
   /* if we have a discont, move buffers to the decode list */
   if (!buf || GST_BUFFER_IS_DISCONT (buf)) {
     GST_DEBUG_OBJECT (dec, "received discont");
-    while (priv->gather) {
-      GstBuffer *gbuf;
 
-      gbuf = GST_BUFFER_CAST (priv->gather->data);
+    /* parse and decode stuff in the gather and parse queues */
-      /* remove from the gather list */
-      priv->gather = g_list_delete_link (priv->gather, priv->gather);
-      /* copy to parse queue */
-      priv->parse = g_list_prepend (priv->parse, gbuf);
-    }
-    /* parse and decode stuff in the parse queue */
     gst_video_decoder_flush_parse (dec);
   }
 
@@ -2031,7 +2064,7 @@ no_output_buffer:
 }
 
 static void
-gst_video_decoder_do_finish_frame (GstVideoDecoder * dec,
+gst_video_decoder_release_frame (GstVideoDecoder * dec,
     GstVideoCodecFrame * frame)
 {
   GList *link;
@@ -2097,7 +2130,7 @@ gst_video_decoder_drop_frame (GstVideoDecoder * dec, GstVideoCodecFrame * frame)
   gst_element_post_message (GST_ELEMENT_CAST (dec), qos_msg);
 
   /* now free the frame */
-  gst_video_decoder_do_finish_frame (dec, frame);
+  gst_video_decoder_release_frame (dec, frame);
 
   GST_VIDEO_DECODER_STREAM_UNLOCK (dec);
 
@@ -2172,7 +2205,7 @@ gst_video_decoder_finish_frame (GstVideoDecoder * decoder,
   }
 
 done:
-  gst_video_decoder_do_finish_frame (decoder, frame);
+  gst_video_decoder_release_frame (decoder, frame);
   GST_VIDEO_DECODER_STREAM_UNLOCK (decoder);
   return ret;
 }
@@ -2236,10 +2269,9 @@ gst_video_decoder_clip_and_push_buf (GstVideoDecoder * decoder, GstBuffer * buf)
     priv->time = GST_CLOCK_TIME_NONE;
   }
 
-  gst_buffer_set_caps (buf, GST_PAD_CAPS (decoder->srcpad));
+  GST_DEBUG_OBJECT (decoder, "pushing buffer %p of size %" G_GSIZE_FORMAT ", "
-
+      "time %" GST_TIME_FORMAT ", dur %" GST_TIME_FORMAT, buf,
-  GST_LOG_OBJECT (decoder, "pushing buffer %p of size %u ts %" GST_TIME_FORMAT
+      gst_buffer_get_size (buf),
-      ", duration %" GST_TIME_FORMAT, buf, gst_buffer_get_size (buf),
       GST_TIME_ARGS (GST_BUFFER_TIMESTAMP (buf)),
       GST_TIME_ARGS (GST_BUFFER_DURATION (buf)));
 
@@ -2247,12 +2279,8 @@ gst_video_decoder_clip_and_push_buf (GstVideoDecoder * decoder, GstBuffer * buf)
    * the error count, if there is one */
   if (G_UNLIKELY (priv->error_count))
     priv->error_count--;
-  if (decoder->output_segment.rate < 0.0) {
+
-    GST_LOG_OBJECT (decoder, "queued buffer");
+  ret = gst_pad_push (decoder->srcpad, buf);
-    priv->output_queued = g_list_prepend (priv->output_queued, buf);
-  } else {
-    ret = gst_pad_push (decoder->srcpad, buf);
-  }
 
 done:
   return ret;