v4l2codecs: vp9: Fix duplicating a duplicated picture
authorNicolas Dufresne <nicolas.dufresne@collabora.com>
Wed, 9 Feb 2022 20:05:14 +0000 (15:05 -0500)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Wed, 9 Feb 2022 22:37:55 +0000 (22:37 +0000)
Duplicating a picture what was already a dup was leading to a crash. Rename
the custom picture flags as HOLDS_BUFFER to make its meaning clear. Then save
then ref and store the picture as userdata, so it can be obtained when
duplicating. Finally, mark the doplicated as HOLDS_BUFFER to avoid thinking it
holds a request.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1681>

subprojects/gst-plugins-bad/sys/v4l2codecs/gstv4l2codecvp9dec.c

index 9aa2d86..1fefb08 100644 (file)
@@ -34,7 +34,7 @@ GST_DEBUG_CATEGORY_STATIC (v4l2_vp9dec_debug);
 #define GST_CAT_DEFAULT v4l2_vp9dec_debug
 
 /* Used to mark picture that have been outputed */
-#define FLAG_PICTURE_OUTPUTED GST_MINI_OBJECT_FLAG_LAST
+#define FLAG_PICTURE_HOLDS_BUFFER GST_MINI_OBJECT_FLAG_LAST
 
 enum
 {
@@ -882,10 +882,22 @@ gst_v4l2_codec_vp9_dec_duplicate_picture (GstVp9Decoder * decoder,
   new_picture->frame_hdr = picture->frame_hdr;
   new_picture->system_frame_number = frame->system_frame_number;
 
-  if (GST_MINI_OBJECT_FLAG_IS_SET (picture, FLAG_PICTURE_OUTPUTED)) {
+  if (GST_MINI_OBJECT_FLAG_IS_SET (picture, FLAG_PICTURE_HOLDS_BUFFER)) {
     GstBuffer *output_buffer = gst_vp9_picture_get_user_data (picture);
-    if (output_buffer)
+
+    if (output_buffer) {
       frame->output_buffer = gst_buffer_ref (output_buffer);
+
+      /* We need to also hold on the picture so it stays alive, but also to
+       * ensure we can duplicate it too. */
+      gst_vp9_picture_set_user_data (new_picture,
+          gst_buffer_ref (frame->output_buffer),
+          (GDestroyNotify) gst_buffer_unref);
+    }
+
+    /* Flag regardless if the buffer is null, so we don't start thinking it
+     * should hold a request unconditionally. */
+    GST_MINI_OBJECT_FLAG_SET (new_picture, FLAG_PICTURE_HOLDS_BUFFER);
   } else {
     GstV4l2Request *request = gst_vp9_picture_get_user_data (picture);
     gst_vp9_picture_set_user_data (new_picture, gst_v4l2_request_ref (request),
@@ -902,11 +914,14 @@ gst_v4l2_codec_vp9_dec_output_picture (GstVp9Decoder * decoder,
 {
   GstV4l2CodecVp9Dec *self = GST_V4L2_CODEC_VP9_DEC (decoder);
   GstVideoDecoder *vdec = GST_VIDEO_DECODER (decoder);
-  GstV4l2Request *request = gst_vp9_picture_get_user_data (picture);
+  GstV4l2Request *request = NULL;
   gint ret;
 
   GST_DEBUG_OBJECT (self, "Output picture %u", picture->system_frame_number);
 
+  if (!GST_MINI_OBJECT_FLAG_IS_SET (picture, FLAG_PICTURE_HOLDS_BUFFER))
+    request = gst_vp9_picture_get_user_data (picture);
+
   if (request) {
     ret = gst_v4l2_request_set_done (request);
     if (ret == 0) {
@@ -931,7 +946,7 @@ gst_v4l2_codec_vp9_dec_output_picture (GstVp9Decoder * decoder,
         gst_buffer_ref (frame->output_buffer),
         (GDestroyNotify) gst_buffer_unref);
 
-    GST_MINI_OBJECT_FLAG_SET (picture, FLAG_PICTURE_OUTPUTED);
+    GST_MINI_OBJECT_FLAG_SET (picture, FLAG_PICTURE_HOLDS_BUFFER);
   }
 
   /* This may happen if we duplicate a picture witch failed to decode */