[vlc-devel] [PATCH 08/19] picture: pass a picture_gc_t directly to picture_NewFromResource()

Steve Lhomme robux4 at ycbcr.xyz
Thu Jul 30 14:16:49 CEST 2020


Either pass a picture_gc_t, a picture_resource_t or both. They can both be NULL.

When no gc is passed the caller can set the p_sys of the picture manually as
it's not part of the init resources anymore. Otherwise the picture p_sys is set
to the same value as gc.opaque to keep compatibility with the old
picture_resource_t structure.
---
 include/vlc_picture.h                    | 15 +++++++++++----
 include/vlc_picture_pool.h               |  2 +-
 modules/codec/vt_utils.c                 |  2 +-
 modules/hw/d3d11/d3d11_surface.c         |  4 ++--
 modules/hw/d3d9/dxa9.c                   |  4 ++--
 modules/hw/nvdec/nvdec.c                 |  6 ++----
 modules/hw/vaapi/vlc_vaapi.c             |  9 +++------
 modules/hw/vdpau/picture.c               |  7 +++----
 modules/video_chroma/copy.c              |  5 +++--
 modules/video_output/android/display.c   |  8 ++------
 modules/video_output/fb.c                |  2 +-
 modules/video_output/kms.c               |  2 +-
 modules/video_output/opengl/interop_sw.c |  7 ++-----
 modules/video_output/vmem.c              |  4 ++--
 modules/video_output/win32/direct3d11.c  |  7 ++-----
 src/misc/picture.c                       | 24 +++++++++++++++---------
 src/misc/picture.h                       |  6 ------
 17 files changed, 53 insertions(+), 61 deletions(-)

diff --git a/include/vlc_picture.h b/include/vlc_picture.h
index 7bb9ee5083c..11c2fc70ac9 100644
--- a/include/vlc_picture.h
+++ b/include/vlc_picture.h
@@ -188,9 +188,6 @@ VLC_API picture_t * picture_NewFromFormat( const video_format_t *p_fmt ) VLC_USE
  */
 typedef struct
 {
-    void *p_sys;
-    void (*pf_destroy)(picture_t *);
-
     /* Plane resources
      * XXX all fields MUST be set to the right value.
      */
@@ -203,10 +200,20 @@ typedef struct
 
 } picture_resource_t;
 
+/**
+ * Callback structure to release allocated resources when a picture is released
+ * for good.
+ */
+typedef struct
+{
+    void (*destroy)(picture_t *);
+    void *opaque;
+} picture_gc_t;
+
 /**
  * This function will create a new picture using the provided resource.
  */
