vaapidecode: delayed src caps negotiation

Currently the src caps are set immediately after the sink caps are set, but in
that moment the pipeline might not fully constructed and the video sink has
not negotiated its supported caps and features. As a consequence, in many cases
of playback, the least optimized caps feature is forced. This is partially the
responsible of bug #744039.

Also, vaapidecode doesn't attend the reconfigure events from downstream,
which is a problem too, since the video sink can be changed with different
caps features.

This patch delays the src caps, setting them until the first frame arrives to
the decoder, assuming until that very moment the whole pipeline is already
negotiated. Particularly, it checks if the src pad needs to be reconfigured,
as a consequence of a reconfiguration event from downstream.

A key part of this patch is the new GstVaapiCapsFeature
GST_VAAPI_CAPS_FEATURE_NOT_NEGOTIATED, which is returned when the src pad
doesn't have a peer yet. Also, for a better report of the caps allowed
through the src pad and its peer, this patch uses gst_pad_get_allowed_caps()
instead of gst_pad_peer_query_caps() when looking for the preferred feature.

v3: move the input_state unref to close(), since videodecoder resets at
some events such as navigation.

v4: a) the state_changed() callback replaces the input_state if the media
changed, so this case is also handled.
    b) since the parameter ref_state in gst_vaapidecode_update_src_caps() is
always the input_state, the parameter were removed.
    c) there were a lot of repeated code handling the input_state, so I
refactored it with the function gst_vaapi_decode_input_state_replace().

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

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Signed-off-by: Sreerenj Balachandran <sreerenj.balachandran@intel.com>

diff --git a/gst/vaapi/gstvaapidecode.c b/gst/vaapi/gstvaapidecode.c
index d2213c1..8054a1b 100644 (file)
--- a/gst/vaapi/gstvaapidecode.c
+++ b/gst/vaapi/gstvaapidecode.c
@@ -106,8 +106,11 @@ G_DEFINE_TYPE_WITH_CODE(
     GST_VAAPI_PLUGIN_BASE_INIT_INTERFACES)
 
 static gboolean
-gst_vaapidecode_update_src_caps(GstVaapiDecode *decode,
-    const GstVideoCodecState *ref_state);
+gst_vaapidecode_update_src_caps(GstVaapiDecode *decode);
+
+static gboolean
+gst_vaapi_decode_input_state_replace(GstVaapiDecode *decode,
+    const GstVideoCodecState *new_state);
 
 static void
 gst_vaapi_decoder_state_changed(GstVaapiDecoder *decoder,
@@ -119,12 +122,36 @@ gst_vaapi_decoder_state_changed(GstVaapiDecoder *decoder,
 
     g_assert(decode->decoder == decoder);
 
-    if (gst_vaapidecode_update_src_caps(decode, codec_state)) {
-        if (!gst_video_decoder_negotiate(vdec))
-            return;
-        if (!gst_vaapi_plugin_base_set_caps(plugin, NULL, decode->srcpad_caps))
-            return;
-   }
+    if (!gst_vaapi_decode_input_state_replace(decode, codec_state))
+        return;
+    if (!gst_vaapidecode_update_src_caps(decode))
+        return;
+    if (!gst_video_decoder_negotiate(vdec))
+        return;
+    if (!gst_vaapi_plugin_base_set_caps(plugin, NULL, decode->srcpad_caps))
+        return;
+}
+
+static gboolean
+gst_vaapi_decode_input_state_replace(GstVaapiDecode *decode,
+    const GstVideoCodecState *new_state)
+{
+    if (decode->input_state) {
+        if (new_state) {
+            const GstCaps *curcaps = decode->input_state->caps;
+            if (gst_caps_is_always_compatible(curcaps, new_state->caps))
+                return FALSE;
+        }
+        gst_video_codec_state_unref(decode->input_state);
+    }
+
+    if (new_state)
+        decode->input_state = gst_video_codec_state_ref
+            ((GstVideoCodecState*) new_state);
+    else
+        decode->input_state = NULL;
+
+    return TRUE;
 }
 
 static inline gboolean
@@ -135,13 +162,18 @@ gst_vaapidecode_update_sink_caps(GstVaapiDecode *decode, GstCaps *caps)
 }
 
 static gboolean
