[vlc-devel] [PATCH 04/26] video_output: split vout_Request into vout_HoldDevice and vout_RequestDisplay

[Steve Lhomme]

On 2019-09-23 15:33, Thomas Guillem wrote:
> 
> 
> On Fri, Sep 20, 2019, at 16:28, Steve Lhomme wrote:
>> This will be used by the HW decoder to get the "decoder device". Then
>> when the VA
>> is created (or not) based on this "decoder device", the display will be
>> created.
>>
>> Don't create a new display in vout_RequestDisplay() if there's already one.
>> Maybe we need to check if the format changed to know if a new display is needed.
>> ---
>>   src/video_output/video_output.c  | 35 ++++++++++++++++++++++++--------
>>   src/video_output/vout_internal.h | 26 ++++++++++++++++++++++++
>>   2 files changed, 53 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>> index 2a38bd677c4..cd33834f1d2 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -1862,6 +1862,16 @@ vout_thread_t *vout_Hold(vout_thread_t *vout)
>>   
>>   int vout_Request(const vout_configuration_t *cfg, vlc_decoder_device
>> **dec_dev,
>>                    input_thread_t *input)
>> +{
>> +    int res = vout_HoldDevice(cfg, dec_dev);
>> +    if (res == 0)
>> +    {
>> +        res = vout_RequestDisplay(cfg, input);
>> +    }
>> +    return res;
>> +}
>> +
>> +int vout_HoldDevice(const vout_configuration_t *cfg,
>> vlc_decoder_device **dec_dev)
>>   {
>>       vout_thread_t *vout = cfg->vout;
>>       vout_thread_sys_t *sys = vout->p;
>> @@ -1917,7 +1927,9 @@ int vout_Request(const vout_configuration_t *cfg,
>> vlc_decoder_device **dec_dev,
>>   
>>           if (vout_window_Enable(sys->display_cfg.window, &wcfg)) {
>>               vlc_mutex_unlock(&sys->window_lock);
>> -            goto error;
>> +            msg_Err(vout, "window enabling failed");
>> +            video_format_Clean(&sys->original);
>> +            return -1;
>>           }
>>           sys->window_enabled = true;
>>   
>> @@ -1931,8 +1943,22 @@ int vout_Request(const vout_configuration_t
>> *cfg, vlc_decoder_device **dec_dev,
>>       sys->clock = cfg->clock;
>>       sys->delay = 0;
>>   
>> +    if (dec_dev)
>> +        *dec_dev = sys->dec_device ? vlc_decoder_device_Hold(
>> sys->dec_device ) : NULL;
>> +
>>       vlc_mutex_unlock(&sys->window_lock);
>>   
>> +    return 0;
>> +}
>> +
>> +int vout_RequestDisplay(const vout_configuration_t *cfg, input_thread_t *input)
>> +{
>> +    vout_thread_t *vout = cfg->vout;
>> +    vout_thread_sys_t *sys = vout->p;
>> +
>> +    if (sys->display != NULL)
>> +        return VLC_SUCCESS;
>> +
>>       if (vout_Start(vout, cfg))
>>       {
>>           vlc_mutex_lock(&sys->window_lock);
>> @@ -1954,13 +1980,6 @@ error:
>>           return -1;
>>       }
>>   
>> -    if (dec_dev)
>> -    {
>> -        vlc_mutex_lock(&sys->window_lock);
>> -        *dec_dev = sys->dec_device ? vlc_decoder_device_Hold(
>> sys->dec_device ) : NULL;
>> -        vlc_mutex_unlock(&sys->window_lock);
>> -    }
>> -
>>       if (input != NULL && sys->spu)
>>           spu_Attach(sys->spu, input);
>>       vout_IntfReinit(vout);
>> diff --git a/src/video_output/vout_internal.h
>> b/src/video_output/vout_internal.h
>> index 1f88997779c..cf0ca124f54 100644
>> --- a/src/video_output/vout_internal.h
>> +++ b/src/video_output/vout_internal.h
>> @@ -197,6 +197,32 @@ vout_thread_t *vout_Create(vlc_object_t *obj)
>> VLC_USED;
>>   
>>   vout_thread_t *vout_CreateDummy(vlc_object_t *obj) VLC_USED;
>>   
>> +/**
>> + * Returns a suitable vout with an associated decoder device.
>> + *
>> + * \param cfg the video configuration requested.
>> + * \param dec_dev pointer to receive the decoder device reference to
>> use with the vout or NULL
>> + * \retval 0 on success
>> + * \retval -1 on error
>> + */
>> +int vout_HoldDevice(const vout_configuration_t *cfg,
>> vlc_decoder_device **dec_dev);
>> +
>> +/**
>> + * Returns a suitable vout or release the given one.
> 
> It's weird since this function returns an int.
> 
> Are you creating a vout or starting a display module of an existing vout ? It's not that obvious with the comment/function_name that are not matching.

It's not creating a vout, there's vout_Create() for that. It's not 
starting a display module either, which is done by vout_RequestDisplay().

I'll update the function documentation which was mostly copy/pasted.