-VLC_API picture_t * picture_NewFromResource( const video_format_t *, const picture_resource_t * ) VLC_USED;
+VLC_API picture_t * picture_NewFromResource( const video_format_t *, const picture_gc_t *, const picture_resource_t * ) VLC_USED;
 
 /**
  * Destroys a picture without references.
diff --git a/include/vlc_picture_pool.h b/include/vlc_picture_pool.h
index 4f4010f745a..bd362bf1364 100644
--- a/include/vlc_picture_pool.h
+++ b/include/vlc_picture_pool.h
@@ -73,7 +73,7 @@ VLC_API picture_pool_t * picture_pool_NewFromFormat(const video_format_t *fmt,
  * or picture_pool_NewFromFormat().
  *
  * @note If there are no pending references to the pooled pictures, and the
- * picture_resource_t.pf_destroy callback was not NULL, it will be invoked.
+ * picture_gc_t.pf_destroy callback was not NULL, it will be invoked.
  * Otherwise the default callback will be used.
  *
  * @warning If there are pending references (a.k.a. late pictures), the
diff --git a/modules/codec/vt_utils.c b/modules/codec/vt_utils.c
index 55375dc2ff4..a27b42e58f4 100644
--- a/modules/codec/vt_utils.c
+++ b/modules/codec/vt_utils.c
@@ -178,7 +178,7 @@ cvpxpic_create_mapped(const video_format_t *fmt, CVPixelBufferRef cvpx,
     void (*pf_destroy)(picture_context_t *) = readonly ?
         cvpxpic_destroy_mapped_ro_cb : cvpxpic_destroy_mapped_rw_cb;
 
-    picture_t *pic = picture_NewFromResource(fmt, &rsc);
+    picture_t *pic = picture_NewFromResource(fmt, NULL, &rsc);
     if (pic == NULL
      || cvpxpic_attach_common(pic, cvpx, pf_destroy, vctx, NULL) != VLC_SUCCESS)
     {
diff --git a/modules/hw/d3d11/d3d11_surface.c b/modules/hw/d3d11/d3d11_surface.c
index a5fc45ccf2f..f0b2ff6e016 100644
--- a/modules/hw/d3d11/d3d11_surface.c
+++ b/modules/hw/d3d11/d3d11_surface.c
@@ -614,8 +614,8 @@ static picture_t *AllocateCPUtoGPUTexture(filter_t *p_filter, filter_sys_t *p_sy
     video_format_Copy(&fmt_staging, &p_filter->fmt_out.video);
     fmt_staging.i_chroma = cfg->fourcc;
 
-    picture_resource_t dummy_res = {};
-    picture_t *p_dst = picture_NewFromResource(&fmt_staging, &dummy_res);
+    // allocate a picture with the planes setup but no attached buffer
+    picture_t *p_dst = picture_NewFromResource(&fmt_staging, NULL, NULL);
     if (p_dst == NULL) {
         msg_Err(p_filter, "Failed to map create the temporary picture.");
         goto done;
diff --git a/modules/hw/d3d9/dxa9.c b/modules/hw/d3d9/dxa9.c
index 563818dc708..a6f8d9171bd 100644
--- a/modules/hw/d3d9/dxa9.c
+++ b/modules/hw/d3d9/dxa9.c
@@ -347,8 +347,8 @@ static picture_t *AllocateCPUtoGPUTexture(filter_t *p_filter)
     video_format_Copy(&fmt_staging, &p_filter->fmt_out.video);
     fmt_staging.i_chroma = format;
 
-    picture_resource_t dummy_res = {};
-    picture_t *p_dst = picture_NewFromResource(&fmt_staging, &dummy_res);
+    // allocate a picture with the planes setup but no attached buffer
+    picture_t *p_dst = picture_NewFromResource(&fmt_staging, NULL, NULL);
     if (p_dst == NULL) {
         msg_Err(p_filter, "Failed to map create the temporary picture.");
         goto done;
diff --git a/modules/hw/nvdec/nvdec.c b/modules/hw/nvdec/nvdec.c
index acf8e06a15d..3ae4836c04b 100644
--- a/modules/hw/nvdec/nvdec.c
+++ b/modules/hw/nvdec/nvdec.c
@@ -178,12 +178,10 @@ static nvdec_pool_t* nvdec_pool_Create(vlc_video_context *vctx,
         if (ret != CUDA_SUCCESS || pool->outputDevicePtr[i] == 0)
             goto free_pool;
 
-        picture_resource_t res = {
-            .p_sys = pool->outputDevicePtr[i],
-        };
-        pics[i] = picture_NewFromResource(fmt, &res);
+        pics[i] = picture_NewFromResource(fmt, NULL, NULL);
         if (!pics[i])
             goto free_pool;
+        pics[i]->p_sys = (void*)(uintptr_t)pool->outputDevicePtr[i];
     }
 
     pool->picture_pool = picture_pool_New(ARRAY_SIZE(pool->outputDevicePtr), pics);
diff --git a/modules/hw/vaapi/vlc_vaapi.c b/modules/hw/vaapi/vlc_vaapi.c
index 31304fd6c33..1d77a04297a 100644
--- a/modules/hw/vaapi/vlc_vaapi.c
+++ b/modules/hw/vaapi/vlc_vaapi.c
@@ -432,7 +432,7 @@ pool_pic_destroy_cb(picture_t *pic)
                           instance->num_render_targets);
         free(instance);
     }
-    free(pic->p_sys);
+    free(p_sys);
 }
 
 static void
@@ -512,11 +512,8 @@ vlc_vaapi_PoolNew(vlc_object_t *o, vlc_video_context *vctx,
         p_sys->ctx.ctx.surface = instance->render_targets[i];
         p_sys->ctx.ctx.va_dpy = dpy;
         p_sys->ctx.picref = NULL;
-        picture_resource_t rsc = {
-            .p_sys = p_sys,
-            .pf_destroy = pool_pic_destroy_cb,
-        };
-        pics[i] = picture_NewFromResource(fmt, &rsc);
+        picture_gc_t gc = { pool_pic_destroy_cb, p_sys };
+        pics[i] = picture_NewFromResource(fmt, &gc, NULl);
         if (pics[i] == NULL)
         {
             free(p_sys);
diff --git a/modules/hw/vdpau/picture.c b/modules/hw/vdpau/picture.c
index 7f4e3322536..0cc7cbba5c6 100644
--- a/modules/hw/vdpau/picture.c
+++ b/modules/hw/vdpau/picture.c
@@ -169,12 +169,11 @@ error:
         return NULL;
     }
 
-    picture_resource_t res = {
-        .p_sys = sys,
-        .pf_destroy = vlc_vdp_output_surface_destroy,
+    picture_gc_t gc = {
+        vlc_vdp_output_surface_destroy, sys,
     };
 
-    picture_t *pic = picture_NewFromResource(fmt, &res);
+    picture_t *pic = picture_NewFromResource(fmt, &gc, NULL);
     if (unlikely(pic == NULL))
     {
         vdp_output_surface_destroy(vdpau_dev->vdp, sys->surface);
diff --git a/modules/video_chroma/copy.c b/modules/video_chroma/copy.c
index 2ad9d2b7a71..95fc17a3fcb 100644
--- a/modules/video_chroma/copy.c
+++ b/modules/video_chroma/copy.c
@@ -1095,7 +1095,7 @@ static picture_t *pic_new_unaligned(const video_format_t *fmt)
      * from the source picture */
     const vlc_chroma_description_t *dsc = vlc_fourcc_GetChromaDescription(fmt->i_chroma);
     assert(dsc);