-gst_vaapidecode_update_src_caps(GstVaapiDecode *decode,
-    const GstVideoCodecState *ref_state)
+gst_vaapidecode_update_src_caps(GstVaapiDecode *decode)
 {
     GstVideoDecoder * const vdec = GST_VIDEO_DECODER(decode);
-    GstVideoCodecState *state;
+    GstVideoCodecState *state, *ref_state;
     GstVideoInfo *vi, vis;
     GstVideoFormat format, out_format;
+
+    if (!decode->input_state)
+        return FALSE;
+
+    ref_state = decode->input_state;
+
 #if GST_CHECK_VERSION(1,1,0)
     GstCapsFeatures *features = NULL;
     GstVaapiCapsFeature feature;
@@ -149,6 +181,9 @@ gst_vaapidecode_update_src_caps(GstVaapiDecode *decode,
     feature = gst_vaapi_find_preferred_caps_feature(
         GST_VIDEO_DECODER_SRC_PAD(vdec),
         GST_VIDEO_INFO_FORMAT(&ref_state->info), &out_format);
+
+    if (feature == GST_VAAPI_CAPS_FEATURE_NOT_NEGOTIATED)
+        return FALSE;
 #endif
 
     format = GST_VIDEO_INFO_FORMAT(&ref_state->info);
@@ -398,14 +433,43 @@ error_commit_buffer:
 static GstFlowReturn
 gst_vaapidecode_handle_frame(GstVideoDecoder *vdec, GstVideoCodecFrame *frame)
 {
+    GstVaapiDecode * const decode = GST_VAAPIDECODE(vdec);
     GstFlowReturn ret;
 
+    if (!decode->input_state)
+        goto not_negotiated;
+
+    if (G_UNLIKELY(!decode->active) ||
+        gst_pad_needs_reconfigure(GST_VIDEO_DECODER_SRC_PAD(vdec))) {
+        GST_DEBUG_OBJECT(decode, "activating the decoder");
+        if (!gst_vaapidecode_update_src_caps(decode))
+            goto not_negotiated;
+
+        if (!gst_video_decoder_negotiate(vdec))
+            goto not_negotiated;
+
+        GstVaapiPluginBase * const plugin = GST_VAAPI_PLUGIN_BASE(vdec);
+        if (!gst_vaapi_plugin_base_set_caps(plugin, NULL, decode->srcpad_caps))
+            goto not_negotiated;
+
+        decode->active = TRUE;
+    }
+
     /* Make sure to release the base class stream lock so that decode
        loop can call gst_video_decoder_finish_frame() without blocking */
     GST_VIDEO_DECODER_STREAM_UNLOCK(vdec);
     ret = gst_vaapidecode_decode_frame(vdec, frame);
     GST_VIDEO_DECODER_STREAM_LOCK(vdec);
     return ret;
+
+    /* ERRORS */
+not_negotiated:
+  {
+      GST_ERROR_OBJECT (decode, "not negotiated");
+      ret = GST_FLOW_NOT_NEGOTIATED;
+      gst_video_decoder_drop_frame (vdec, frame);
+      return ret;
+  }
 }
 
 static void
@@ -545,7 +609,7 @@ gst_vaapidecode_decide_allocation(GstVideoDecoder *vdec, GstQuery *query)
             gst_video_info_change_format(&state->info, out_format,
                 GST_VIDEO_INFO_WIDTH(&state->info),
                 GST_VIDEO_INFO_HEIGHT(&state->info));
-        gst_vaapidecode_update_src_caps(decode, state);
+        gst_vaapidecode_update_src_caps(decode);
     }
     gst_video_codec_state_unref(state);
 
@@ -639,6 +703,9 @@ gst_vaapidecode_destroy(GstVaapiDecode *decode)
     gst_pad_stop_task(GST_VAAPI_PLUGIN_BASE_SRC_PAD(decode));
     gst_vaapi_decoder_replace(&decode->decoder, NULL);
     gst_caps_replace(&decode->decoder_caps, NULL);
+
+    decode->active = FALSE;
+
     gst_vaapidecode_release(decode);
 }
 
