[vlc-devel] [PATCH v2 04/10] core: use refcounter helper for pictures

[Steve Lhomme]

On 2018-07-02 17:07, Romain Vimont wrote:
> On Mon, Jul 02, 2018 at 04:58:30PM +0200, Steve Lhomme wrote:
>> On 2018-07-02 16:49, Romain Vimont wrote:
>>> ---
>>>    src/misc/picture.c | 9 +++------
>>>    src/misc/picture.h | 5 ++---
>>>    2 files changed, 5 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/misc/picture.c b/src/misc/picture.c
>>> index e633cee42bc..5cabd420aab 100644
>>> --- a/src/misc/picture.c
>>> +++ b/src/misc/picture.c
>>> @@ -205,7 +205,7 @@ static picture_priv_t *picture_NewPrivate(const video_format_t *restrict p_fmt)
>>>            return NULL;
>>>        }
>>> -    atomic_init( &priv->gc.refs, 1 );
>>> +    vlc_atomic_rc_init( &priv->gc.rc );
>>>        priv->gc.opaque = NULL;
>>>        return priv;
>>> @@ -305,8 +305,7 @@ picture_t *picture_Hold( picture_t *p_picture )
>>>        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 );
>>> +    vlc_atomic_rc_inc( &priv->gc.rc );
>> No way to get an assert here ? Or it can't happen given the asserts in the
>> refcount API ?
> The assert has been moved into vlc_atomic_rc_inc() (patch 1).

Alright so LGTM

>
>>>        return p_picture;
>>>    }
>>> @@ -315,9 +314,7 @@ void picture_Release( picture_t *p_picture )
>>>        assert( p_picture != NULL );
>>>        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 )
>>> +    if( !vlc_atomic_rc_dec( &priv->gc.rc ) )
>>>            return;
>>>        PictureDestroyContext( p_picture );
>>> diff --git a/src/misc/picture.h b/src/misc/picture.h
>>> index 70ee64878da..4a70b6a6c53 100644
>>> --- a/src/misc/picture.h
>>> +++ b/src/misc/picture.h
>>> @@ -18,8 +18,7 @@
>>>     * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>>>     *****************************************************************************/
>>> -#include <stdatomic.h>
>>> -
>>> +#include <vlc_atomic.h>
>>>    #include <vlc_picture.h>
>>>    typedef struct
>>> @@ -27,7 +26,7 @@ typedef struct
>>>        picture_t picture;
>>>        struct
>>>        {
>>> -        atomic_uintptr_t refs;
>>> +        vlc_atomic_rc_t rc;
>>>            void (*destroy)(picture_t *);
>>>            void *opaque;
>>>        } gc;
>>> -- 
>>> 2.18.0
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel