[vlc-devel] [PATCH v2 13/13] opengl: add plane filtering to mock filter
Alexandre Janniaux
ajanni at videolabs.io
Tue Apr 6 08:57:59 UTC 2021
On Tue, Apr 06, 2021 at 10:21:46AM +0200, Romain Vimont wrote:
> 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.
There´s almost never a submodule prefix for variables, especially
since submodules (ie. all modules) only have shortcuts, not names
since they share the main module´s name. It also changes nothing
to variable inheritency. But...
> 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.
I agree that it would be inconsistent but that would be something
to fix here, instead of just keeping adding additional features.
However, I eventually agree with your point below, regarding
exposing raw features for tests, so this patchset LGTM.
> 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
> _______________________________________________
> 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