Remove some helper func of GstVaapiVideoBuffer which is easy to leak memory

1. Remove functions in GstVaapiVideoBuffer:
      gst_vaapi_video_buffer_typed_new_with_image
      gst_vaapi_video_buffer_typed_new_with_surface
2. Remove functions in gstvaapipluginbuffer:
      gst_vaapi_video_buffer_new_with_image
      gst_vaapi_video_buffer_new_with_surface
3. Change to local/staic func in GstVaapiVideoBuffer to avoid misuse:
    gst_vaapi_video_buffer_set_image
    gst_vaapi_video_buffer_set_surface
4. gst_vaapi_video_buffer_typed_new_from_buffer is also problemtic,
   but not removed yet since it is still used by vaapiupload
5. gst_vaapi_video_buffer_typed_new_with_surface_proxy is problemtic,
   but not remove yet since it is used by a wacky de-interlace method
6. Add fourcc support in GstVaapiSurfacePool
7. update vaapiupload to use gst_vaapi_video_buffer_set_image_from_pool
8. update vaapisink _buffer_alloc since GstVaapiSurfacePool supports fourcc

diff --git a/gst-libs/gst/vaapi/gstvaapisurfacepool.c b/gst-libs/gst/vaapi/gstvaapisurfacepool.c
index 528d6f4..ca37bda 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapisurfacepool.c
+++ b/gst-libs/gst/vaapi/gstvaapisurfacepool.c
@@ -42,6 +42,7 @@ G_DEFINE_TYPE(
 
 struct _GstVaapiSurfacePoolPrivate {
     GstVaapiChromaType  chroma_type;
+    guint               fourcc;
     guint               width;
     guint               height;
 };
@@ -52,6 +53,7 @@ gst_vaapi_surface_pool_set_caps(GstVaapiVideoPool *pool, GstCaps *caps)
     GstVaapiSurfacePoolPrivate *priv = GST_VAAPI_SURFACE_POOL(pool)->priv;
     GstStructure *structure;
     gint width, height;
+    GstVideoFormat format;
 
     structure = gst_caps_get_structure(caps, 0);
     gst_structure_get_int(structure, "width", &width);
@@ -60,6 +62,14 @@ gst_vaapi_surface_pool_set_caps(GstVaapiVideoPool *pool, GstCaps *caps)
     priv->chroma_type   = GST_VAAPI_CHROMA_TYPE_YUV420;
     priv->width         = width;
     priv->height        = height;
+
+    /* Translate from Gst video format to VA image format */
+    if (gst_video_format_parse_caps(caps, &format, NULL, NULL)) {
+        priv->fourcc = gst_vaapi_image_format_from_video(format);
+    }
+    else
+        priv->fourcc = 0;
+
 }
 
 gpointer
@@ -69,11 +79,21 @@ gst_vaapi_surface_pool_alloc_object(
 )
 {
     GstVaapiSurfacePoolPrivate *priv = GST_VAAPI_SURFACE_POOL(pool)->priv;
-
-    return gst_vaapi_surface_new(display,
+    GstVaapiSurface *out_surface = NULL;
+    if (priv->fourcc) {
+        out_surface = gst_vaapi_surface_new_with_fourcc(
+                    display, priv->fourcc,
+                    priv->width, priv->height);
+
+    }
+    else {
+        out_surface = gst_vaapi_surface_new(display,
                                  priv->chroma_type,
                                  priv->width,
                                  priv->height);
+   }
+
+   return out_surface;
 }
 
 static void
@@ -105,6 +125,7 @@ gst_vaapi_surface_pool_init(GstVaapiSurfacePool *pool)
     priv->chroma_type   = 0;
     priv->width         = 0;
     priv->height        = 0;
+    priv->fourcc        = 0;
 }
 
 /**

diff --git a/gst-libs/gst/vaapi/gstvaapivideobuffer.c b/gst-libs/gst/vaapi/gstvaapivideobuffer.c
index cd1c948..f71ecfb 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapivideobuffer.c
+++ b/gst-libs/gst/vaapi/gstvaapivideobuffer.c
@@ -54,6 +54,17 @@ struct _GstVaapiVideoBufferPrivate {
 };
 
 static void
+gst_vaapi_video_buffer_set_image(
+    GstVaapiVideoBuffer *buffer,
+    GstVaapiImage       *image
+);
+void
+static gst_vaapi_video_buffer_set_surface(
+    GstVaapiVideoBuffer *buffer,
+    GstVaapiSurface     *surface
+);
+
+static void
 set_display(GstVaapiVideoBuffer *buffer, GstVaapiDisplay *display)
 {
     GstVaapiVideoBufferPrivate * const priv = buffer->priv;
@@ -252,6 +263,14 @@ gst_vaapi_video_buffer_typed_new_from_pool(GType type, GstVaapiVideoPool *pool)
  *
  * Return value: the newly allocated #GstBuffer, or %NULL on error
  */