-    picture_resource_t rsc = { .pf_destroy = pic_rsc_destroy };
+    picture_resource_t rsc = {};
     for (unsigned i = 0; i < dsc->plane_count; i++)
     {
         rsc.p[i].i_lines = ((fmt->i_visible_height + (dsc->p[i].h.den - 1)) / dsc->p[i].h.den) * dsc->p[i].h.num;
@@ -1103,7 +1103,8 @@ static picture_t *pic_new_unaligned(const video_format_t *fmt)
         rsc.p[i].p_pixels = malloc(rsc.p[i].i_lines * rsc.p[i].i_pitch);
         assert(rsc.p[i].p_pixels);
     }
-    return picture_NewFromResource(fmt, &rsc);
+    picture_gc_t gc = { pic_rsc_destroy, NULL };
+    return picture_NewFromResource(fmt, &gc, &rsc);
 }
 
 int main(void)
diff --git a/modules/video_output/android/display.c b/modules/video_output/android/display.c
index e412bd75bb0..3c657324ce5 100644
--- a/modules/video_output/android/display.c
+++ b/modules/video_output/android/display.c
@@ -185,13 +185,9 @@ static picture_t *PictureAlloc(video_format_t *fmt)
     if (unlikely(p_picsys == NULL))
         return NULL;
 
-    picture_resource_t rsc = {
-        .p_sys = p_picsys
-    };
+    picture_gc_t gc = { AndroidPicture_Destroy, p_picsys };
 
-    rsc.pf_destroy = AndroidPicture_Destroy;
-
-    p_pic = picture_NewFromResource(fmt, &rsc);
+    p_pic = picture_NewFromResource(fmt, &gc, NULL);
     if (!p_pic)
     {
         free(p_picsys);
diff --git a/modules/video_output/fb.c b/modules/video_output/fb.c
index 1538af34659..60910d6dba1 100644
--- a/modules/video_output/fb.c
+++ b/modules/video_output/fb.c
@@ -606,7 +606,7 @@ static int OpenDisplay(vout_display_t *vd, bool force_resolution)
        },
     };
 
-    sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
+    sys->picture = picture_NewFromResource(&vd->fmt, NULL, &rsc);
     if (unlikely(sys->picture == NULL)) {
         munmap(rsc.p[0].p_pixels, sys->video_size);
         ioctl(sys->fd, FBIOPUT_VSCREENINFO, &sys->old_info);
diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
index 8d2e3382c22..a9d24f8a7b1 100644
--- a/modules/video_output/kms.c
+++ b/modules/video_output/kms.c
@@ -584,7 +584,7 @@ static int OpenDisplay(vout_display_t *vd)
         rsc.p[i].i_pitch  = sys->stride;
     }
 
-    sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
+    sys->picture = picture_NewFromResource(&vd->fmt, NULL, &rsc);
 
     if (!sys->picture)
         goto err_out;
diff --git a/modules/video_output/opengl/interop_sw.c b/modules/video_output/opengl/interop_sw.c
index 486b2b3f1dd..5f9c589e4e3 100644
--- a/modules/video_output/opengl/interop_sw.c
+++ b/modules/video_output/opengl/interop_sw.c
@@ -66,11 +66,8 @@ pbo_picture_create(const struct vlc_gl_interop *interop)
     if (unlikely(picsys == NULL))
         return NULL;
 
