[vlc-devel] [PATCH 2/2] picture: factorize the code to clone a picture

[Steve Lhomme]

Also document why gc.opaque is a picture when the destroy is not
picture_pool_ReleasePicture.
---
 src/misc/picture.c      | 15 +++++++++++----
 src/misc/picture.h      |  2 ++
 src/misc/picture_pool.c | 18 ++----------------
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/src/misc/picture.c b/src/misc/picture.c
index a49add7355d..5a7158fe0bc 100644
--- a/src/misc/picture.c
+++ b/src/misc/picture.c
@@ -420,12 +420,12 @@ static void picture_DestroyClone(picture_t *clone)
     picture_Release(picture);
 }
 
-picture_t *picture_Clone(picture_t *picture)
+picture_t *picture_InternalClone(picture_t *picture,
+                                 void (*pf_destroy)(picture_t *), void *opaque)
 {
-    /* TODO: merge common code with picture_pool_ClonePicture(). */
     picture_resource_t res = {
         .p_sys = picture->p_sys,
-        .pf_destroy = picture_DestroyClone,
+        .pf_destroy = pf_destroy,
     };
 
     for (int i = 0; i < picture->i_planes; i++) {
@@ -436,9 +436,16 @@ picture_t *picture_Clone(picture_t *picture)
 
     picture_t *clone = picture_NewFromResource(&picture->format, &res);
     if (likely(clone != NULL)) {
-        ((picture_priv_t *)clone)->gc.opaque = picture;
+        ((picture_priv_t *)clone)->gc.opaque = opaque;
         picture_Hold(picture);
+    }
+    return clone;
+}
 
+picture_t *picture_Clone(picture_t *picture)
+{
+    picture_t *clone = picture_InternalClone(picture, picture_DestroyClone, picture);
+    if (likely(clone != NULL)) {
         if (picture->context != NULL)
             clone->context = picture->context->copy(picture->context);
     }
diff --git a/src/misc/picture.h b/src/misc/picture.h
index a87164ce866..684eb2ada69 100644
--- a/src/misc/picture.h
+++ b/src/misc/picture.h
@@ -35,3 +35,5 @@ typedef struct
 
 void *picture_Allocate(int *, size_t);
 void picture_Deallocate(int, void *, size_t);
+
+picture_t * picture_InternalClone(picture_t *, void (*pf_destroy)(picture_t *), void *);
diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
index 7015f07fa07..18b1d86918c 100644
--- a/src/misc/picture_pool.c
+++ b/src/misc/picture_pool.c
@@ -95,23 +95,8 @@ static picture_t *picture_pool_ClonePicture(picture_pool_t *pool,
 {
     picture_t *picture = pool->picture[offset];
     uintptr_t sys = ((uintptr_t)pool) + offset;
-    picture_resource_t res = {
-        .p_sys = picture->p_sys,
-        .pf_destroy = picture_pool_ReleasePicture,
-    };
-
-    for (int i = 0; i < picture->i_planes; i++) {
-        res.p[i].p_pixels = picture->p[i].p_pixels;
-        res.p[i].i_lines = picture->p[i].i_lines;
-        res.p[i].i_pitch = picture->p[i].i_pitch;
-    }
 
-    picture_t *clone = picture_NewFromResource(&picture->format, &res);
-    if (likely(clone != NULL)) {
-        ((picture_priv_t *)clone)->gc.opaque = (void *)sys;
-        picture_Hold(picture);
-    }
-    return clone;
+    return picture_InternalClone(picture, picture_pool_ReleasePicture, sys);
 }
 
 picture_pool_t *picture_pool_NewExtended(const picture_pool_configuration_t *cfg)
@@ -295,6 +280,7 @@ bool picture_pool_OwnsPic(picture_pool_t *pool, picture_t *pic)
         if (priv->gc.opaque == NULL)
             return false; /* not a pooled picture */
 
+        /* cloned picture from picture_Clone() */
         pic = priv->gc.opaque;
         priv = (picture_priv_t *)pic;
     }
-- 
2.17.1