+ /*
+   * CAUTION!
+   * This function clones  a new GstVaapiVideoBuffer with increase ref_count for GstVaapiImage and GstVaapiSurface.
+   * but when GstVaapiVideoBuffer is destroied, the ref_count of GstVaapiImage and GstVaapiSurface isn't checked at all.
+   * it is problemtic.
+   * propose to drop it, does ref input buffer work?
+   * it is still used by vaapiupload now, let's examine it latter.
+ */
 GstBuffer *
 gst_vaapi_video_buffer_typed_new_from_buffer(GType type, GstBuffer *buffer)
 {
@@ -281,59 +300,6 @@ gst_vaapi_video_buffer_typed_new_from_buffer(GType type, GstBuffer *buffer)
 }
 
 /**
- * gst_vaapi_video_buffer_typed_new_with_image:
- * @image: a #GstVaapiImage
- *
- * Creates a #GstBuffer with the specified @image. The resulting
- * buffer holds an additional reference to the @image.
- *
- * This function shall only be called from within gstreamer-vaapi
- * plugin elements.
- *
- * Return value: the newly allocated #GstBuffer, or %NULL on error
- */
-GstBuffer *
-gst_vaapi_video_buffer_typed_new_with_image(GType type, GstVaapiImage *image)
-{
-    GstVaapiVideoBuffer *buffer;
-
-    g_return_val_if_fail(GST_VAAPI_IS_IMAGE(image), NULL);
-
-    buffer = _gst_vaapi_video_buffer_typed_new(type);
-    if (buffer)
-        gst_vaapi_video_buffer_set_image(buffer, image);
-    return GST_BUFFER(buffer);
-}
-
-/**
- * gst_vaapi_video_buffer_typed_new_with_surface:
- * @surface: a #GstVaapiSurface
- *
- * Creates a #GstBuffer with the specified @surface. The resulting
- * buffer holds an additional reference to the @surface.
- *
- * This function shall only be called from within gstreamer-vaapi
- * plugin elements.
- *
- * Return value: the newly allocated #GstBuffer, or %NULL on error
- */
-GstBuffer *
-gst_vaapi_video_buffer_typed_new_with_surface(
-    GType            type,
-    GstVaapiSurface *surface
-)
-{
-    GstVaapiVideoBuffer *buffer;
-
-    g_return_val_if_fail(GST_VAAPI_IS_SURFACE(surface), NULL);
-
-    buffer = _gst_vaapi_video_buffer_typed_new(type);
-    if (buffer)
-        gst_vaapi_video_buffer_set_surface(buffer, surface);
-    return GST_BUFFER(buffer);
-}
-
-/**
  * gst_vaapi_video_buffer_typed_new_with_surface_proxy:
  * @proxy: a #GstVaapiSurfaceProxy
  *
@@ -343,6 +309,11 @@ gst_vaapi_video_buffer_typed_new_with_surface(
  * This function shall only be called from within gstreamer-vaapi
  * plugin elements.
  *
+ * CAUTION!
+ * it may lead to memory leak when priv->surface_pool isn't set.
+ * caller have to recycle the surface before destroy GstVaapiVideoBuffer.
+ * it is still used by the strange de-interlace method, let's examine it latter.
+ *
  * Return value: the newly allocated #GstBuffer, or %NULL on error
  */
 GstBuffer *
@@ -406,8 +377,14 @@ gst_vaapi_video_buffer_get_image(GstVaapiVideoBuffer *buffer)
  * Binds @image to the @buffer. If the @buffer contains another image
  * previously allocated from a pool, it's pushed back to its parent
  * pool and the pool is also released.
+ *
+ * CAUTION!
+ * it may lead to memory leak when priv->image_pool isn't set.
+ * caller have to recycle the image before destroy GstVaapiVideoBuffer.
+ * it is still used by gst_vaapi_video_buffer_typed_new_from_buffer(), let's examine it later.
+ * change it to local function for now.
  */
