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

[Rémi Denis-Courmont]

We've already been through this too. We  have why it is BS in the past and Thomas summed up some of the arguments again as well.

Le 19 février 2019 08:44:25 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>We've been through this already. Calling a technical solution bullshit
>with no other argument is not useful in any way.
>
>As for the solution itself, so far noone convinced me that writing less
>code to do more checks is a bad thing. But I removed this feature for
>now since the majority is against that solution.
>
>> On 18 Feb 2019, at 17:38, Rémi Denis-Courmont <remi at remlab.net>
>wrote:
>> 
>> Le maanantaina 18. helmikuuta 2019, 17.52.32 EET Steve Lhomme a écrit
>:
>>> 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=4a79e6a4929085cd7
>>>>>> 8024e0c0367c2bc01ab11b4>> 
>>>>> ---
>>>>> 
>>>>>  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 your point is that because the bug is hard to observe on Windows,
>it should 
>> be made hard to observe everywhere? This is punishing everybody for
>your 
>> personal selection of development environment. What the hell...
>> 
>> -- 
>> 雷米‧德尼-库尔蒙
>> http://www.remlab.net/
>> 
>> 
>> 
>> _______________________________________________
>> 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

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190219/b48770a6/attachment.html>