picture: privatize reference counting

include/vlc_picture.h

@@ -32,7 +32,6 @@
  */
 
 #include <vlc_es.h>
-#include <vlc_atomic.h>
 
 /** Description of a planar graphic field */
 typedef struct plane_t
@@ -99,14 +98,6 @@ struct picture_t
      * keep track of the picture */
     picture_sys_t * p_sys;
 
-    /** This way the picture_Release can be overloaded */
-    struct
-    {
-        atomic_uintptr_t refcount;
-        void (*pf_destroy)( picture_t * );
-        picture_gc_sys_t *p_sys;
-    } gc;
-
     /** Next picture in a FIFO a pictures */
     struct picture_t *p_next;
 };

src/Makefile.am

@@ -447,6 +447,7 @@ SOURCES_libvlc_common = \
 	misc/fourcc_list.h \
 	misc/es_format.c \
 	misc/picture.c \
+	misc/picture.h \
 	misc/picture_fifo.c \
 	misc/picture_pool.c \
 	modules/modules.h \

src/misc/picture.c

@@ -34,7 +34,7 @@
 #include <assert.h>
 
 #include <vlc_common.h>
-#include <vlc_picture.h>
+#include "picture.h"
 #include <vlc_image.h>
 #include <vlc_block.h>
 
@@ -148,9 +148,6 @@ int picture_Setup( picture_t *p_picture, const video_format_t *restrict fmt )
         p->i_pixel_pitch = 0;
     }
 