-void
+static void
 gst_vaapi_video_buffer_set_image(
     GstVaapiVideoBuffer *buffer,
     GstVaapiImage       *image
@@ -483,9 +460,14 @@ gst_vaapi_video_buffer_get_surface(GstVaapiVideoBuffer *buffer)
  * Binds @surface to the @buffer. If the @buffer contains another
  * surface previously allocated from a pool, it's pushed back to its
  * parent pool and the pool is also released.
+ * CAUTION!
+ * it may lead to memory leak when priv->surface_pool isn't set.
+ * caller have to recycle the image before destroy GstVaapiVideoBuffer.
+ * it is still used by gst_vaapi_video_buffer_typed_new_from_buffer(), let's examine it later.
+ * change it to local function for now.
  */
 void
-gst_vaapi_video_buffer_set_surface(
+static gst_vaapi_video_buffer_set_surface(
     GstVaapiVideoBuffer *buffer,
     GstVaapiSurface     *surface
 )

diff --git a/gst-libs/gst/vaapi/gstvaapivideobuffer.h b/gst-libs/gst/vaapi/gstvaapivideobuffer.h
index 5d479b2..fd7a9b3 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapivideobuffer.h
+++ b/gst-libs/gst/vaapi/gstvaapivideobuffer.h
@@ -91,12 +91,6 @@ gst_vaapi_video_buffer_get_display(GstVaapiVideoBuffer *buffer);
 GstVaapiImage *
 gst_vaapi_video_buffer_get_image(GstVaapiVideoBuffer *buffer);
 
-void
-gst_vaapi_video_buffer_set_image(
-    GstVaapiVideoBuffer *buffer,
-    GstVaapiImage       *image
-);
-
 gboolean
 gst_vaapi_video_buffer_set_image_from_pool(
     GstVaapiVideoBuffer *buffer,
@@ -106,12 +100,6 @@ gst_vaapi_video_buffer_set_image_from_pool(
 GstVaapiSurface *
 gst_vaapi_video_buffer_get_surface(GstVaapiVideoBuffer *buffer);
 
-void
-gst_vaapi_video_buffer_set_surface(
-    GstVaapiVideoBuffer *buffer,
-    GstVaapiSurface     *surface
-);
-
 gboolean
 gst_vaapi_video_buffer_set_surface_from_pool(
     GstVaapiVideoBuffer *buffer,

diff --git a/gst-libs/gst/vaapi/gstvaapivideobuffer_priv.h b/gst-libs/gst/vaapi/gstvaapivideobuffer_priv.h
index a08edbd..81f6ca9 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapivideobuffer_priv.h
+++ b/gst-libs/gst/vaapi/gstvaapivideobuffer_priv.h
@@ -40,15 +40,6 @@ GstBuffer *
 gst_vaapi_video_buffer_typed_new_from_buffer(GType type, GstBuffer *buffer);
 
 GstBuffer *
-gst_vaapi_video_buffer_typed_new_with_image(GType type, GstVaapiImage *image);
-
-GstBuffer *
-gst_vaapi_video_buffer_typed_new_with_surface(
-    GType            type,
-    GstVaapiSurface *surface
-);
-
-GstBuffer *
 gst_vaapi_video_buffer_typed_new_with_surface_proxy(
     GType                 type,
     GstVaapiSurfaceProxy *proxy

diff --git a/gst/vaapi/gstvaapipluginbuffer.c b/gst/vaapi/gstvaapipluginbuffer.c
index 33cb917..473cc31 100644 (file)
--- a/gst/vaapi/gstvaapipluginbuffer.c
+++ b/gst/vaapi/gstvaapipluginbuffer.c
@@ -87,36 +87,6 @@ gst_vaapi_video_buffer_new_from_buffer(GstBuffer *buffer)
 }
 
 GstBuffer *
-gst_vaapi_video_buffer_new_with_image(GstVaapiImage *image)
-{
-    GstVaapiDisplay *display;
-
-    g_return_val_if_fail(GST_VAAPI_IS_IMAGE(image), NULL);
-
-    display = gst_vaapi_object_get_display(GST_VAAPI_OBJECT(image));
-    if (!display)
-        return NULL;
-
-    return gst_vaapi_video_buffer_typed_new_with_image(
-        get_type(display), image);
-}
-
-GstBuffer *
-gst_vaapi_video_buffer_new_with_surface(GstVaapiSurface *surface)
-{
-    GstVaapiDisplay *display;
-
-    g_return_val_if_fail(GST_VAAPI_IS_SURFACE(surface), NULL);
-
-    display = gst_vaapi_object_get_display(GST_VAAPI_OBJECT(surface));
-    if (!display)
-        return NULL;
-
-    return gst_vaapi_video_buffer_typed_new_with_surface(
-        get_type(display), surface);
-}
-
-GstBuffer *
 gst_vaapi_video_buffer_new_with_surface_proxy(GstVaapiSurfaceProxy *proxy)
 {
     GstVaapiDisplay *display;

diff --git a/gst/vaapi/gstvaapipluginbuffer.h b/gst/vaapi/gstvaapipluginbuffer.h
index d6798d5..9c3ea1a 100644 (file)
--- a/gst/vaapi/gstvaapipluginbuffer.h
+++ b/gst/vaapi/gstvaapipluginbuffer.h
@@ -36,14 +36,6 @@ gst_vaapi_video_buffer_new_from_buffer(GstBuffer *buffer);
 
 G_GNUC_INTERNAL
 GstBuffer *
-gst_vaapi_video_buffer_new_with_image(GstVaapiImage *image);
-
-G_GNUC_INTERNAL
-GstBuffer *
-gst_vaapi_video_buffer_new_with_surface(GstVaapiSurface *surface);
-
-G_GNUC_INTERNAL
-GstBuffer *
 gst_vaapi_video_buffer_new_with_surface_proxy(GstVaapiSurfaceProxy *proxy);
 
 #endif /* GST_VAAPI_PLUGIN_BUFFER_H */

diff --git a/gst/vaapi/gstvaapisink.c b/gst/vaapi/gstvaapisink.c
index f13cb4e..573228e 100755 (executable)
--- a/gst/vaapi/gstvaapisink.c
+++ b/gst/vaapi/gstvaapisink.c
@@ -1043,6 +1043,7 @@ gst_vaapisink_buffer_alloc(
 {
     GstVaapiSink * const sink = GST_VAAPISINK(base_sink);
     GstStructure *structure;
+    GstCaps *dst_caps = NULL;
     GstBuffer *buf;
 
     *pbuf = NULL;
@@ -1053,9 +1054,18 @@ gst_vaapisink_buffer_alloc(
 
     if (!gst_vaapi_uploader_ensure_display(sink->uploader, sink->display))
         return GST_FLOW_NOT_SUPPORTED;
-    if (!gst_vaapi_uploader_ensure_caps(sink->uploader, caps, NULL))
-        return GST_FLOW_NOT_SUPPORTED;
-
+    gint width, height;
+    gst_structure_get_int(structure, "width", &width);
+    gst_structure_get_int(structure, "height", &height);
+
+    dst_caps =  gst_caps_new_simple ("video/x-surface",
+    // "format", G_TYPE_STRING, "I420", // leave format as empty, so video driver default format will be use to create surface pool.
+    "width", G_TYPE_INT, width,
+    "height", G_TYPE_INT, height,
+     NULL);
+    gboolean ret = gst_vaapi_uploader_ensure_caps(sink->uploader, caps, dst_caps);
+    gst_caps_unref(dst_caps);
+    if(!ret) return GST_FLOW_NOT_SUPPORTED;
     if (!gst_vaapi_uploader_has_direct_rendering(sink->uploader)) {
         return GST_FLOW_OK;
     }

diff --git a/gst/vaapi/gstvaapiuploader.c b/gst/vaapi/gstvaapiuploader.c
index dc3e934..ab340d4 100755 (executable)
--- a/gst/vaapi/gstvaapiuploader.c
+++ b/gst/vaapi/gstvaapiuploader.c
@@ -406,10 +406,7 @@ gst_vaapi_uploader_process(
         /* Regular GstBuffer that needs to be uploaded to a VA image */
         image = gst_vaapi_video_buffer_get_image(out_vbuffer);
         if (!image) {
-            image = gst_vaapi_video_pool_get_object(uploader->priv->images);
-            if (!image)
-                return FALSE;
-            gst_vaapi_video_buffer_set_image(out_vbuffer, image);
+            gst_vaapi_video_buffer_set_image_from_pool(out_vbuffer, uploader->priv->images);
         }
         if (!gst_vaapi_image_update_from_buffer(image, src_buffer, NULL))
             return FALSE;