-    picture_resource_t rsc = {
-        .p_sys = picsys,
-        .pf_destroy = pbo_picture_destroy,
-    };
-    picture_t *pic = picture_NewFromResource(&interop->fmt_out, &rsc);
+    picture_gc_t gc = { pbo_picture_destroy, picsys };
+    picture_t *pic = picture_NewFromResource(&interop->fmt_out, &gc, NULL);
     if (pic == NULL)
     {
         free(picsys);
diff --git a/modules/video_output/vmem.c b/modules/video_output/vmem.c
index 2bdcd63d9e6..90f61363f9e 100644
--- a/modules/video_output/vmem.c
+++ b/modules/video_output/vmem.c
@@ -239,7 +239,7 @@ static void Prepare(vout_display_t *vd, picture_t *pic, subpicture_t *subpic,
 {
     VLC_UNUSED(date);
     vout_display_sys_t *sys = vd->sys;
-    picture_resource_t rsc = { .p_sys = NULL };
+    picture_resource_t rsc = { };
     void *planes[PICTURE_PLANE_MAX];
 
     sys->pic_opaque = sys->lock(sys->opaque, planes);
@@ -250,7 +250,7 @@ static void Prepare(vout_display_t *vd, picture_t *pic, subpicture_t *subpic,
         rsc.p[i].i_pitch  = sys->pitches[i];
     }
 
-    picture_t *locked = picture_NewFromResource(&vd->fmt, &rsc);
+    picture_t *locked = picture_NewFromResource(&vd->fmt, NULL, &rsc);
     if (likely(locked != NULL)) {
         picture_CopyPixels(locked, pic);
         picture_Release(locked);
diff --git a/modules/video_output/win32/direct3d11.c b/modules/video_output/win32/direct3d11.c
index 47eecf9582f..25576cf43b8 100644
--- a/modules/video_output/win32/direct3d11.c
+++ b/modules/video_output/win32/direct3d11.c
@@ -1339,11 +1339,8 @@ static int Direct3D11MapSubpicture(vout_display_t *vd, int *subpicture_region_co
                 free(d3dquad);
                 continue;
             }
-            picture_resource_t picres = {
-                .p_sys      = (picture_sys_d3d11_t *) d3dquad,
-                .pf_destroy = DestroyPictureQuad,
-            };
-            (*region)[i] = picture_NewFromResource(&r->p_picture->format, &picres);
+            picture_gc_t gc = { DestroyPictureQuad, d3dquad };
+            (*region)[i] = picture_NewFromResource(&r->p_picture->format, &gc, NULL);
             if ((*region)[i] == NULL) {
                 msg_Err(vd, "Failed to create %dx%d picture for OSD",
                         r->fmt.i_width, r->fmt.i_height);
diff --git a/src/misc/picture.c b/src/misc/picture.c
index 8d3d4f80be4..0ee2e102417 100644
--- a/src/misc/picture.c
+++ b/src/misc/picture.c
@@ -202,10 +202,9 @@ static bool picture_InitPrivate(const video_format_t *restrict p_fmt,
     return true;
 }
 
-picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_resource_t *p_resource )
+picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_gc_t *gc,
+                                    const picture_resource_t *p_resource )
 {
-    assert(p_resource != NULL);
-
     picture_priv_t *priv = malloc(sizeof(*priv));
     if (unlikely(priv == NULL))
         return NULL;
@@ -216,21 +215,28 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
         return NULL;
     }
 
-    priv->gc = (picture_gc_t) {
-        p_resource->pf_destroy,
-        p_resource->p_sys,
-    };
-
     picture_t *p_picture = &priv->picture;
 
-    p_picture->p_sys = p_resource->p_sys;
+    if (gc != NULL)
+    {
+        priv->gc = *gc;
+        p_picture->p_sys = gc->opaque;
+    }
+    else
+    {
+        priv->gc = (picture_gc_t) { NULL, NULL };
+        p_picture->p_sys = NULL;
+    }
 
+    if (p_resource != NULL)
+    {
     for( int i = 0; i < p_picture->i_planes; i++ )
     {
         p_picture->p[i].p_pixels = p_resource->p[i].p_pixels;
         p_picture->p[i].i_lines  = p_resource->p[i].i_lines;
         p_picture->p[i].i_pitch  = p_resource->p[i].i_pitch;
     }
+    }
 
     return p_picture;
 }
diff --git a/src/misc/picture.h b/src/misc/picture.h
index 0afaca95f44..a4df027832c 100644
--- a/src/misc/picture.h
+++ b/src/misc/picture.h
@@ -23,12 +23,6 @@
 
 #include <vlc_picture.h>
 
-typedef struct
-{
-    void (*destroy)(picture_t *);
-    void *opaque;
-} picture_gc_t;
-
 typedef struct
 {
     picture_t    picture;
-- 
2.26.2



More information about the vlc-devel mailing list