@@ -728,6 +795,7 @@ gst_vaapidecode_close(GstVideoDecoder *vdec)
 {
     GstVaapiDecode * const decode = GST_VAAPIDECODE(vdec);
 
+    gst_vaapi_decode_input_state_replace(decode, NULL);
     gst_vaapidecode_destroy(decode);
     gst_vaapi_plugin_base_close(GST_VAAPI_PLUGIN_BASE(decode));
     return TRUE;
@@ -750,20 +818,15 @@ gst_vaapidecode_set_format(GstVideoDecoder *vdec, GstVideoCodecState *state)
     GstVaapiPluginBase * const plugin = GST_VAAPI_PLUGIN_BASE(vdec);
     GstVaapiDecode * const decode = GST_VAAPIDECODE(vdec);
 
+    if (!gst_vaapi_decode_input_state_replace(decode, state))
+        return TRUE;
     if (!gst_vaapidecode_update_sink_caps(decode, state->caps))
         return FALSE;
     if (!gst_vaapi_plugin_base_set_caps(plugin, decode->sinkpad_caps, NULL))
         return FALSE;
-
-    if (gst_vaapidecode_update_src_caps(decode, state)) {
-        if (!gst_video_decoder_negotiate(vdec))
-            return FALSE;
-        if (!gst_vaapi_plugin_base_set_caps(plugin, NULL, decode->srcpad_caps))
-            return FALSE;
-    }
-
     if (!gst_vaapidecode_reset_full(decode, decode->sinkpad_caps, FALSE))
         return FALSE;
+
     return TRUE;
 }
 

diff --git a/gst/vaapi/gstvaapidecode.h b/gst/vaapi/gstvaapidecode.h
index 7128709..02f192e 100644 (file)
--- a/gst/vaapi/gstvaapidecode.h
+++ b/gst/vaapi/gstvaapidecode.h
@@ -73,6 +73,9 @@ struct _GstVaapiDecode {
     GstCaps            *allowed_caps;
     guint               current_frame_size;
     guint               has_texture_upload_meta : 1;
+
+    GstVideoCodecState *input_state;
+    volatile gboolean   active;
 };
 
 struct _GstVaapiDecodeClass {

diff --git a/gst/vaapi/gstvaapipluginutil.c b/gst/vaapi/gstvaapipluginutil.c
index 1bb7c97..e075f83 100644 (file)
--- a/gst/vaapi/gstvaapipluginutil.c
+++ b/gst/vaapi/gstvaapipluginutil.c
@@ -659,9 +659,11 @@ gst_vaapi_find_preferred_caps_feature (GstPad * pad, GstVideoFormat format,
   GstCaps *out_caps;
   GstVideoFormat out_format;
 
-  out_caps = gst_pad_peer_query_caps (pad, NULL);
-  if (!out_caps)
+  out_caps= gst_pad_get_allowed_caps (pad);
+  if (!out_caps) {
+    feature = GST_VAAPI_CAPS_FEATURE_NOT_NEGOTIATED;
     goto cleanup;
+  }
 
   out_format = format == GST_VIDEO_FORMAT_ENCODED ?
     GST_VIDEO_FORMAT_I420 : format;

diff --git a/gst/vaapi/gstvaapipluginutil.h b/gst/vaapi/gstvaapipluginutil.h
index b72b01c..7520cdd 100644 (file)
--- a/gst/vaapi/gstvaapipluginutil.h
+++ b/gst/vaapi/gstvaapipluginutil.h
@@ -70,7 +70,8 @@ gst_vaapi_value_set_format_list (GValue * value, GArray * formats);
 /* Helpers to build video caps */
 typedef enum
 {
-  GST_VAAPI_CAPS_FEATURE_SYSTEM_MEMORY = 1,
+  GST_VAAPI_CAPS_FEATURE_NOT_NEGOTIATED,
+  GST_VAAPI_CAPS_FEATURE_SYSTEM_MEMORY,
   GST_VAAPI_CAPS_FEATURE_GL_TEXTURE_UPLOAD_META,
   GST_VAAPI_CAPS_FEATURE_VAAPI_SURFACE,
 } GstVaapiCapsFeature;