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

[Thomas Guillem]

On Wed, Oct 2, 2019, at 11:33, Steve Lhomme wrote:
> 
> 
> 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.

Fine with me.

> 
> 
> >> +    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
> > 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel