[vlc-devel] [vlc-commits] decoder: add Destroy helper function for decoder owners
Steve Lhomme
robux4 at ycbcr.xyz
Tue Feb 19 08:01:46 CET 2019
> There is not many functions in VLC that set the pointer to NULL.
Because we didn't do it so far doesn't mean we should never do it, nor
that it's a bad thing.
> This will add more confusion.
Maybe another name than Destroy would remove some of that 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.
This is true when calling Destroy twice is a bug. It isn't with this
patch. What this actually proves is that it would be one less kind of
bug that we need to take care of closely.
On 19/02/2019 07:50, Rémi Denis-Courmont wrote:
> 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é.
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list