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

[Steve Lhomme]

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
>