-    atomic_init( &p_picture->gc.refcount, 0 );
-    p_picture->gc.p_sys = NULL;
-
     p_picture->i_nb_fields = 2;
 
     video_format_Setup( &p_picture->format, fmt->i_chroma, fmt->i_width, fmt->i_height,
@@ -219,10 +216,15 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
         video_format_CopyCrop( &fmt, p_fmt );
 
     /* */
-    picture_t *p_picture = calloc( 1, sizeof(*p_picture) );
-    if( !p_picture )
+    picture_priv_t *priv = malloc( sizeof (*priv) );
+    if( unlikely(priv == NULL) )
         return NULL;
 
+    picture_t *p_picture = &priv->picture;
+
+    memset( p_picture, 0, sizeof( *p_picture ) );
+    p_picture->format = fmt;
+
     /* Make sure the real dimensions are a multiple of 16 */
     if( picture_Setup( p_picture, &fmt ) )
     {
@@ -230,12 +232,17 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
         return NULL;
     }
 
+    atomic_init( &priv->gc.refs, 1 );
+    priv->gc.opaque = NULL;
+
     if( p_resource )
     {
         p_picture->p_sys = p_resource->p_sys;
-        p_picture->gc.pf_destroy = p_resource->pf_destroy;
-        if( p_picture->gc.pf_destroy == NULL )
-            p_picture->gc.pf_destroy = picture_DestroyFromResource;
+
+        if( p_resource->pf_destroy != NULL )
+            priv->gc.destroy = p_resource->pf_destroy;
+        else
+            priv->gc.destroy = picture_DestroyFromResource;
 
         for( int i = 0; i < p_picture->i_planes; i++ )
         {
@@ -251,14 +258,9 @@ picture_t *picture_NewFromResource( const video_format_t *p_fmt, const picture_r
             free( p_picture );
             return NULL;
         }
-        p_picture->gc.pf_destroy = picture_Destroy;
+        priv->gc.destroy = picture_Destroy;
     }
 
-    /* */
-    p_picture->format = fmt;
-
-    atomic_init( &p_picture->gc.refcount, 1 );
-
     return p_picture;
 }
 
@@ -284,7 +286,10 @@ picture_t *picture_New( vlc_fourcc_t i_chroma, int i_width, int i_height, int i_
 
 picture_t *picture_Hold( picture_t *p_picture )
 {
-    uintptr_t refs = atomic_fetch_add( &p_picture->gc.refcount, 1 );
+    assert( p_picture != NULL );
+
+    picture_priv_t *priv = (picture_priv_t *)p_picture;
+    uintptr_t refs = atomic_fetch_add( &priv->gc.refs, 1 );
     assert( refs > 0 );
     return p_picture;
 }
@@ -293,19 +298,22 @@ void picture_Release( picture_t *p_picture )
 {
     assert( p_picture != NULL );
 
-    uintptr_t refs = atomic_fetch_sub( &p_picture->gc.refcount, 1 );
+    picture_priv_t *priv = (picture_priv_t *)p_picture;
+    uintptr_t refs = atomic_fetch_sub( &priv->gc.refs, 1 );
     assert( refs != 0 );
     if( refs > 1 )
         return;
 
     PictureDestroyContext( p_picture );
-    assert( p_picture->gc.pf_destroy != NULL );
-    p_picture->gc.pf_destroy( p_picture );
+    assert( priv->gc.destroy != NULL );
+    priv->gc.destroy( p_picture );
 }
 
 bool picture_IsReferenced( picture_t *p_picture )
 {
-    return atomic_load( &p_picture->gc.refcount ) > 1;
+    picture_priv_t *priv = (picture_priv_t *)p_picture;
+
+    return atomic_load( &priv->gc.refs ) > 1;
 }
 
 /*****************************************************************************

src/misc/picture.h

+/*****************************************************************************
+ * picture.h: picture internals
+ *****************************************************************************
+ * Copyright (C) 2015 Remi Denis-Courmont
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#include <vlc_picture.h>
+#include <vlc_atomic.h>
+
+typedef struct
+{
+    picture_t picture;
+    struct
+    {
+        atomic_uintptr_t refs;
+        void (*destroy)(picture_t *);
+        picture_gc_sys_t *opaque;
+    } gc;
+} picture_priv_t;

src/misc/picture_pool.c

@@ -33,6 +33,7 @@
 
 #include <vlc_common.h>
 #include <vlc_picture_pool.h>
+#include "picture.h"
 
 /*****************************************************************************
  *
@@ -70,10 +71,10 @@ void picture_pool_Release(picture_pool_t *pool)
 
     for (unsigned i = 0; i < pool->picture_count; i++) {
         picture_t *picture = pool->picture[i];
-        picture_gc_sys_t *sys = picture->gc.p_sys;
+        picture_priv_t *priv = (picture_priv_t *)picture;
 
-        picture_Release(sys->picture);
-        free(sys);
+        picture_Release(priv->gc.opaque->picture);
+        free(priv->gc.opaque);
         free(picture);
     }
 
@@ -84,7 +85,8 @@ void picture_pool_Release(picture_pool_t *pool)
 
 static void picture_pool_ReleasePicture(picture_t *picture)
 {
-    picture_gc_sys_t *sys = picture->gc.p_sys;
+    picture_priv_t *priv = (picture_priv_t *)picture;
+    picture_gc_sys_t *sys = priv->gc.opaque;
     picture_pool_t *pool = sys->pool;
 
     if (pool->pic_unlock != NULL)
@@ -123,7 +125,7 @@ static picture_t *picture_pool_ClonePicture(picture_pool_t *pool,
 
     picture_t *clone = picture_NewFromResource(&picture->format, &res);
     if (likely(clone != NULL))
-        clone->gc.p_sys = sys;
+        ((picture_priv_t *)clone)->gc.opaque = sys;
     else
         free(sys);
 
@@ -163,7 +165,7 @@ picture_pool_t *picture_pool_NewExtended(const picture_pool_configuration_t *cfg
         if (unlikely(picture == NULL))
             abort();
 
-        atomic_init(&picture->gc.refcount, 0);
+        atomic_init(&((picture_priv_t *)picture)->gc.refs, 0);
 
         pool->picture[i] = picture;
     }
@@ -236,7 +238,8 @@ picture_t *picture_pool_Get(picture_pool_t *pool)
 
     for (unsigned i = 0; i < pool->picture_count; i++) {
         picture_t *picture = pool->picture[i];
-        picture_gc_sys_t *sys = picture->gc.p_sys;
+        picture_priv_t *priv = (picture_priv_t *)picture;
+        picture_gc_sys_t *sys = priv->gc.opaque;
         uint64_t tick;
 
         if (sys->in_use)
@@ -256,8 +259,8 @@ picture_t *picture_pool_Get(picture_pool_t *pool)
 
         sys->tick = tick;
 
-        assert(atomic_load(&picture->gc.refcount) == 0);
-        atomic_init(&picture->gc.refcount, 1);
+        assert(atomic_load(&((picture_priv_t *)picture)->gc.refs) == 0);
+        atomic_init(&((picture_priv_t *)picture)->gc.refs, 1);
         picture->p_next = NULL;
         return picture;
     }
@@ -275,7 +278,8 @@ retry:
 
     for (unsigned i = 0; i < pool->picture_count; i++) {
         picture_t *picture = pool->picture[i];
-        picture_gc_sys_t *sys = picture->gc.p_sys;
+        picture_priv_t *priv = (picture_priv_t *)picture;
+        picture_gc_sys_t *sys = priv->gc.opaque;
 
         if (sys->in_use) {
             vlc_mutex_unlock(&pool->lock);