videodecoder: parse any source data that is still available.

Fix gst_video_decoder_parse_available() to really parse any pending
source data that is still available in the adapter. This is a memory
optimization to avoid expansion of video packed added to the adapter,
but also a fix to EOS condition when the subclass parse() function
ultimately only needed to call into gvd_have_frame() and no additional
source bytes were consumed, i.e. gvd_add_to_frame() is not called.

This situation can occur when decoding H.264 streams in byte-stream/nal
mode for instance. A decoder always requires the next NAL unit to be
parsed so that to determine picture boundaries. When a new picture is
found, no byte is consumed (i.e. gvd_add_to_frame() is not called)
but gvd_have_frame() is called (i.e. priv->current_frame is gone).

Also make sure to avoid infinite loops caused by incorrect subclass
parse() implementations. This can occur when no byte gets consumed
and no appropriate indication (GST_VIDEO_DECODER_FLOW_NEED_DATA) is
returned.

https://bugzilla.gnome.org/show_bug.cgi?id=731974

Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>

diff --git a/gst-libs/gst/video/gstvideodecoder.c b/gst-libs/gst/video/gstvideodecoder.c
index fe6acea..95577d1 100644 (file)
--- a/gst-libs/gst/video/gstvideodecoder.c
+++ b/gst-libs/gst/video/gstvideodecoder.c
@@ -926,26 +926,46 @@ gst_video_decoder_parse_available (GstVideoDecoder * dec, gboolean at_eos,
   GstVideoDecoderClass *decoder_class = GST_VIDEO_DECODER_GET_CLASS (dec);
   GstVideoDecoderPrivate *priv = dec->priv;
   GstFlowReturn ret = GST_FLOW_OK;
-  gsize start_size, available;
+  gsize was_available, available;
+  guint inactive = 0;
 
   available = gst_adapter_available (priv->input_adapter);
-  start_size = 0;
 
-  while (ret == GST_FLOW_OK && ((available && start_size != available)
-          || new_buffer)) {
+  while (available || new_buffer) {
     new_buffer = FALSE;
     /* current frame may have been parsed and handled,
      * so we need to set up a new one when asking subclass to parse */
     if (priv->current_frame == NULL)
       priv->current_frame = gst_video_decoder_new_frame (dec);
 
-    start_size = available;
+    was_available = available;
     ret = decoder_class->parse (dec, priv->current_frame,
         priv->input_adapter, at_eos);
+    if (ret != GST_FLOW_OK)
+      break;
+
+    /* if the subclass returned success (GST_FLOW_OK), it is expected
+     * to have collected and submitted a frame, i.e. it should have
+     * called gst_video_decoder_have_frame(), or at least consumed a
+     * few bytes through gst_video_decoder_add_to_frame().
+     *
+     * Otherwise, this is an implementation bug, and we error out
+     * after 2 failed attempts */
     available = gst_adapter_available (priv->input_adapter);
+    if (!priv->current_frame || available != was_available)
+      inactive = 0;
+    else if (++inactive == 2)
+      goto error_inactive;
   }
 
   return ret;
+
+  /* ERRORS */
+error_inactive:
+  {
+    GST_ERROR_OBJECT (dec, "Failed to consume data. Error in subclass?");
+    return GST_FLOW_ERROR;
+  }
 }
 
 static GstFlowReturn