[vlc-devel] [PATCH 2/2] vda: avoid bad access when getting core video buffer.

[Sebastien Zwickert]

On Jul 24, 2013, at 10:52 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:

> Le mercredi 24 juillet 2013 16:08:18 Sebastien Zwickert a écrit :
>> The use of reference counting for video decoder introduced
>> by commit b217f60be52bfb3b9f03d7a38f261f84c3218365 causes
>> a bad access in avcodec/vda module.
>> ---
>> modules/codec/avcodec/vda.c |    4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/modules/codec/avcodec/vda.c b/modules/codec/avcodec/vda.c
>> index 76db798..13b4a64 100644
>> --- a/modules/codec/avcodec/vda.c
>> +++ b/modules/codec/avcodec/vda.c
>> @@ -258,10 +258,14 @@ static int Extract( vlc_va_t *external, picture_t
>> *p_picture, AVFrame *p_ff ) static void Release( vlc_va_t *external,
>> AVFrame *p_ff )
>> {
>>     VLC_UNUSED( external );
>> +#if LIBAVCODEC_VERSION_MAJOR >= 55
>> +    VLC_UNUSED( p_ff );
>> +#else
>>     CVPixelBufferRef cv_buffer = ( CVPixelBufferRef )p_ff->data[3];
>> 
>>     if ( cv_buffer )
>>         CVPixelBufferRelease( cv_buffer );
>> +#endif
> 
> This code makes no sense, even on older libavcodec versions and should be 
> rewritten. Somehow I doubt that (void *)1 is a valid CVPixelBufferRef.

Before the use of buffer reference-counted in libavcodec the VDA clients have
to release themself the core video buffer they get from ->data[3]. So, this release
callback was a good place to free this buffer.
I wrote this patch in a hurry and I forgot that the core video buffer is now released
in Get() to fix memory leaks when linking against libavcodec with reference-counted
buffers. So this code could be removed too on older libavcodec versions.

> Normally, Get() sets ->data[3] to the hardware surface handle, ->data[0] to 
> something non-NULL (preferably the same as ->data[3]), and optionally opaque 
> to plugin internal data.
> 
> Extract() copies the hardware surface from the AVFrame into a VLC picture.
> 
> Release() destroys the hardware surface allocated in Get().
> 
>> }
>> 
>> static void Close( vlc_va_t *external )
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel