va: decoder: store output buffer rather than surface

GstVaDecodePicture stored the processed VASurfaceID, under the
assumption that the bufferpool with keep the referenced buffers, but
this approach is fragil.

This patch changes GstVaDecodePicture to store the output buffer,
which already contains its VASurfaceID, and provides a new method to
retrieve the VASurfaceID directly from picture.

Based on He Junyan <junyan.he@intel.com> patches for
https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1587

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1594>

sys/va/gstvadecoder.c

@@ -24,6 +24,7 @@
 
 #include "gstvadecoder.h"
 
+#include "gstvaallocator.h"
 #include "gstvacaps.h"
 #include "gstvadisplay_wrapped.h"
 #include "gstvavideoformat.h"
@@ -568,18 +569,25 @@ gst_va_decoder_decode (GstVaDecoder * self, GstVaDecodePicture * pic)
 {
   VADisplay dpy;
   VAStatus status;
+  VASurfaceID surface;
   gboolean ret = FALSE;
 
   g_return_val_if_fail (GST_IS_VA_DECODER (self), FALSE);
   g_return_val_if_fail (self->context != VA_INVALID_ID, FALSE);
-  g_return_val_if_fail (pic && pic->surface != VA_INVALID_ID, FALSE);
+  g_return_val_if_fail (pic, FALSE);
 
-  GST_TRACE_OBJECT (self, "Decode to surface %#x", pic->surface);
+  surface = gst_va_decode_picture_get_surface (pic);
+  if (surface == VA_INVALID_ID) {
+    GST_ERROR_OBJECT (self, "Decode picture without VASurfaceID");
+    return FALSE;
+  }
+
+  GST_TRACE_OBJECT (self, "Decode to surface %#x", surface);
 
   dpy = gst_va_display_get_va_dpy (self->display);
 
   gst_va_display_lock (self->display);
-  status = vaBeginPicture (dpy, self->context, pic->surface);
+  status = vaBeginPicture (dpy, self->context, surface);
   gst_va_display_unlock (self->display);
   if (status != VA_STATUS_SUCCESS) {
     GST_WARNING_OBJECT (self, "vaBeginPicture: %s", vaErrorStr (status));
@@ -638,13 +646,20 @@ gst_va_decoder_destroy_buffers (GstVaDecoder * self, GstVaDecodePicture * pic)
   VABufferID buffer;
   VADisplay dpy;
   VAStatus status;
+  VASurfaceID surface;
   guint i;
   gboolean ret = TRUE;
 
   g_return_val_if_fail (GST_IS_VA_DECODER (self), FALSE);
-  g_return_val_if_fail (pic && pic->surface != VA_INVALID_ID, FALSE);
+  g_return_val_if_fail (pic, FALSE);
 
-  GST_TRACE_OBJECT (self, "Destroy buffers of surface %#x", pic->surface);
+  surface = gst_va_decode_picture_get_surface (pic);
+  if (surface == VA_INVALID_ID) {
+    GST_ERROR_OBJECT (self, "Decode picture without VASurfaceID");
+    return FALSE;
+  }
+
+  GST_TRACE_OBJECT (self, "Destroy buffers of surface %#x", surface);
 
   dpy = gst_va_display_get_va_dpy (self->display);
 
@@ -679,20 +694,29 @@ gst_va_decoder_destroy_buffers (GstVaDecoder * self, GstVaDecodePicture * pic)
 
 
 GstVaDecodePicture *
-gst_va_decode_picture_new (VASurfaceID surface)
+gst_va_decode_picture_new (GstBuffer * buffer)
 {
   GstVaDecodePicture *pic;
 
-  g_return_val_if_fail (surface != VA_INVALID_ID, NULL);
+  g_return_val_if_fail (buffer && GST_IS_BUFFER (buffer), NULL);
 
   pic = g_slice_new (GstVaDecodePicture);
-  pic->surface = surface;
+  pic->gstbuffer = gst_buffer_ref (buffer);
   pic->buffers = g_array_sized_new (FALSE, FALSE, sizeof (VABufferID), 16);
   pic->slices = g_array_sized_new (FALSE, FALSE, sizeof (VABufferID), 64);
 
   return pic;
 }
 
+VASurfaceID
+gst_va_decode_picture_get_surface (GstVaDecodePicture * pic)
+{
+  g_return_val_if_fail (pic, VA_INVALID_ID);
+  g_return_val_if_fail (pic->gstbuffer, VA_INVALID_ID);
+
+  return gst_va_buffer_get_surface (pic->gstbuffer, NULL);
+}
+
 void
 gst_va_decode_picture_free (GstVaDecodePicture * pic)
 {
@@ -701,6 +725,7 @@ gst_va_decode_picture_free (GstVaDecodePicture * pic)
   if (pic->buffers->len > 0 || pic->slices->len > 0)
     GST_WARNING ("VABufferID are leaked");
 
+  gst_buffer_unref (pic->gstbuffer);
   g_clear_pointer (&pic->buffers, g_array_unref);
   g_clear_pointer (&pic->slices, g_array_unref);
 

sys/va/gstvadecoder.h

@@ -29,7 +29,7 @@ struct _GstVaDecodePicture
 {
   GArray *buffers;
   GArray *slices;
-  VASurfaceID surface;
+  GstBuffer *gstbuffer;
 };
 
 #define GST_TYPE_VA_DECODER (gst_va_decoder_get_type())
@@ -69,7 +69,8 @@ gboolean              gst_va_decoder_decode               (GstVaDecoder * self,
 gboolean              gst_va_decoder_destroy_buffers      (GstVaDecoder * self,
                                                            GstVaDecodePicture * pic);
 
-GstVaDecodePicture *  gst_va_decode_picture_new           (VASurfaceID surface);
+GstVaDecodePicture *  gst_va_decode_picture_new           (GstBuffer * buffer);
+VASurfaceID           gst_va_decode_picture_get_surface   (GstVaDecodePicture * pic);
 void                  gst_va_decode_picture_free          (GstVaDecodePicture * pic);
 
 G_END_DECLS

sys/va/gstvah264dec.c

@@ -240,7 +240,7 @@ _fill_vaapi_pic (VAPictureH264 * va_picture, GstH264Picture * picture)
     return;
   }
 
-  va_picture->picture_id = va_pic->surface;
+  va_picture->picture_id = gst_va_decode_picture_get_surface (va_pic);
   va_picture->flags = 0;
 
   if (picture->ref && picture->long_term) {
@@ -553,19 +553,17 @@ gst_va_h264_dec_new_picture (GstH264Decoder * decoder,
   GstVaH264Dec *self = GST_VA_H264_DEC (decoder);
   GstVaDecodePicture *pic;
   GstVideoDecoder *vdec = GST_VIDEO_DECODER (decoder);
-  VASurfaceID surface;
 
   self->last_ret = gst_video_decoder_allocate_output_frame (vdec, frame);
   if (self->last_ret != GST_FLOW_OK)
     goto error;
 
-  surface = gst_va_buffer_get_surface (frame->output_buffer, NULL);
+  pic = gst_va_decode_picture_new (frame->output_buffer);
-
-  pic = gst_va_decode_picture_new (surface);
   gst_h264_picture_set_user_data (picture, pic,
       (GDestroyNotify) gst_va_decode_picture_free);
 
-  GST_LOG_OBJECT (self, "New va decode picture %p - %#x", pic, pic->surface);
+  GST_LOG_OBJECT (self, "New va decode picture %p - %#x", pic,
+      gst_va_decode_picture_get_surface (pic));
 
   return TRUE;