[vlc-devel] [PATCH v2 13/13] opengl: add plane filtering to mock filter

Romain Vimont rom1v at videolabs.io
Tue Apr 6 08:21:46 UTC 2021


On Thu, Apr 01, 2021 at 08:22:30PM +0200, Alexandre Janniaux wrote:
> Hi,
> 
> On Thu, Apr 01, 2021 at 07:06:03PM +0200, Romain Vimont wrote:
> > On Thu, Apr 01, 2021 at 06:37:42PM +0200, Alexandre Janniaux wrote:
> > > Hi,
> > >
> > > If the mock{plane} is not able to use any other options, I´d
> > > rather have it as a separate submodule mock_plane than an
> > > option of the filter, if that´s ok with you.
> >
> > I'd prefer to keep only 1 module and 1 file for OpenGL mocks for
> > simplicity.
> >
> > Moreover, in theory, the mock filter plane could use many of existing
> > options (angle, speed, mask).
> >
> > Regards
> 
> Well, there´s two reason between this actually:
> 
>  - --MOCK_CFG_PREFIX-plane would set all mock filter into
>    plane filtering by default. But plane filtering and
>    picture filtering look like very different things at
>    first so this is a bit unexpected imho. A different
>    module name won´t have this issue.

The only difference is that it applies on individual planes instead of
the final picture. We could enable the filter_planes flag on the mask
mock filter (but we don't handle it because it brings little benefit to
test the OpenGL filters behavior).

>  - You can have a different submodule which can still share
>    the same option as the first, but only enable the one it
>    supports, making it much clearer on what is supported and
>    what is not.
>
> Besides, A submodule doesn´t really add complexity, it
> removes complexity by scoping things and add verbosity
> instead.
> 
> What do you think?

I think that using a submodule adds complexity compared to not using
one: we need to have in mind what happens to VLC variables, how they are
shared/inherited, how they must be named from the command-line (with the
parent prefix or the submodule prefix?), etc.

For a simple mock plugin, I personally prefer without submodules.

Moreover, it would be inconsistent to make the mock plane filter a
submodule, but not the others (mask VS blend, which share the same angle
and speed variables). So they should be separate submodules too. IMO
it's not worth it.

