[vlc-devel] [PATCH 1/2] interop_dxva2: load extension from vlc_gl_t
Alexandre Janniaux
ajanni at videolabs.io
Tue Feb 16 14:49:44 UTC 2021
NoteBis:
> It does not prevent using it when OpenGL context has been setup
> by two different API within the same thread, but I'm not sure
> how to handle this and we don't have this case for now since the
> callback/the OpenGL are setup in one of our thread anyway, and
> it still better than the previous situation where none of the
> interop were usable because none of the extensions were loaded
> by the vglmem module.
This is not entirely true that we completely control the thread the
context is being created in, since we create the vout thread after
having initialized the display, but well... It's at least called
from the decoder thread and not the application, and having decoders
running graphics is rare (not that we don't have some doing it though)
and is usually not OpenGL.
Regards,
--
Alexandre Janniaux
Videolabs
On Tue, Feb 16, 2021 at 10:12:07AM +0100, Alexandre Janniaux wrote:
> Hi,
>
> On Tue, Feb 16, 2021 at 08:05:44AM +0100, Steve Lhomme wrote:
> > On 2021-02-15 19:05, Alexandre Janniaux wrote:
> > > The only users of those extensions are interop modules, so only
> > > interop_dxva2 for extension related to WGL integration.
> > >
> > > By moving the loading of those extension to the interop, we can have
> > > them working even when the OpenGL platform context is provided by the
> > > vglmem (vgl.c) module callbacks, allowing hardware acceleration even
> > > when using the libvlc OpenGL callbacks.
> > >
> > > Filters would likely be using only OpenGL extensions that are already
> > > available, but could be using the same mechanism in the worst-case
> > > scenario.
> > >
> > > Fixes #25234
> > > ---
> > > modules/video_output/Makefile.am | 2 +-
> > > modules/video_output/opengl/interop_dxva2.c | 30 ++++++++++++++++++---
> > > 2 files changed, 28 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/modules/video_output/Makefile.am b/modules/video_output/Makefile.am
> > > index 375da4b28d..86ed27d85d 100644
> > > --- a/modules/video_output/Makefile.am
> > > +++ b/modules/video_output/Makefile.am
> > > @@ -121,7 +121,7 @@ libdirect3d9_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)'
> > > libglinterop_dxva2_plugin_la_SOURCES = video_output/opengl/interop_dxva2.c \
> > > video_output/opengl/interop.h
> > > libglinterop_dxva2_plugin_la_CFLAGS = $(AM_CFLAGS) $(GL_CFLAGS)
> > > -libglinterop_dxva2_plugin_la_LIBADD = libd3d9_common.la
> > > +libglinterop_dxva2_plugin_la_LIBADD = libd3d9_common.la -lopengl32
> > > if HAVE_WIN32_DESKTOP
> > > vout_LTLIBRARIES += $(LTLIBdirect3d9)
> > > diff --git a/modules/video_output/opengl/interop_dxva2.c b/modules/video_output/opengl/interop_dxva2.c
> > > index 2b32e93272..225db1d430 100644
> > > --- a/modules/video_output/opengl/interop_dxva2.c
> > > +++ b/modules/video_output/opengl/interop_dxva2.c
> > > @@ -71,6 +71,8 @@ vlc_module_begin ()
> > > vlc_module_end ()
> > > struct wgl_vt {
> > > + PFNWGLGETEXTENSIONSSTRINGEXTPROC GetExtensionsStringEXT;
> > > + PFNWGLGETEXTENSIONSSTRINGARBPROC GetExtensionsStringARB;
> > > PFNWGLDXSETRESOURCESHAREHANDLENVPROC DXSetResourceShareHandleNV;
> > > PFNWGLDXOPENDEVICENVPROC DXOpenDeviceNV;
> > > PFNWGLDXCLOSEDEVICENVPROC DXCloseDeviceNV;
> > > @@ -422,15 +424,37 @@ GLConvOpen(vlc_object_t *obj)
> > > return VLC_EGENERIC;
> > > }
> > > - if (interop->gl->ext != VLC_GL_EXT_WGL || !interop->gl->wgl.getExtensionsString)
> > > + if (interop->gl->surface == NULL)
> > > return VLC_EGENERIC;
> > > - const char *wglExt = interop->gl->wgl.getExtensionsString(interop->gl);
> > > + HDC hGLDC = NULL;
> > > + switch (interop->gl->surface->type)
> > > + {
> > > + case VOUT_WINDOW_TYPE_HWND:
> > > + case VOUT_WINDOW_TYPE_DUMMY:
> >
> > The wgl version only allowed VOUT_WINDOW_TYPE_HWND. I suppose it is safe
> > because there is a check on interop->gl->surface ? If so why limit to the
> > type of window at all ?
>
> Indeed, I had the switch because of a previous version of the
> patch where I was using GetDC() for the HWND type but it's of
> no use now.
>
> > Also if wglGetExtensionsStringEXT is available this is created but will
> > never be used at all. May it's easier to handle the
> > wglGetExtensionsStringEXT case first and if it's not there try the HDC way ?
>
> wglGetCurrentDC() won't create an object anyway, but checking
> that there is a DC current to the thread and bound to an OpenGL
> context prevent using the interop when one OpenGL context has
> been setup with another API (think ANGLE here for instance).
>
> It does not prevent using it when OpenGL context has been setup
> by two different API within the same thread, but I'm not sure
> how to handle this and we don't have this case for now since the
> callback/the OpenGL are setup in one of our thread anyway, and
> it still better than the previous situation where none of the
> interop were usable because none of the extensions were loaded
> by the vglmem module.
>
> In this particular case, we could just check wglGetCurrentContext
> instead and implement a QueryPlatformExtension just like Thomas
> did in the past, but that's quite equivalent and this way doesn't
> need an API addition.
>
> > > + hGLDC = wglGetCurrentDC();
> > > + break;
> > > + default: return VLC_EGENERIC;
> > > + }
> > > +
> > > + if (hGLDC == NULL)
> > > + return VLC_EGENERIC;
> > > +
> > > + struct wgl_vt vt;
> > > + vt.GetExtensionsStringEXT =
> > > + vlc_gl_GetProcAddress(interop->gl, "wglGetExtensionsStringEXT");
> > > + vt.GetExtensionsStringARB =
> > > + vlc_gl_GetProcAddress(interop->gl, "wglGetExtensionsStringARB");
> >
> > This seems like what the LOAD_EXT macro defined below is doing. Maybe move
> > the macro up and use it, for consistency. You already move the struct it's
> > writing to anyway.
>
> Not sure we need a macro for two function call TBH, though
> casting might be missing.
>
> > > +
> > > + const char *wglExt = NULL;
> > > + if (vt.GetExtensionsStringEXT)
> > > + wglExt = vt.GetExtensionsStringEXT();
> > > + else if (vt.GetExtensionsStringARB)
> > > + wglExt = vt.GetExtensionsStringARB(hGLDC);
> > > if (wglExt == NULL || !vlc_gl_StrHasToken(wglExt, "WGL_NV_DX_interop"))
> > > return VLC_EGENERIC;
> > > - struct wgl_vt vt;
> > > #define LOAD_EXT(name, type) do { \
> > > vt.name = (type) vlc_gl_GetProcAddress(interop->gl, "wgl" #name); \
> > > if (!vt.name) { \
> > > --
> > > 2.30.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
More information about the vlc-devel
mailing list