[vlc-devel] [PATCH 09/31] decoder: split the decoder format update in 2 parts
Steve Lhomme
robux4 at ycbcr.xyz
Tue Sep 24 12:49:04 CEST 2019
On 2019-09-24 11:04, Rémi Denis-Courmont wrote:
> 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.
But here it doesn't try to hold a reference on something it already
knows. It actually gets the value, and on top of that it increases the
reference count of what it receives.
I 'RTFS' as you kindly suggested and I can't find an example of get or
hold function/callback that combines both. I looked at all the
vlc_atomic_rc_inc() calls. There's never a _get function/callback in there.
> 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é.
>
>
> _______________________________________________
> 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