<html><head></head><body>Hi,<br><br>The contemporary decoder owner is actually the owner (in the usual VLC/nefrir naming sense) when using the input decoder functions, for ES output and sout_display. Definitely don't add generic / non-input-specific state in owner.<br><br>There is currently no common generic API for creating/destroying a decoder. There just was not much use for it, and nobody cared until now. That's why owner and decoder can and are allocated together. If you need to add internal common values, you should separate owner and decoder objects.<br><br>However, I doubt that we can share the decoder thread, which is the bulk of decoder.c at the moment, because it is so intertwined with the input at the moment.<br><br><div class="gmail_quote">Le 20 février 2019 10:09:53 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 20/02/2019 09:01, Steve Lhomme wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">On 19/02/2019 15:10, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Hi,<br><br> I violently agree that there should be a clear separation between the <br> generic ("owner") functions, and the ES output functions.<br><br> However, I think it makes more sense to keep the (vlc_)dec(oder)_ <br> prefix for the generic case, and (vlc_)input_dec(oder)_ prefix for <br> the ES output.<br></blockquote>I actually created a separate C file so they don't end up being mixed. <br>But it's still a WIP. I've been bitten hard while working on this by <br>assuming I could put stuff (like the pool and the context) in the <br>decoder owner. <br></blockquote><br>The reason for this is that a lot of things that should be generic for <br>all decoder use (flushing, delay, pause, preroll, mouse, etc) are <br>actually tied to a particular """owner""". This should change. But I <br>haven't worked on that.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">But I did not realize the "owner" is not an owner. It's the actual <br>module. It's created as a vlc_object and never released as such, <br>because done on the decoder which is actually its own owner. And then <br>one wonder why I'd like to reset objects to NULL after being released...<br></blockquote><br><br>><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">As for the Load/Destroy/Init/Unload naming, I don't really care. This <br>can be changed at any time.<br></blockquote><br>For the separation between actual decoder and owner related code I agree <br>with Remi. It's better to keep (vlc_)decoder_ for the generic decoder <br>code and input_dec_ for what constitutes most of decoder.c.<br><br>><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"> Le 19 février 2019 14:41:48 GMT+02:00, Thomas Guillem <br> <thomas@gllm.fr> a écrit :<br><br> Why not calling it decoder_Unload() since its mean goal is to <br> unload the decoder module.<br><br> You should really use an other prefix for decoder owner <br> functions. I really don't like having the the same prefix for <br> functions that can only be used by the owner and some by module <br> implementations.<br><br> vlc_dec_owner, for example.<br><br> On Mon, Feb 18, 2019, at 16:11, Steve Lhomme wrote:<br><br> vlc | branch: master | Steve Lhomme <robux4@ycbcr.xyz> | Mon<br> Feb 18 15:02:10 2019 +0100|<br> [55ae595eb5e80b4fc66de90803e802e2aa70a77f] | committer: Steve<br> Lhomme decoder: make decoder_Clean() public The decoder and<br> decoder tests actually load/unload decoders without resetting<br> the whole structure.<br><br> <a href="http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=55ae595eb5e80b4fc66de90803e802e2aa70a77f">http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=55ae595eb5e80b4fc66de90803e802e2aa70a77f</a><hr> include/vlc_codec.h | 7 +++++++ src/input/decoder.c | 32<br> +++++--------------------------- src/input/decoder_helpers.c |<br> 2 +- src/libvlccore.sym | 1 + test/src/input/decoder.c | 20<br> ++------------------ 5 files changed, 16 insertions(+), 46<br> deletions(-) diff --git a/include/vlc_codec.h<br> b/include/vlc_codec.h index 2042ad2424..3961b8c9a1 100644 ---<br> a/include/vlc_codec.h +++ b/include/vlc_codec.h @@ -327,6<br> +327,13 @@ VLC_API void decoder_Init( decoder_t *dec, const<br> es_format_t * ); VLC_API void decoder_Destroy( decoder_t<br> **p_dec ); /** + * Unload a decoder module and reset the<br> input/output formats. + * + * To be used by decoder owners. +<br> */ +VLC_API void decoder_Clean( decoder_t *p_dec ); + +/** *<br> This function queues a single picture to the video output. * *<br> \note diff --git a/src/input/decoder.c b/src/input/decoder.c<br> index 05ba7f6abb..582a2eaf2e 100644 --- a/src/input/decoder.c<br> +++ b/src/input/decoder.c @@ -189,38 +189,16 @@ static int<br> LoadDecoder( decoder_t *p_dec, bool b_packetizer, if(<br> !p_dec->p_module ) { - es_format_Clean( &p_dec->fmt_in ); +<br> decoder_Clean( p_dec ); return -1; } - else - return 0; -} -<br> -/** - * Unload a decoder module - */ -static void<br> UnloadDecoder( decoder_t *p_dec ) -{ - if( p_dec->p_module ) -<br> { - module_unneed( p_dec, p_dec->p_module ); - p_dec->p_module<br> = NULL; - } - - if( p_dec->p_description ) - { -<br> vlc_meta_Delete( p_dec->p_description ); -<br> p_dec->p_description = NULL; - } - - es_format_Clean(<br> &p_dec->fmt_in ); - es_format_Clean( &p_dec->fmt_out ); +<br> return 0; } static int ReloadDecoder( decoder_t *p_dec, bool<br> b_packetizer, const es_format_t *restrict p_fmt, enum reload<br> reload ) { - /* Copy p_fmt since it can be destroyed by<br> UnloadDecoder */ + /* Copy p_fmt since it can be destroyed by<br> decoder_Clean */ struct decoder_owner *p_owner =<br> dec_get_owner( p_dec ); es_format_t fmt_in; if(<br> es_format_Copy( &fmt_in, p_fmt ) != VLC_SUCCESS ) @@ -230,7<br> +208,7 @@ static int ReloadDecoder( decoder_t *p_dec, bool<br> b_packetizer, } /* Restart the decoder module */ -<br> UnloadDecoder( p_dec ); + decoder_Clean( p_dec );<br> p_owner->error = false; if( reload == RELOAD_DECODER_AOUT ) @@<br> -1974,7 +1952,7 @@ static void DeleteDecoder( decoder_t *<br> p_dec ) (char*)&p_dec->fmt_in.i_codec ); const enum<br> es_format_category_e i_cat =p_dec->fmt_in.i_cat; -<br> UnloadDecoder( p_dec ); + decoder_Clean( p_dec ); /* Free all<br> packets still in the decoder fifo. */ block_FifoRelease(<br> p_owner->p_fifo ); diff --git a/src/input/decoder_helpers.c<br> b/src/input/decoder_helpers.c index b60b830123..693c6b3955<br> 100644 --- a/src/input/decoder_helpers.c +++<br> b/src/input/decoder_helpers.c @@ -46,7 +46,7 @@ void<br> decoder_Init( decoder_t *p_dec, const es_format_t *restrict<br> p_fmt ) es_format_Init( &p_dec->fmt_out, p_fmt->i_cat, 0 ); }<br> -static void decoder_Clean( decoder_t *p_dec ) +void<br> decoder_Clean( decoder_t *p_dec ) { es_format_Clean(<br> &p_dec->fmt_in ); es_format_Clean( &p_dec->fmt_out ); diff<br> --git a/src/libvlccore.sym b/src/libvlccore.sym index<br> 906a449f2b..d2d1dac6fb 100644 --- a/src/libvlccore.sym +++<br> b/src/libvlccore.sym @@ -74,6 +74,7 @@ date_Decrement<br> date_Increment date_Init decoder_Init +decoder_Clean<br> decoder_Destroy decoder_AbortPictures decoder_NewAudioBuffer<br> diff --git a/test/src/input/decoder.c<br> b/test/src/input/decoder.c index 6ecc584120..df05fcf4ce 100644<br> --- a/test/src/input/decoder.c +++ b/test/src/input/decoder.c<br> @@ -106,28 +106,12 @@ static int decoder_load(decoder_t<br> *decoder, bool is_packetizer, if (!decoder->p_module) { -<br> es_format_Clean(&decoder->fmt_in); + decoder_Clean( decoder );<br> return VLC_EGENERIC; } return VLC_SUCCESS; } -static void<br> decoder_unload(decoder_t *decoder) -{ - if (decoder->p_module<br> != NULL) - { - module_unneed(decoder, decoder->p_module); -<br> decoder->p_module = NULL; -<br> es_format_Clean(&decoder->fmt_out); - } -<br> es_format_Clean(&decoder->fmt_in); - if<br> (decoder->p_description) - { -<br> vlc_meta_Delete(decoder->p_description); -<br> decoder->p_description = NULL; - } -} - void<br> test_decoder_destroy(decoder_t *decoder) { struct<br> decoder_owner *owner = dec_get_owner(decoder); @@ -237,7<br> +221,7 @@ int test_decoder_process(decoder_t *decoder, block_t<br> *p_block) decoder->pf_decode(decoder, NULL); /* Reload decoder<br> */ - decoder_unload(decoder); + decoder_Clean(decoder); if<br> (decoder_load(decoder, false, &packetizer->fmt_out) !=<br> VLC_SUCCESS) { block_ChainRelease(p_packetized_block);<hr> vlc-commits mailing list vlc-commits@videolan.org<br> <a href="https://mailman.videolan.org/listinfo/vlc-commits">https://mailman.videolan.org/listinfo/vlc-commits</a><hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br><br> -- <br> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez <br> excuser ma brièveté.<hr> vlc-devel mailing list<br> To unsubscribe or modify your subscription options:<br> <a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>