[vlc-devel] [PATCH] display: use an optional callback to set the viewpoint
Alexandre Janniaux
ajanni at videolabs.io
Mon Sep 7 12:26:21 CEST 2020
Hi,
On Mon, Sep 07, 2020 at 12:04:57PM +0200, Steve Lhomme wrote:
> 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.
>
This seems fine for me, but it mostly needs to be in the document
instead of staying in the ML. ;)
It can be in this patch or in a later patch, as you wish. It can
also be changed later but having it decided for in-progress and
future modules would be great to have something unified.
> 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.
If it's up to the display module, it's probably more difficult
to force a new render ASAP and we might just wait the redisplay
timeout. I don't have any proper plan for the implementation too
so there's probably no need to hurry on adding something for now.
We can document it as-is currently and open a ticket to have it
changed in a better way.
> > 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
> >
> _______________________________________________
> 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