[vlc-devel] [PATCH 09/31] decoder: split the decoder format update in 2 parts

[Rémi Denis-Courmont]

This is not specifically about callbacks. Our function names follow that convention. If you don't believe me, you can RTFS. It's not a subjective fact to disagree on.

Le 24 septembre 2019 10:52:07 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 2019-09-23 19:13, Rémi Denis-Courmont wrote:
>> Le maanantaina 23. syyskuuta 2019, 18.01.14 EEST Steve Lhomme a écrit
>:
>>> The first part is to create the decoder device.
>>> The second part is to create the display module (or other depending
>on the
>>> decoder owner).
>>>
>>> Turn decoder_UpdateVideoFormat() is calling the two new functions.
>>> ---
>>>   include/vlc_codec.h         | 38
>+++++++++++++++++++++++++++++++++++++
>>>   src/input/decoder_helpers.c | 20 +++++++++++++++++++
>>>   src/libvlccore.sym          |  2 ++
>>>   3 files changed, 60 insertions(+)
>>>
>>> diff --git a/include/vlc_codec.h b/include/vlc_codec.h
>>> index 28a65846d9e..8d2680f1965 100644
>>> --- a/include/vlc_codec.h
>>> +++ b/include/vlc_codec.h
>>> @@ -49,6 +49,7 @@ struct decoder_owner_callbacks
>>>       {
>>>           struct
>>>           {
>>> +            int         (*hold_device)( decoder_t *,
>vlc_decoder_device **
>>> );
>> 
>> Uh, that's very unconventional naming. Hold normally increase the
>reference
>> count on an object that you already have. Get returns a reference to
>an object
>> that you do not have. Ditto earlier.
>
>I disagree. get_XXX usually only gets the value (often a structure). 
>There is no hold_XXX callback in the code right now.
>
>IMO it's important here to tell that a reference is added to the 
>returned object. Maybe hold_ alone would sound like it would hold the 
>device if one is passed. How about `get_and_hold_device` ?
>
>>> int         (*format_update)( decoder_t * );
>>>               /* cf. decoder_NewPicture, can be called from any
>decoder
>>> thread */ @@ -253,6 +254,43 @@ struct encoder_t
>>>    * @{
>>>    */
>>>
>>> +/**
>>> + * Creates/Updates the output decoder device.
>>> + *
>>> + * This function notifies the video output pipeline of a new video
>output
>>> + * format (fmt_out.video). If there was no decoder device so far or
>a new
>>> + * decoder device is required, a new decoder device will be set up.
>>> + * decoder_UpdateVideoOutput() can then be used.
>>> + *
>>> + * If the format is unchanged, this function has no effects and
>returns
>>> zero. + *
>>> + * \param dec the decoder object
>>> + * \param pp_dec_dev the received of the held decoder device, NULL
>not to
>>> get one + *
>>> + * \note
>>> + * This function is not reentrant.
>>> + *
>>> + * @return 0 if the video output was set up successfully, -1
>otherwise.
>>> + */
>>> +VLC_API int decoder_HoldDecoderDevice( decoder_t *dec,
>vlc_decoder_device
>>> **pp_dec_dev ); +
>>> +/**
>>> + * Creates/Updates the rest of the video output pipeline.
>>> + *
>>> + * After a call to decoder_HoldDecoderDevice() this function
>notifies the
>>> + * video output pipeline of a new video output format
>(fmt_out.video). If
>>> there + * was no video output from the decoder so far, a new decoder
>video
>>> output will + * be set up. decoder_NewPicture() can then be used to
>>> allocate picture buffers. + *
>>> + * If the format is unchanged, this function has no effects and
>returns
>>> zero. + *
>>> + * \note
>>> + * This function is not reentrant.
>>> + *
>>> + * @return 0 if the video output was set up successfully, -1
>otherwise.
>>> + */
>>> +VLC_API int decoder_UpdateVideoOutput( decoder_t *dec );
>>> +
>>>   /**
>>>    * Updates the video output format.
>>>    *
>>> diff --git a/src/input/decoder_helpers.c
>b/src/input/decoder_helpers.c
>>> index 6d5b0050ed7..51e097961b9 100644
>>> --- a/src/input/decoder_helpers.c
>>> +++ b/src/input/decoder_helpers.c
>>> @@ -76,6 +76,26 @@ void decoder_Destroy( decoder_t *p_dec )
>>>   }
>>>
>>>   int decoder_UpdateVideoFormat( decoder_t *dec )
>>> +{
>>> +    int res = decoder_HoldDecoderDevice( dec, NULL );
>>> +    if (res == 0)
>>> +        res = decoder_UpdateVideoOutput( dec );
>>> +    return res;
>>> +}
>>> +
>>> +int decoder_HoldDecoderDevice( decoder_t *dec, vlc_decoder_device
>>> **pp_dec_dev ) +{
>>> +    vlc_assert( dec->fmt_in.i_cat == VIDEO_ES && dec->cbs != NULL
>);
>>> +    if ( unlikely(dec->fmt_in.i_cat != VIDEO_ES || dec->cbs == NULL
>) )
>>> +        return -1;
>>> +
>>> +    if ( dec->cbs->video.hold_device == NULL )
>>> +        return 0; /* TODO make it mandatory for all decoder owners
>*/
>>> +
>>> +    return dec->cbs->video.hold_device( dec, pp_dec_dev );
>>> +}
>>> +
>>> +int decoder_UpdateVideoOutput( decoder_t *dec )
>>>   {
>>>       vlc_assert( dec->fmt_in.i_cat == VIDEO_ES && dec->cbs != NULL
>);
>>>       if ( unlikely(dec->fmt_in.i_cat != VIDEO_ES || dec->cbs ==
>NULL ||
>>> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>>> index e5241c411d0..fb2952ee5ef 100644
>>> --- a/src/libvlccore.sym
>>> +++ b/src/libvlccore.sym
>>> @@ -82,6 +82,8 @@ decoder_Destroy
>>>   decoder_AbortPictures
>>>   decoder_NewAudioBuffer
>>>   decoder_UpdateVideoFormat
>>> +decoder_HoldDecoderDevice
>>> +decoder_UpdateVideoOutput
>>>   vlc_decoder_device_Hold
>>>   vlc_decoder_device_Release
>>>   demux_PacketizerDestroy
>> 
>> 
>> -- 
>> 雷米‧德尼-库尔蒙
>> 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/20190924/7e619543/attachment.html>