[vlc-devel] [PATCH] video_output: refactor vlc_gl_api into opengl providers

[Alexandre Janniaux]

Hi,

On Mon, Sep 21, 2020 at 02:49:12PM +0200, Thomas Guillem wrote:
> > diff --git a/modules/video_output/ios.m b/modules/video_output/ios.m
> > index 07c8355d45..fd6be2face 100644
> > --- a/modules/video_output/ios.m
> > +++ b/modules/video_output/ios.m
> > @@ -48,6 +48,7 @@
> >  #import "opengl/filter_draw.h"
> >  #import "opengl/renderer.h"
> >  #import "opengl/vout_helper.h"
> > +#import "opengl/gl_api.h"
> >
> >  /**
> >   * Forward declarations
> > @@ -124,6 +125,7 @@ struct vout_display_sys_t
> >      VLCOpenGLES2VideoView *glESView;
> >
> >      vlc_gl_t *gl;
> > +    struct vlc_gl_api api;
> >
> >      vout_window_t *embed;
> >  };
> > @@ -209,6 +211,10 @@ static int Open(vout_display_t *vd, const
> > vout_display_cfg_t *cfg,
> >          if (vlc_gl_MakeCurrent(sys->gl) != VLC_SUCCESS)
> >              goto bailout;
> >
> > +        if (vlc_gl_api_Init(&sys->api, sys->gl) != VLC_SUCCESS)
> > +            goto bailout;
> > +        sys->gl->api = &sys->api;
>
> So, you are creating a vlc_gl_api_t using a vlc_gl_t, and then storing it in the vlc_gl_t. This seems weird.
> What about a function like vlc_gl_InitApi(vlc_gl_t *gl) that fetch the api and store it in gl->api. Then you could remove the api pointer in all modules (since they already have a gl pointer).

I just remember why it was designed like this, vlc_gl_api is a
structure exposed in modules/ (because it needs OpenGL headers)
so it cannot be exposed otherwise than a pointer from the core.

We could move it to the core if we find a suitable way to define
the OpenGL interface but I'd very much prefer splitting both
as much as I can and not have the core fails to compile in the
case OpenGL is not supported.

We could also move the OpenGL layer out of the core and in a
dedicated support static library (eg. libvlc_opengl) but I'd
prefer this to be done after this patch is merged like I mentioned
in the commit message and previous mails, since it's quite
different.

Regards,
--
Alexandre Janniaux
Videolabs