[vlc-commits] picture: partially inline and relax picture_Release()

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Mon Dec 10 23:48:10 2018 +0200| [128fdbb8d4b30d31b17e3ca9dc64090995efc8df] | committer: Rémi Denis-Courmont

picture: partially inline and relax picture_Release()

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=128fdbb8d4b30d31b17e3ca9dc64090995efc8df
---

 include/vlc_picture.h | 25 ++++++++++++++++++++++---
 src/libvlccore.sym    |  2 +-
 src/misc/picture.c    | 22 ++++++++++------------
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/include/vlc_picture.h b/include/vlc_picture.h
index 3629aa4d72..d71e177713 100644
--- a/include/vlc_picture.h
+++ b/include/vlc_picture.h
@@ -34,6 +34,7 @@
 using std::atomic_uintptr_t;
 using std::memory_order;
 using std::memory_order_relaxed;
+using std::memory_order_release;
 #endif
 
 /**
@@ -168,6 +169,14 @@ typedef struct
 VLC_API picture_t * picture_NewFromResource( const video_format_t *, const picture_resource_t * ) VLC_USED;
 
 /**
+ * Destroys a picture without references.
+ *
+ * This function destroys a picture with zero references left.
+ * Never call this function directly. Use picture_Release() instead.
+ */
+VLC_API void picture_Destroy(picture_t *picture);
+
+/**
  * Increments the picture reference count.
  *
  * \return picture
@@ -180,10 +189,20 @@ static inline picture_t *picture_Hold(picture_t *picture)
 }
 
 /**
- * This function will release a picture.
- * It will not have any effect on picture obtained from vout
+ * Decrements the picture reference count.
+ *
+ * If the reference count reaches zero, the picture is destroyed. If it was
+ * allocated from a pool, the underlying picture buffer will be returned to the
+ * pool. Otherwise, the picture buffer will be freed.
  */
-VLC_API void picture_Release( picture_t *p_picture );
+static inline void picture_Release(picture_t *picture)
+{
+    uintptr_t refs = atomic_fetch_sub_explicit(&picture->refs, (uintptr_t)1,
+                                               memory_order_release);
+    vlc_assert(refs > 0);
+    if (refs == 1)
+        picture_Destroy(picture);
+}
 
 /**
  * This function will copy all picture dynamic properties.
diff --git a/src/libvlccore.sym b/src/libvlccore.sym
index c0c6429dba..db68d712c9 100644
--- a/src/libvlccore.sym
+++ b/src/libvlccore.sym
@@ -298,7 +298,7 @@ NTPtime64
 picture_BlendSubpicture
 picture_Clone
 picture_CopyPixels
-picture_Release
+picture_Destroy
 picture_CopyProperties
 picture_Copy
 picture_Export
diff --git a/src/misc/picture.c b/src/misc/picture.c
index 924d2ca960..be6de71ea7 100644
--- a/src/misc/picture.c
+++ b/src/misc/picture.c
@@ -62,7 +62,7 @@ static void picture_DestroyFromResource( picture_t *p_picture )
 /**
  * Destroys a picture allocated with picture_NewFromFormat().
  */
-static void picture_Destroy(picture_t *pic)
+static void picture_DestroyFromFormat(picture_t *pic)
 {
     picture_buffer_t *res = pic->p_sys;
 
@@ -257,7 +257,7 @@ picture_t *picture_NewFromFormat(const video_format_t *restrict fmt)
     if (unlikely(priv == NULL))
         return NULL;
 
-    priv->gc.destroy = picture_Destroy;
+    priv->gc.destroy = picture_DestroyFromFormat;
 
     picture_t *pic = &priv->picture;
     if (pic->i_planes == 0) {
@@ -320,19 +320,17 @@ picture_t *picture_New( vlc_fourcc_t i_chroma, int i_width, int i_height, int i_
  *
  *****************************************************************************/
 
-void picture_Release( picture_t *p_picture )
+void picture_Destroy(picture_t *picture)
 {
-    assert( p_picture != NULL );
+    /* See changes from other threads */
+    atomic_thread_fence(memory_order_acquire);
+    assert(atomic_load_explicit(&picture->refs, memory_order_relaxed) == 0);
 
-    picture_priv_t *priv = (picture_priv_t *)p_picture;
-    uintptr_t refs = atomic_fetch_sub(&p_picture->refs, 1);
-    assert( refs != 0 );
-    if( refs > 1 )
-        return;
+    PictureDestroyContext(picture);
 
-    PictureDestroyContext( p_picture );
-    assert( priv->gc.destroy != NULL );
-    priv->gc.destroy( p_picture );
+    picture_priv_t *priv = container_of(picture, picture_priv_t, picture);
+    assert(priv->gc.destroy != NULL);
+    priv->gc.destroy(picture);
 }
 
 /*****************************************************************************