In the API, blend/mask and plane/non-plane are exposed as flags. Some
combinations are also forbidden (a plane filter may not be blend,
because we don't support it, since it is not useful in practice). I
think it's better to just expose the same flags in the mock filter as in
the API it tests.

Regards

> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> > >
> > > Regards,
> > > --
> > > Alexandre Janniaux
> > > Videolabs
> > >
> > > On Tue, Mar 30, 2021 at 01:14:11PM +0200, Romain Vimont wrote:
> > > > This helps for testing plane filtering.
> > > > ---
> > > >  modules/video_output/opengl/filter_mock.c | 129 +++++++++++++++++++++-
> > > >  1 file changed, 125 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/modules/video_output/opengl/filter_mock.c b/modules/video_output/opengl/filter_mock.c
> > > > index 258c102f92..41851fe35d 100644
> > > > --- a/modules/video_output/opengl/filter_mock.c
> > > > +++ b/modules/video_output/opengl/filter_mock.c
> > > > @@ -49,8 +49,12 @@
> > > >   * Several instances may be combined:
> > > >   *
> > > >   *     ./vlc file.mkv --video-filter='opengl{filter="mock{mask,speed=1}:mock{angle=180,speed=-1}"}'
> > > > + *
> > > > + * It can also be used to filter planes separately, drawing each one with a
> > > > + * small offset:
> > > > + *
> > > > + *     ./vlc file.mkv --video-filter='opengl{filter=mock{plane}}'
> > > >   */
> > > > -
> > > >  #ifdef HAVE_CONFIG_H
> > > >  # include "config.h"
> > > >  #endif
> > > > @@ -80,7 +84,7 @@
> > > >  #define MOCK_CFG_PREFIX "mock-"
> > > >
> > > >  static const char *const filter_options[] = {
> > > > -    "angle", "mask", "msaa", "speed", NULL
> > > > +    "angle", "mask", "msaa", "plane", "speed", NULL
> > > >  };
> > > >
> > > >  struct sys {
> > > > @@ -92,6 +96,7 @@ struct sys {
> > > >          GLint vertex_pos;
> > > >          GLint rotation_matrix;
> > > >          GLint vertex_color; // blend (non-mask) only
> > > > +        GLint plane;
> > > >      } loc;
> > > >
> > > >      float theta0;
> > > > @@ -193,6 +198,31 @@ DrawMask(struct vlc_gl_filter *filter, const struct vlc_gl_input_meta *meta)
> > > >      return VLC_SUCCESS;
> > > >  }
> > > >
> > > > +static int
> > > > +DrawPlane(struct vlc_gl_filter *filter, const struct vlc_gl_input_meta *meta)
> > > > +{
> > > > +    struct sys *sys = filter->sys;
> > > > +
> > > > +    const opengl_vtable_t *vt = &filter->api->vt;
> > > > +
> > > > +    vt->UseProgram(sys->program_id);
> > > > +
> > > > +    struct vlc_gl_sampler *sampler = vlc_gl_filter_GetSampler(filter);
> > > > +    vlc_gl_sampler_Load(sampler);
> > > > +
> > > > +    vt->Uniform1i(sys->loc.plane, (int) meta->plane);
> > > > +
> > > > +    vt->BindBuffer(GL_ARRAY_BUFFER, sys->vbo);
> > > > +    vt->EnableVertexAttribArray(sys->loc.vertex_pos);
> > > > +    vt->VertexAttribPointer(sys->loc.vertex_pos, 2, GL_FLOAT, GL_FALSE, 0,
> > > > +                            (const void *) 0);
> > > > +
> > > > +    vt->Clear(GL_COLOR_BUFFER_BIT);
> > > > +    vt->DrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> > > > +
> > > > +    return VLC_SUCCESS;
> > > > +}
> > > > +
> > > >  static void
> > > >  Close(struct vlc_gl_filter *filter)
> > > >  {
> > > > @@ -250,6 +280,10 @@ InitBlend(struct vlc_gl_filter *filter)
> > > >      sys->loc.vertex_color = vt->GetAttribLocation(program_id, "vertex_color");
> > > >      assert(sys->loc.vertex_color != -1);
> > > >
> > > > +    sys->loc.rotation_matrix = vt->GetUniformLocation(sys->program_id,
> > > > +                                                      "rotation_matrix");
> > > > +    assert(sys->loc.rotation_matrix != -1);
> > > > +
> > > >      vt->GenBuffers(1, &sys->vbo);
> > > >
> > > >      static const GLfloat data[] = {
> > > > @@ -356,6 +390,88 @@ InitMask(struct vlc_gl_filter *filter)
> > > >      return VLC_SUCCESS;
> > > >  }
> > > >
> > > > +static int
> > > > +InitPlane(struct vlc_gl_filter *filter)
> > > > +{
> > > > +    struct sys *sys = filter->sys;
> > > > +    const opengl_vtable_t *vt = &filter->api->vt;
> > > > +
> > > > +    /* Must be initialized before calling vlc_gl_filter_GetSampler() */
> > > > +    filter->config.filter_planes = true;
> > > > +
> > > > +    struct vlc_gl_sampler *sampler = vlc_gl_filter_GetSampler(filter);
> > > > +
> > > > +    static const char *const VERTEX_SHADER =
> > > > +        SHADER_VERSION
> > > > +        "attribute vec2 vertex_pos;\n"
> > > > +        "varying vec2 tex_coords;\n"
> > > > +        "void main() {\n"
> > > > +        "  gl_Position = vec4(vertex_pos, 0.0, 1.0);\n"
> > > > +        "  tex_coords = vec2((vertex_pos.x + 1.0) / 2.0,\n"
> > > > +        "                    (vertex_pos.y + 1.0) / 2.0);\n"
> > > > +        "}\n";
> > > > +
> > > > +    static const char *const FRAGMENT_SHADER_TEMPLATE =
> > > > +        SHADER_VERSION
> > > > +        "%s\n" /* extensions */
> > > > +        FRAGMENT_SHADER_PRECISION
> > > > +        "%s\n" /* vlc_texture definition */
> > > > +        "varying vec2 tex_coords;\n"
> > > > +        "uniform int plane;\n"
> > > > +        "void main() {\n"
> > > > +        "  vec2 offset = vec2(float(plane) * 0.02);\n"
> > > > +        "  gl_FragColor = vlc_texture(fract(tex_coords + offset));\n"
> > > > +        "}\n";
> > > > +
> > > > +    const char *extensions = sampler->shader.extensions
> > > > +                           ? sampler->shader.extensions : "";
> > > > +
> > > > +    char *fragment_shader;
> > > > +    int ret = asprintf(&fragment_shader, FRAGMENT_SHADER_TEMPLATE, extensions,
> > > > +                       sampler->shader.body);
> > > > +    if (ret < 0)
> > > > +        return VLC_EGENERIC;
> > > > +
> > > > +    GLuint program_id =
> > > > +        vlc_gl_BuildProgram(VLC_OBJECT(filter), vt,
> > > > +                            1, (const char **) &VERTEX_SHADER,
> > > > +                            1, (const char **) &fragment_shader);
> > > > +    free(fragment_shader);
> > > > +    if (!program_id)
> > > > +        return VLC_EGENERIC;
> > > > +
> > > > +    sys->program_id = program_id;
> > > > +
> > > > +    vlc_gl_sampler_FetchLocations(sampler, program_id);
> > > > +
> > > > +    sys->loc.vertex_pos = vt->GetAttribLocation(sys->program_id, "vertex_pos");
> > > > +    assert(sys->loc.vertex_pos != -1);
> > > > +
> > > > +    sys->loc.plane = vt->GetUniformLocation(program_id, "plane");
> > > > +    assert(sys->loc.plane != -1);
> > > > +
> > > > +    static const GLfloat vertex_pos[] = {
> > > > +        -1,  1,
> > > > +        -1, -1,
> > > > +         1,  1,
> > > > +         1, -1,
> > > > +    };
> > > > +
> > > > +    vt->BindBuffer(GL_ARRAY_BUFFER, sys->vbo);
> > > > +    vt->BufferData(GL_ARRAY_BUFFER, sizeof(vertex_pos), vertex_pos,
> > > > +                   GL_STATIC_DRAW);
> > > > +
> > > > +    vt->BindBuffer(GL_ARRAY_BUFFER, 0);
> > > > +
> > > > +    static const struct vlc_gl_filter_ops ops = {
> > > > +        .draw = DrawPlane,
> > > > +        .close = Close,
> > > > +    };
> > > > +    filter->ops = &ops;
> > > > +
> > > > +    return VLC_SUCCESS;
> > > > +}
> > > > +
> > > >  static vlc_gl_filter_open_fn Open;
> > > >  static int
> > > >  Open(struct vlc_gl_filter *filter, const config_chain_t *config,
> > > > @@ -366,6 +482,7 @@ Open(struct vlc_gl_filter *filter, const config_chain_t *config,
> > > >      config_ChainParse(filter, MOCK_CFG_PREFIX, filter_options, config);
> > > >
> > > >      bool mask = var_InheritBool(filter, MOCK_CFG_PREFIX "mask");
> > > > +    bool plane = var_InheritBool(filter, MOCK_CFG_PREFIX "plane");
> > > >      float angle = var_InheritFloat(filter, MOCK_CFG_PREFIX "angle");
> > > >      float speed = var_InheritFloat(filter, MOCK_CFG_PREFIX "speed");
> > > >      int msaa = var_InheritInteger(filter, MOCK_CFG_PREFIX "msaa");
> > > > @@ -375,7 +492,9 @@ Open(struct vlc_gl_filter *filter, const config_chain_t *config,
> > > >          return VLC_EGENERIC;
> > > >
> > > >      int ret;
> > > > -    if (mask)
> > > > +    if (plane)
> > > > +        ret = InitPlane(filter);
> > > > +    else if (mask)
> > > >          ret = InitMask(filter);
> > > >      else
> > > >          ret = InitBlend(filter);
> > > > @@ -387,7 +506,8 @@ Open(struct vlc_gl_filter *filter, const config_chain_t *config,
> > > >      sys->theta0 = angle * M_PI / 180; /* angle in degrees, theta0 in radians */
> > > >      sys->speed = speed;
> > > >
> > > > -    filter->config.msaa_level = msaa;
> > > > +    /* MSAA is not supported for plane filters */
> > > > +    filter->config.msaa_level = plane ? 0 : msaa;
> > > >
> > > >      return VLC_SUCCESS;
> > > >
> > > > @@ -407,5 +527,6 @@ vlc_module_begin()
> > > >      add_float(MOCK_CFG_PREFIX "angle", 0.f, NULL, NULL, false) /* in degrees */
> > > >      add_float(MOCK_CFG_PREFIX "speed", 0.f, NULL, NULL, false) /* in rotations per minute */
> > > >      add_bool(MOCK_CFG_PREFIX "mask", false, NULL, NULL, false)
> > > > +    add_bool(MOCK_CFG_PREFIX "plane", false, NULL, NULL, false)
> > > >      add_integer(MOCK_CFG_PREFIX "msaa", 4, NULL, NULL, false);
> > > >  vlc_module_end()
> > > > --
> > > > 2.31.0
> > > >
> > > > _______________________________________________
> > > > 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
> _______________________________________________
> 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