[vlc-devel] [PATCH 08/38] video_output: add vout_GetDevice to get the vout decoder device on demand

[Steve Lhomme]

On 2019-10-02 8:52, Thomas Guillem wrote:
> 
> 
> On Tue, Oct 1, 2019, at 13:12, Steve Lhomme wrote:
>> Only a few decoders will request it.
>>
>> A decoder device can only be created if the vout has an enabled window.
>>
>> The caller receives a reference to the decoder device or NULL.
>> ---
>>   src/video_output/video_output.c  | 32 ++++++++++++++++++++++++++++++++
>>   src/video_output/vout_internal.h | 10 ++++++++++
>>   2 files changed, 42 insertions(+)
>>
>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>> index 131fda74225..dea1141ceae 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -49,6 +49,7 @@
>>   #include <vlc_vout_osd.h>
>>   #include <vlc_image.h>
>>   #include <vlc_plugin.h>
>> +#include <vlc_codec.h>
>>   
>>   #include <libvlc.h>
>>   #include "vout_internal.h"
>> @@ -1727,6 +1728,11 @@ void vout_Stop(vout_thread_t *vout)
>>           vout_StopDisplay(vout);
>>   
>>       vlc_mutex_lock(&sys->window_lock);
>> +    if (sys->dec_device)
>> +    {
>> +        vlc_decoder_device_Release(sys->dec_device);
>> +        sys->dec_device = NULL;
>> +    }
>>       if (sys->window_enabled) {
>>           vout_window_Disable(sys->display_cfg.window);
>>           sys->window_enabled = false;
>> @@ -1774,6 +1780,9 @@ void vout_Release(vout_thread_t *vout)
>>       vlc_mutex_destroy(&vout->p->window_lock);
>>       vlc_mutex_destroy(&vout->p->filter.lock);
>>   
>> +    if (sys->dec_device)
>> +        vlc_decoder_device_Release(sys->dec_device);
>> +
>>       assert(!sys->window_enabled);
>>       vout_display_window_Delete(sys->display_cfg.window);
>>   
>> @@ -1989,6 +1998,11 @@ int vout_Request(const vout_configuration_t
>> *cfg, input_thread_t *input)
>>           vlc_mutex_lock(&sys->window_lock);
>>           vout_window_Disable(sys->display_cfg.window);
>>           sys->window_enabled = false;
>> +        if (sys->dec_device)
>> +        {
>> +            vlc_decoder_device_Release(sys->dec_device);
>> +            sys->dec_device = NULL;
>> +        }
>>           vlc_mutex_unlock(&sys->window_lock);
>>           goto error;
>>       }
>> @@ -2005,3 +2019,21 @@ error:
>>       vout_IntfReinit(vout);
>>       return 0;
>>   }
>> +
>> +vlc_decoder_device *vout_GetDevice(const vout_configuration_t *cfg)
>> +{
>> +    vout_thread_t *vout = cfg->vout;
>> +    vout_thread_sys_t *sys = vout->p;
>> +
>> +    if (VoutEnsureEnabledWindow(cfg) != 0)
>> +        return NULL;
>> +
>> +    vlc_mutex_lock(&sys->window_lock);
> 
> VoutEnsureEnabledWindow() and vlc_decoder_device_Create() should be called within the same lock to respect atomicity.
> 
> I propose you to move the lock oustide the VoutEnsureEnabledWindow() function.

This would call vout_StopDisplay() under a lock which wasn't the case 
before. This function does a *lot* of things.

I'd rather create the decoder device in VoutEnsureEnabledWindow() on 
demand, rather the locking more calls than it used to.


>> +    assert(sys->window_enabled);
>> +    if (sys->dec_device == NULL)
>> +        sys->dec_device =
>> vlc_decoder_device_Create(sys->display_cfg.window);
>> +    vlc_decoder_device *dec_device = sys->dec_device ?
>> vlc_decoder_device_Hold( sys->dec_device ) : NULL;
>> +    vlc_mutex_unlock(&sys->window_lock);
>> +
>> +    return dec_device;
>> +}
>> diff --git a/src/video_output/vout_internal.h
>> b/src/video_output/vout_internal.h
>> index 29f2144f82b..30f79794e12 100644
>> --- a/src/video_output/vout_internal.h
>> +++ b/src/video_output/vout_internal.h
>> @@ -173,6 +173,7 @@ struct vout_thread_sys_t
>>       /* Video output window */
>>       bool            window_enabled;
>>       vlc_mutex_t     window_lock;
>> +    vlc_decoder_device *dec_device;
>>   
>>       /* Video output display */
>>       vout_display_cfg_t display_cfg;
>> @@ -195,6 +196,14 @@ vout_thread_t *vout_Create(vlc_object_t *obj) VLC_USED;
>>   
>>   vout_thread_t *vout_CreateDummy(vlc_object_t *obj) VLC_USED;
>>   
>> +/**
>> + * Setup the vout for the given configuration and get an associated
>> decoder device.
>> + *
>> + * \param cfg the video configuration requested.
>> + * \return pointer to a decoder device reference to use with the vout
>> or NULL
>> + */
>> +vlc_decoder_device *vout_GetDevice(const vout_configuration_t *cfg);
>> +
>>   /**
>>    * Returns a suitable vout or release the given one.
>>    *
>> @@ -206,6 +215,7 @@ vout_thread_t *vout_CreateDummy(vlc_object_t *obj)
>> VLC_USED;
>>    *
>>    * \param cfg the video configuration requested.
>>    * \param input used to get attachments for spu filters
>> + * \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
>>    */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> 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
>