[vlc-devel] [vlc-commits] decoder: make decoder_Clean() public
Thomas Guillem
thomas at gllm.fr
Wed Feb 20 09:56:53 CET 2019
Please, send future decoder patches on the ML first.
This part is really tricky. You can see, that in the past, Rémi and me always went through the ML when touching the decoder (and we disagreed a lot).
On Wed, Feb 20, 2019, at 09:10, Steve Lhomme wrote:
> On 20/02/2019 09:01, Steve Lhomme wrote:
> > On 19/02/2019 15:10, Rémi Denis-Courmont wrote:
> >> Hi,
> >>
> >> I violently agree that there should be a clear separation between the
> >> generic ("owner") functions, and the ES output functions.
> >>
> >> However, I think it makes more sense to keep the (vlc_)dec(oder)_
> >> prefix for the generic case, and (vlc_)input_dec(oder)_ prefix for
> >> the ES output.
I see 3 different decoder API:
- vlc_dec_: used by decoder modules (currently: decoder_*())
- vlc_input_dec: used by es_out.c and implemented in input/decoder.c (but could be used by other modules) (currently: input_Decoder*())
- vlc_dec_owner: your new helpers that could be used by modules. (currently (decoder_*())
Maybe we don't need vlc_dec_owner and always go through vlc_input_dec for every modules.
> >
> > I actually created a separate C file so they don't end up being mixed.
> > But it's still a WIP. I've been bitten hard while working on this by
> > assuming I could put stuff (like the pool and the context) in the
> > decoder owner.
>
> The reason for this is that a lot of things that should be generic for
> all decoder use (flushing, delay, pause, preroll, mouse, etc) are
> actually tied to a particular """owner""". This should change. But I
> haven't worked on that.
>
> > But I did not realize the "owner" is not an owner. It's the actual
> > module. It's created as a vlc_object and never released as such,
> > because done on the decoder which is actually its own owner. And then
> > one wonder why I'd like to reset objects to NULL after being released...
Again with the owner. You know that it is like that since the beginning ?
I just moved it for 3.0 from dec->owner to owner.dec. This didn't change anything: input/decoder.c function were always incompatible with own module decoder implementation.
>
>
> >
> > As for the Load/Destroy/Init/Unload naming, I don't really care. This
> > can be changed at any time.
>
> For the separation between actual decoder and owner related code I agree
> with Remi. It's better to keep (vlc_)decoder_ for the generic decoder
> code and input_dec_ for what constitutes most of decoder.c.
>
> >
> >>
> >> Le 19 février 2019 14:41:48 GMT+02:00, Thomas Guillem
> >> <thomas at gllm.fr> a écrit :
> >>
> >> Why not calling it decoder_Unload() since its mean goal is to
> >> unload the decoder module.
> >>
> >> You should really use an other prefix for decoder owner
> >> functions. I really don't like having the the same prefix for
> >> functions that can only be used by the owner and some by module
> >> implementations.
> >>
> >> vlc_dec_owner, for example.
> >>
> >> On Mon, Feb 18, 2019, at 16:11, Steve Lhomme wrote:
> >>
> >> vlc | branch: master | Steve Lhomme <robux4 at ycbcr.xyz> | Mon
> >> Feb 18 15:02:10 2019 +0100|
> >> [55ae595eb5e80b4fc66de90803e802e2aa70a77f] | committer: Steve
> >> Lhomme decoder: make decoder_Clean() public The decoder and
> >> decoder tests actually load/unload decoders without resetting
> >> the whole structure.
> >>
> >> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=55ae595eb5e80b4fc66de90803e802e2aa70a77f
> >>
> >>
> >> ------------------------------------------------------------------------
> >> include/vlc_codec.h | 7 +++++++ src/input/decoder.c | 32
> >> +++++--------------------------- src/input/decoder_helpers.c |
> >> 2 +- src/libvlccore.sym | 1 + test/src/input/decoder.c | 20
> >> ++------------------ 5 files changed, 16 insertions(+), 46
> >> deletions(-) diff --git a/include/vlc_codec.h
> >> b/include/vlc_codec.h index 2042ad2424..3961b8c9a1 100644 ---
> >> a/include/vlc_codec.h +++ b/include/vlc_codec.h @@ -327,6
> >> +327,13 @@ VLC_API void decoder_Init( decoder_t *dec, const
> >> es_format_t * ); VLC_API void decoder_Destroy( decoder_t
> >> **p_dec ); /** + * Unload a decoder module and reset the
> >> input/output formats. + * + * To be used by decoder owners. +
> >> */ +VLC_API void decoder_Clean( decoder_t *p_dec ); + +/** *
> >> This function queues a single picture to the video output. * *
> >> \note diff --git a/src/input/decoder.c b/src/input/decoder.c
> >> index 05ba7f6abb..582a2eaf2e 100644 --- a/src/input/decoder.c
> >> +++ b/src/input/decoder.c @@ -189,38 +189,16 @@ static int
> >> LoadDecoder( decoder_t *p_dec, bool b_packetizer, if(
> >> !p_dec->p_module ) { - es_format_Clean( &p_dec->fmt_in ); +
> >> decoder_Clean( p_dec ); return -1; } - else - return 0; -} -
> >> -/** - * Unload a decoder module - */ -static void
> >> UnloadDecoder( decoder_t *p_dec ) -{ - if( p_dec->p_module ) -
> >> { - module_unneed( p_dec, p_dec->p_module ); - p_dec->p_module
> >> = NULL; - } - - if( p_dec->p_description ) - { -
> >> vlc_meta_Delete( p_dec->p_description ); -
> >> p_dec->p_description = NULL; - } - - es_format_Clean(
> >> &p_dec->fmt_in ); - es_format_Clean( &p_dec->fmt_out ); +
> >> return 0; } static int ReloadDecoder( decoder_t *p_dec, bool
> >> b_packetizer, const es_format_t *restrict p_fmt, enum reload
> >> reload ) { - /* Copy p_fmt since it can be destroyed by
> >> UnloadDecoder */ + /* Copy p_fmt since it can be destroyed by
> >> decoder_Clean */ struct decoder_owner *p_owner =
> >> dec_get_owner( p_dec ); es_format_t fmt_in; if(
> >> es_format_Copy( &fmt_in, p_fmt ) != VLC_SUCCESS ) @@ -230,7
> >> +208,7 @@ static int ReloadDecoder( decoder_t *p_dec, bool
> >> b_packetizer, } /* Restart the decoder module */ -
> >> UnloadDecoder( p_dec ); + decoder_Clean( p_dec );
> >> p_owner->error = false; if( reload == RELOAD_DECODER_AOUT ) @@
> >> -1974,7 +1952,7 @@ static void DeleteDecoder( decoder_t *
> >> p_dec ) (char*)&p_dec->fmt_in.i_codec ); const enum
> >> es_format_category_e i_cat =p_dec->fmt_in.i_cat; -
> >> UnloadDecoder( p_dec ); + decoder_Clean( p_dec ); /* Free all
> >> packets still in the decoder fifo. */ block_FifoRelease(
> >> p_owner->p_fifo ); diff --git a/src/input/decoder_helpers.c
> >> b/src/input/decoder_helpers.c index b60b830123..693c6b3955
> >> 100644 --- a/src/input/decoder_helpers.c +++
> >> b/src/input/decoder_helpers.c @@ -46,7 +46,7 @@ void
> >> decoder_Init( decoder_t *p_dec, const es_format_t *restrict
> >> p_fmt ) es_format_Init( &p_dec->fmt_out, p_fmt->i_cat, 0 ); }
> >> -static void decoder_Clean( decoder_t *p_dec ) +void
> >> decoder_Clean( decoder_t *p_dec ) { es_format_Clean(
> >> &p_dec->fmt_in ); es_format_Clean( &p_dec->fmt_out ); diff
> >> --git a/src/libvlccore.sym b/src/libvlccore.sym index
> >> 906a449f2b..d2d1dac6fb 100644 --- a/src/libvlccore.sym +++
> >> b/src/libvlccore.sym @@ -74,6 +74,7 @@ date_Decrement
> >> date_Increment date_Init decoder_Init +decoder_Clean
> >> decoder_Destroy decoder_AbortPictures decoder_NewAudioBuffer
> >> diff --git a/test/src/input/decoder.c
> >> b/test/src/input/decoder.c index 6ecc584120..df05fcf4ce 100644
> >> --- a/test/src/input/decoder.c +++ b/test/src/input/decoder.c
> >> @@ -106,28 +106,12 @@ static int decoder_load(decoder_t
> >> *decoder, bool is_packetizer, if (!decoder->p_module) { -
> >> es_format_Clean(&decoder->fmt_in); + decoder_Clean( decoder );
> >> return VLC_EGENERIC; } return VLC_SUCCESS; } -static void
> >> decoder_unload(decoder_t *decoder) -{ - if (decoder->p_module
> >> != NULL) - { - module_unneed(decoder, decoder->p_module); -
> >> decoder->p_module = NULL; -
> >> es_format_Clean(&decoder->fmt_out); - } -
> >> es_format_Clean(&decoder->fmt_in); - if
> >> (decoder->p_description) - { -
> >> vlc_meta_Delete(decoder->p_description); -
> >> decoder->p_description = NULL; - } -} - void
> >> test_decoder_destroy(decoder_t *decoder) { struct
> >> decoder_owner *owner = dec_get_owner(decoder); @@ -237,7
> >> +221,7 @@ int test_decoder_process(decoder_t *decoder, block_t
> >> *p_block) decoder->pf_decode(decoder, NULL); /* Reload decoder
> >> */ - decoder_unload(decoder); + decoder_Clean(decoder); if
> >> (decoder_load(decoder, false, &packetizer->fmt_out) !=
> >> VLC_SUCCESS) { block_ChainRelease(p_packetized_block);
> >> ------------------------------------------------------------------------
> >> vlc-commits mailing list vlc-commits at videolan.org
> >> https://mailman.videolan.org/listinfo/vlc-commits
> >> ------------------------------------------------------------------------
> >> 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
> >
> > _______________________________________________
> > 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
More information about the vlc-devel
mailing list