[vlc-devel] [PATCH] display: use an optional callback to set the viewpoint

Steve Lhomme robux4 at ycbcr.xyz
Mon Sep 7 12:04:57 CEST 2020


On 2020-09-07 11:58, Alexandre Janniaux wrote:
> Hi,
> 
> I'm fine with this patch in the current state.
> 
> We probably expect vlc_vout_display_operations in the
> future but it provides a first step towards this refactor.
> 
> However, it will need much more documentation on the
> expected behaviour too. Is the viewpoint change expected to
> be accounted at the end of the function or will the vout
> thread schedule an update as soon as possible for this new
> state?
> 
> It's particularily important for OpenGL since we cannot
> generally make the OpenGL context current in other threads
> without risking issues (and it's not performant since it
> triggers multiple flush) or without some special handling
> in the different providers.

When called outside of the vout thread (likely the UI thread) it's up to 
the display module do decide what it's best for it, probably keep the 
value for the next time a render happens. We'll probably need a way to 
force a render "ASAP" to minimize the latency.

I haven't implemented anything, maybe it's still cleaner in the display 
thread, we might just need to have a way to force the new render ASAP.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Mon, Sep 07, 2020 at 08:37:49AM +0200, Steve Lhomme wrote:
>> In the future it may be called directly from the thread generating the event to
>> lower the latency.
>> ---
>>   include/vlc_vout_display.h              | 12 ++++++-----
>>   modules/video_output/caopengllayer.m    | 25 +++++++++++-----------
>>   modules/video_output/ios.m              | 12 +++++++----
>>   modules/video_output/macosx.m           | 11 ++++++----
>>   modules/video_output/opengl/display.c   | 10 ++++++---
>>   modules/video_output/win32/direct3d11.c | 28 +++++++++++++------------
>>   modules/video_output/win32/glwin32.c    | 12 ++++++-----
>>   src/video_output/display.c              | 10 +++++----
>>   8 files changed, 69 insertions(+), 51 deletions(-)
>>
>> diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
>> index c0014b2924d..e5ab39d9fd6 100644
>> --- a/include/vlc_vout_display.h
>> +++ b/include/vlc_vout_display.h
>> @@ -214,11 +214,6 @@ enum vout_display_query {
>>        *                      is necessary
>>        */
>>       VOUT_DISPLAY_CHANGE_SOURCE_CROP,
>> -
>> -    /**
>> -     * Notifies a change of VR/360° viewpoint.
>> -     */
>> -    VOUT_DISPLAY_CHANGE_VIEWPOINT,   /* const vlc_viewpoint_t * */
>>   };
>>
>>   /**
>> @@ -359,6 +354,13 @@ struct vout_display_t {
>>        */
>>       int        (*control)(vout_display_t *, int query, va_list);
>>
>> +    /**
>> +     * Notifies a change of VR/360° viewpoint.
>> +     *
>> +     * May be NULL.
>> +     */
>> +    int (*set_viewpoint)(vout_display_t *, const vlc_viewpoint_t*);
>> +
>>       /**
>>        * Destroys the display.
>>        */
>> diff --git a/modules/video_output/caopengllayer.m b/modules/video_output/caopengllayer.m
>> index dd4d9ae0625..cd990944686 100644
>> --- a/modules/video_output/caopengllayer.m
>> +++ b/modules/video_output/caopengllayer.m
>> @@ -102,6 +102,17 @@ struct gl_sys
>>       VLCCAOpenGLLayer *cgLayer;
>>   };
>>
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *vp)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    if (OpenglLock(sys->gl))
>> +        return VLC_EGENERIC;
>> +
>> +    int ret = vout_display_opengl_SetViewpoint(sys->vgl, vp);
>> +    OpenglUnlock(sys->gl);
>> +    return ret;
>> +}
>> +
>>   /*****************************************************************************
>>    * Open: This function allocates and initializes the OpenGL vout method.
>>    *****************************************************************************/
>> @@ -196,6 +207,7 @@ static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
>>           vd->prepare = PictureRender;
>>           vd->display = PictureDisplay;
>>           vd->control = Control;
>> +        vd->set_viewpoint = SetViewpoint;
>>           vd->close   = Close;
>>
>>           if (OSX_SIERRA_AND_HIGHER) {
>> @@ -336,19 +348,6 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>>               return VLC_SUCCESS;
>>           }
>>
>> -        case VOUT_DISPLAY_CHANGE_VIEWPOINT:
>> -        {
>> -            int ret;
>> -
>> -            if (OpenglLock(sys->gl))
>> -                return VLC_EGENERIC;
>> -
>> -            ret = vout_display_opengl_SetViewpoint(sys->vgl,
>> -                                                   va_arg(ap, const vlc_viewpoint_t*));
>> -            OpenglUnlock(sys->gl);
>> -            return ret;
>> -        }
>> -
>>           case VOUT_DISPLAY_RESET_PICTURES:
>>               vlc_assert_unreachable ();
>>           default:
>> diff --git a/modules/video_output/ios.m b/modules/video_output/ios.m
>> index 9bfb145a841..4faaaa1e91c 100644
>> --- a/modules/video_output/ios.m
>> +++ b/modules/video_output/ios.m
>> @@ -139,6 +139,13 @@ static void *OurGetProcAddress(vlc_gl_t *gl, const char *name)
>>       return dlsym(RTLD_DEFAULT, name);
>>   }
>>
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *vp)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    struct gl_sys *glsys = sys->gl->sys;
>> +    return vout_display_opengl_SetViewpoint (glsys->vgl, vp);
>> +}
>> +
>>   static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>                   video_format_t *fmt, vlc_video_context *context)
>>   {
>> @@ -207,6 +214,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>           vd->prepare = PictureRender;
>>           vd->display = PictureDisplay;
>>           vd->control = Control;
>> +        vd->set_viewpoint = SetViewpoint;
>>           vd->close   = Close;
>>
>>           return VLC_SUCCESS;
>> @@ -268,10 +276,6 @@ static int Control(vout_display_t *vd, int query, va_list ap)
>>               return VLC_SUCCESS;
>>           }
>>
>> -        case VOUT_DISPLAY_CHANGE_VIEWPOINT:
>> -            return vout_display_opengl_SetViewpoint(glsys->vgl,
>> -                                                    va_arg(ap, const vlc_viewpoint_t*));
>> -
>>           case VOUT_DISPLAY_RESET_PICTURES:
>>               vlc_assert_unreachable ();
>>           default:
>> diff --git a/modules/video_output/macosx.m b/modules/video_output/macosx.m
>> index 77d8222c232..0d4061051b7 100644
>> --- a/modules/video_output/macosx.m
>> +++ b/modules/video_output/macosx.m
>> @@ -126,6 +126,12 @@ static void *OurGetProcAddress(vlc_gl_t *gl, const char *name)
>>       return dlsym(RTLD_DEFAULT, name);
>>   }
>>
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *vp)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    return vout_display_opengl_SetViewpoint (sys->vgl, vp);
>> +}
>> +
>>   static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
>>                    video_format_t *fmt, vlc_video_context *context)
>>   {
>> @@ -239,6 +245,7 @@ static int Open (vout_display_t *vd, const vout_display_cfg_t *cfg,
>>           vd->prepare = PictureRender;
>>           vd->display = PictureDisplay;
>>           vd->control = Control;
>> +        vd->set_viewpoint = SetViewpoint;
>>           vd->close   = Close;
>>
>>           /* */
>> @@ -382,10 +389,6 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>>                   return VLC_SUCCESS;
>>               }
>>
>> -            case VOUT_DISPLAY_CHANGE_VIEWPOINT:
>> -                return vout_display_opengl_SetViewpoint (sys->vgl,
>> -                                                         va_arg(ap, const vlc_viewpoint_t*));
>> -
>>               case VOUT_DISPLAY_RESET_PICTURES:
>>                   vlc_assert_unreachable ();
>>               default:
>> diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
>> index e8f073c3bac..630869468de 100644
>> --- a/modules/video_output/opengl/display.c
>> +++ b/modules/video_output/opengl/display.c
>> @@ -81,6 +81,12 @@ static void PictureRender (vout_display_t *, picture_t *, subpicture_t *, vlc_ti
>>   static void PictureDisplay (vout_display_t *, picture_t *);
>>   static int Control (vout_display_t *, int, va_list);
>>
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *vp)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    return vout_display_opengl_SetViewpoint (sys->vgl, vp);
>> +}
>> +
>>   /**
>>    * Allocates a surface and an OpenGL context for video output.
>>    */
>> @@ -146,6 +152,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>       vd->prepare = PictureRender;
>>       vd->display = PictureDisplay;
>>       vd->control = Control;
>> +    vd->set_viewpoint = SetViewpoint;
>>       vd->close = Close;
>>       return VLC_SUCCESS;
>>
>> @@ -252,9 +259,6 @@ static int Control (vout_display_t *vd, int query, va_list ap)
>>           sys->place_changed = true;
>>           return VLC_SUCCESS;
>>         }
>> -      case VOUT_DISPLAY_CHANGE_VIEWPOINT:
>> -        return vout_display_opengl_SetViewpoint (sys->vgl,
>> -                                                 va_arg(ap, const vlc_viewpoint_t*));
>>         default:
>>           msg_Err (vd, "Unknown request %d", query);
>>       }
>> diff --git a/modules/video_output/win32/direct3d11.c b/modules/video_output/win32/direct3d11.c
>> index c4d5b488abe..a9405da65e1 100644
>> --- a/modules/video_output/win32/direct3d11.c
>> +++ b/modules/video_output/win32/direct3d11.c
>> @@ -291,6 +291,19 @@ static void UpdateSize(vout_display_t *vd)
>>   #endif
>>   }
>>
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *viewpoint)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +    if ( sys->picQuad.pVertexShaderConstants )
>> +    {
>> +        d3d11_device_lock( sys->d3d_dev );
>> +        D3D11_UpdateViewpoint( vd, sys->d3d_dev, &sys->picQuad, viewpoint,
>> +                                (float) vd->cfg->display.width / vd->cfg->display.height );
>> +        d3d11_device_unlock( sys->d3d_dev );
>> +    }
>> +    return VLC_SUCCESS;
>> +}
>> +
>>   static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>                   video_format_t *fmtp, vlc_video_context *context)
>>   {
>> @@ -380,6 +393,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>       vd->prepare = Prepare;
>>       vd->display = Display;
>>       vd->control = Control;
>> +    vd->set_viewpoint = SetViewpoint;
>>       vd->close = Close;
>>
>>       msg_Dbg(vd, "Direct3D11 Open Succeeded");
>> @@ -402,22 +416,10 @@ static void Close(vout_display_t *vd)
>>   }
>>   static int Control(vout_display_t *vd, int query, va_list args)
>>   {
>> +    VLC_UNUSED(args);
>>       vout_display_sys_t *sys = vd->sys;
>>       int res = CommonControl( vd, &sys->area, &sys->sys, query );
>>
>> -    if (query == VOUT_DISPLAY_CHANGE_VIEWPOINT)
>> -    {
>> -        if ( sys->picQuad.pVertexShaderConstants )
>> -        {
>> -            const vlc_viewpoint_t *viewpoint = va_arg(args, const vlc_viewpoint_t*);
>> -            d3d11_device_lock( sys->d3d_dev );
>> -            D3D11_UpdateViewpoint( vd, sys->d3d_dev, &sys->picQuad, viewpoint,
>> -                                   (float) vd->cfg->display.width / vd->cfg->display.height );
>> -            d3d11_device_unlock( sys->d3d_dev );
>> -            res = VLC_SUCCESS;
>> -        }
>> -    }
>> -
>>       if ( sys->area.place_changed )
>>       {
>>           UpdateSize(vd);
>> diff --git a/modules/video_output/win32/glwin32.c b/modules/video_output/win32/glwin32.c
>> index b88c994c489..765c6bca4eb 100644
>> --- a/modules/video_output/win32/glwin32.c
>> +++ b/modules/video_output/win32/glwin32.c
>> @@ -73,14 +73,15 @@ struct vout_display_sys_t
>>   static void           Prepare(vout_display_t *, picture_t *, subpicture_t *, vlc_tick_t);
>>   static void           Display(vout_display_t *, picture_t *);
>>
>> -static int Control(vout_display_t *vd, int query, va_list args)
>> +static int SetViewpoint(vout_display_t *vd, const vlc_viewpoint_t *vp)
>>   {
>>       vout_display_sys_t *sys = vd->sys;
>> +    return vout_display_opengl_SetViewpoint(sys->vgl, vp);
>> +}
>>
>> -    if (query == VOUT_DISPLAY_CHANGE_VIEWPOINT)
>> -        return vout_display_opengl_SetViewpoint(sys->vgl,
>> -                                                va_arg(args, const vlc_viewpoint_t*));
>> -
>> +static int Control(vout_display_t *vd, int query, va_list args)
>> +{
>> +    vout_display_sys_t *sys = vd->sys;
>>       return CommonControl(vd, &sys->area, &sys->sys, query);
>>   }
>>
>> @@ -161,6 +162,7 @@ static int Open(vout_display_t *vd, const vout_display_cfg_t *cfg,
>>       vd->prepare = Prepare;
>>       vd->display = Display;
>>       vd->control = Control;
>> +    vd->set_viewpoint = SetViewpoint;
>>       vd->close = Close;
>>
>>       return VLC_SUCCESS;
>> diff --git a/src/video_output/display.c b/src/video_output/display.c
>> index b76de8ed269..b340b4711ee 100644
>> --- a/src/video_output/display.c
>> +++ b/src/video_output/display.c
>> @@ -685,10 +685,12 @@ void vout_SetDisplayViewpoint(vout_display_t *vd,
>>
>>           osys->cfg.viewpoint = *p_viewpoint;
>>
>> -        if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_VIEWPOINT,
>> -                                 &osys->cfg.viewpoint)) {
>> -            msg_Err(vd, "Failed to change Viewpoint");
>> -            osys->cfg.viewpoint = old_vp;
>> +        if (vd->set_viewpoint)
>> +        {
>> +            if (vd->set_viewpoint(vd, &osys->cfg.viewpoint)) {
>> +                msg_Err(vd, "Failed to change Viewpoint");
>> +                osys->cfg.viewpoint = old_vp;
>> +            }
>>           }
>>       }
>>   }
>> --
>> 2.26.2
>>
>> _______________________________________________
>> 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
> 


More information about the vlc-devel mailing list