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

[Rémi Denis-Courmont]

And by the way, I have been on the receiving end of much the same complaint about much the same thing. We agreed to use hold for reference count increases and get for getters then.

Le 24 septembre 2019 11:49:02 GMT+03:00, "Rémi Denis-Courmont" <remi at remlab.net> a écrit :
>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é.

-- 
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/3a3fb2ff/attachment.html>