[vlc-devel] [vlc-commits] decoder: add Destroy helper function for decoder owners

[Steve Lhomme]

On 18/02/2019 16:32, Thomas Guillem wrote:
> On Mon, Feb 18, 2019, at 16:11, Steve Lhomme wrote:
>> vlc | branch: master | Steve Lhomme <robux4 at ycbcr.xyz> | Mon Feb 18
>> 14:50:21 2019 +0100| [4a79e6a4929085cd78024e0c0367c2bc01ab11b4] |
>> committer: Steve Lhomme
>>
>> decoder: add Destroy helper function for decoder owners
>>
>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=4a79e6a4929085cd78024e0c0367c2bc01ab11b4
>> ---
>>
>>   include/vlc_codec.h         |  7 +++++++
>>   src/input/decoder_helpers.c | 29 +++++++++++++++++++++++++++++
>>   src/libvlccore.sym          |  1 +
>>   3 files changed, 37 insertions(+)
>>
>> diff --git a/include/vlc_codec.h b/include/vlc_codec.h
>> index c287953ba7..2042ad2424 100644
>> --- a/include/vlc_codec.h
>> +++ b/include/vlc_codec.h
>> @@ -320,6 +320,13 @@ VLC_API void decoder_AbortPictures( decoder_t
>> *dec, bool b_abort );
>>   VLC_API void decoder_Init( decoder_t *dec, const es_format_t * );
>>   
>>   /**
>> + * Destroy a decoder and reset the structure.
>> + *
>> + * To be used by decoder owners.
>> + */
>> +VLC_API void decoder_Destroy( decoder_t **p_dec );
>> +
>> +/**
>>    * This function queues a single picture to the video output.
>>    *
>>    * \note
>> diff --git a/src/input/decoder_helpers.c b/src/input/decoder_helpers.c
>> index 51f7ce8f6d..b60b830123 100644
>> --- a/src/input/decoder_helpers.c
>> +++ b/src/input/decoder_helpers.c
>> @@ -28,6 +28,8 @@
>>   
>>   #include <vlc_common.h>
>>   #include <vlc_codec.h>
>> +#include <vlc_meta.h>
>> +#include <vlc_modules.h>
>>   
>>   void decoder_Init( decoder_t *p_dec, const es_format_t *restrict p_fmt
>> )
>>   {
>> @@ -43,3 +45,30 @@ void decoder_Init( decoder_t *p_dec, const
>> es_format_t *restrict p_fmt )
>>       es_format_Copy( &p_dec->fmt_in, p_fmt );
>>       es_format_Init( &p_dec->fmt_out, p_fmt->i_cat, 0 );
>>   }
>> +
>> +static void decoder_Clean( decoder_t *p_dec )
>> +{
>> +    es_format_Clean( &p_dec->fmt_in );
>> +    es_format_Clean( &p_dec->fmt_out );
>> +
>> +    if ( p_dec->p_description )
>> +    {
>> +        vlc_meta_Delete(p_dec->p_description);
>> +        p_dec->p_description = NULL;
>> +    }
>> +    if ( p_dec->p_module != NULL )
>> +    {
>> +        module_unneed(p_dec, p_dec->p_module);
>> +        p_dec->p_module = NULL;
>> +    }
>> +}
>> +
>> +void decoder_Destroy( decoder_t **p_dec )
>> +{
>> +    if (*p_dec != NULL)
>> +    {
>> +        decoder_Clean( *p_dec );
>> +        vlc_object_release( *p_dec );
>> +        *p_dec = NULL;
>> +    }
> I really prefer to use a decoder_t * here.
> There is not many functions in VLC that set the pointer to NULL. This will add more confusion.
>
> I really prefer having a crash (via assert/ASAN) when a client is misusing an API (calling Destroy with a freed pointer) instead of ignoring it.

A double free doesn't always crash on Windows and there's no ASAN. So 
don't blame me where there are leaks.

I will update my code to use the old way.