[vlc-devel] [PATCH 2/2] opengl: move fragment shader creation to renderer
Alexandre Janniaux
ajanni at videolabs.io
Tue Mar 24 11:22:22 CET 2020
Ok, maybe you can add that the BuildFragmentShader function
is intended to stay in renderer.c to the commit message and
LGTM for me
Regards,
--
Alexandre Janniaux
Videolabs
On Tue, Mar 24, 2020 at 11:15:21AM +0100, Romain Vimont wrote:
>
>
> On 3/24/20 9:55 AM, Alexandre Janniaux wrote:
> > Creating a sampler (from sampler.c) will be shared by
> > sub_renderer and renderer?
>
> Each filter (like the renderer) will receive its own sampler (the input
> picture details will be different for each).
>
> > Will sampler.c have a
> > BuildFragmentShader like function to create the sampler?
> >
> > My point is to find out whether or not it's best to move
> > code from fragment_shader.c to renderer.c or the opposite,
> > and make the code move clearer in the commit message as it
> > is a bit confusing to have the BuildFragmentShader function
> > out of the fragment_shader.c file.
>
> Before the refactor, fragment_shaders.c always created the whole fragment
> shader code.
>
> The purpose of the refactor is to split differently: the renderer (or any
> other "filter" in the future) use their own vertex + fragment shader code to
> do whatever they want, with just some piece of code generated by the sampler
> (to be injected in their GLSL) to access the input picture, (regardless of
> its chroma, padding, orientation, interop, etc.).
>
> The separation between what must be implemented in the filters (renderer)
> and what must be implemented in the sampler is straightforward:
> - everything used to provide access to the input picture (i.e. needed for
> the implementation of "vlc_texture(vec2 coords)") must be in the sampler.
> - everything filter-specific (what the filter does with the input picture)
> must be in the filter.
>
> (so after the refactor, the name "fragment_shaders.c" does not make much
> sense, that's why it is renamed later)
>
> In the end (some commits later on my branch), the "renderer" fragment shader
> is really simple, since it just assigns the color from the input picture:
> https://code.videolan.org/rom1v/vlc/-/blob/7d38529068e68297d42d345a9fb3c7611968b0ac/modules/video_output/opengl/renderer.c#L217-225
>
> But other filters could "adjust" the colors, or do more complicated things.
>
> I hope that clarifies :)
>
> Cheers
>
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Mon, Mar 23, 2020 at 10:26:14PM +0100, Romain Vimont wrote:
> > >
> > >
> > > On 3/23/20 9:14 PM, Alexandre Janniaux wrote:
> > > > Hi,
> > > >
> > > > Why not keep such function in fragment_shader.c, especially
> > > > if part of it has to be moved back afterwards?
> > >
> > > Once the refactor is complete, the idea is to have a "sampler" which
> > > generates "vlc_texture()", a function to expose the input picture (whatever
> > > its storage, orientation, etc.).
> > >
> > > Any OpenGL program can benefit from this "sampler" to draw as they like the
> > > input picture within their own program (vertex + fragment shader), with just
> > > the "vlc_texture()" implementation to be injected.
> > >
> > > (fragment_shader.c is later renamed to sampler.c)
> > >
> > > The "extensions" part of the fragment shader depends on the input picture
> > > (for example, it is necessary for the Android interop), so it must be
> > > handled by the sampler, not the renderer. But to make it work step by step,
> > > for now it's managed by the renderer. In other words, as a first step, only
> > > the "vlc_texture()" GLSL function is injected. The "extensions" part will be
> > > injected later.
> > >
> > > Cheers
> > >
> > > > Regards,
> > > > --
> > > > Alexandre Janniaux
> > > > Videolabs
> > > >
> > > > On Mon, Mar 23, 2020 at 03:55:41PM +0100, Romain Vimont wrote:
> > > > > Make the renderer create the fragment shader, using the vlc_texture()
> > > > > function generated by fragment_shader.c.
> > > > > ---
> > > > > .../video_output/opengl/fragment_shaders.c | 31 +------------
> > > > > modules/video_output/opengl/renderer.c | 46 +++++++++++++++++--
> > > > > 2 files changed, 44 insertions(+), 33 deletions(-)
> > > > >
> > > > > diff --git a/modules/video_output/opengl/fragment_shaders.c b/modules/video_output/opengl/fragment_shaders.c
> > > > > index 4c7b0ae2fa..9bc7e9b6bd 100644
> > > > > --- a/modules/video_output/opengl/fragment_shaders.c
> > > > > +++ b/modules/video_output/opengl/fragment_shaders.c
> > > > > @@ -336,8 +336,6 @@ xyz12_shader_init(struct vlc_gl_renderer *renderer)
> > > > > * - reverse RGB gamma correction
> > > > > */
> > > > > static const char *template =
> > > > > - "#version %u\n"
> > > > > - "%s"
> > > > > "uniform sampler2D Texture0;"
> > > > > "uniform vec4 xyz_gamma = vec4(2.6);"
> > > > > "uniform vec4 rgb_gamma = vec4(1.0/2.2);"
> > > > > @@ -349,7 +347,6 @@ xyz12_shader_init(struct vlc_gl_renderer *renderer)
> > > > > " 0.0, 0.0, 0.0, 1.0 "
> > > > > " );"
> > > > >
> > > > > - "varying vec2 PicCoords;"
> > > > > "vec4 vlc_texture(vec2 pic_coords)\n"
> > > > > "{ "
> > > > > " vec4 v_in, v_out;"
> > > > > @@ -359,17 +356,9 @@ xyz12_shader_init(struct vlc_gl_renderer *renderer)
> > > > > " v_out = pow(v_out, rgb_gamma) ;"
> > > > > " v_out = clamp(v_out, 0.0, 1.0) ;"
> > > > > " return v_out;"
> > > > > - "}\n"
> > > > > - "void main() {\n"
> > > > > - " gl_FragColor = vlc_texture(PicCoords);\n"
> > > > > "}\n";
> > > > >
> > > > > - char *code;
> > > > > - if (asprintf(&code, template, renderer->glsl_version,
> > > > > - renderer->glsl_precision_header) < 0)
> > > > > - return NULL;
> > > > > -
> > > > > - return code;
> > > > > + return strdup(template);
> > > > > }
> > > > >
> > > > > static int
> > > > > @@ -484,15 +473,7 @@ opengl_fragment_shader_init(struct vlc_gl_renderer *renderer, GLenum tex_target,
> > > > > #define ADD(x) vlc_memstream_puts(&ms, x)
> > > > > #define ADDF(x, ...) vlc_memstream_printf(&ms, x, ##__VA_ARGS__)
> > > > >
> > > > > - ADDF("#version %u\n", renderer->glsl_version);
> > > > > -
> > > > > - if (tex_target == GL_TEXTURE_EXTERNAL_OES)
> > > > > - ADDF("#extension GL_OES_EGL_image_external : require\n");
> > > > > -
> > > > > - ADDF("%s", renderer->glsl_precision_header);
> > > > > -
> > > > > - ADD("varying vec2 PicCoords;\n"
> > > > > - "uniform mat4 TransformMatrix;\n"
> > > > > + ADD("uniform mat4 TransformMatrix;\n"
> > > > > "uniform mat4 OrientationMatrix;\n");
> > > > > for (unsigned i = 0; i < interop->tex_count; ++i)
> > > > > ADDF("uniform %s Texture%u;\n"
> > > > > @@ -642,20 +623,12 @@ opengl_fragment_shader_init(struct vlc_gl_renderer *renderer, GLenum tex_target,
> > > > > ADD(" return result * FillColor;\n"
> > > > > "}\n");
> > > > >
> > > > > - ADD("void main() {\n"
> > > > > - " gl_FragColor = vlc_texture(PicCoords);\n"
> > > > > - "}\n");
> > > > > -
> > > > > #undef ADD
> > > > > #undef ADDF
> > > > >
> > > > > if (vlc_memstream_close(&ms) != 0)
> > > > > return NULL;
> > > > >
> > > > > - if (renderer->b_dump_shaders)
> > > > > - msg_Dbg(renderer->gl, "\n=== Fragment shader for fourcc: %4.4s, colorspace: %d ===\n%s\n",
> > > > > - (const char *)&chroma, yuv_space, ms.ptr);
> > > > > -
> > > > > renderer->pf_fetch_locations = renderer_base_fetch_locations;
> > > > > renderer->pf_prepare_shader = renderer_base_prepare_shader;
> > > > >
> > > > > diff --git a/modules/video_output/opengl/renderer.c b/modules/video_output/opengl/renderer.c
> > > > > index 3aaf3b59b9..ae4179be4a 100644
> > > > > --- a/modules/video_output/opengl/renderer.c
> > > > > +++ b/modules/video_output/opengl/renderer.c
> > > > > @@ -273,6 +273,47 @@ BuildVertexShader(const struct vlc_gl_renderer *renderer)
> > > > > return code;
> > > > > }
> > > > >
> > > > > +static char *
> > > > > +BuildFragmentShader(struct vlc_gl_renderer *renderer)
> > > > > +{
> > > > > + struct vlc_gl_interop *interop = renderer->interop;
> > > > > + char *vlc_texture =
> > > > > + opengl_fragment_shader_init(renderer, interop->tex_target,
> > > > > + interop->sw_fmt.i_chroma,
> > > > > + interop->sw_fmt.space);
> > > > > + if (!vlc_texture)
> > > > > + return NULL;
> > > > > +
> > > > > + static const char *template =
> > > > > + "#version %u\n"
> > > > > + "%s" /* extensions */
> > > > > + "%s" /* precision header */
> > > > > + "%s" /* vlc_texture definition */
> > > > > + "varying vec2 PicCoords;\n"
> > > > > + "void main() {\n"
> > > > > + " gl_FragColor = vlc_texture(PicCoords);\n"
> > > > > + "}\n";
> > > > > +
> > > > > + /* TODO move extensions back to fragment_shaders.c */
> > > > > + const char *extensions = interop->tex_target == GL_TEXTURE_EXTERNAL_OES
> > > > > + ? "#extension GL_OES_EGL_image_external : require\n"
> > > > > + : "";
> > > > > +
> > > > > + char *code;
> > > > > + int ret = asprintf(&code, template, renderer->glsl_version, extensions,
> > > > > + renderer->glsl_precision_header, vlc_texture);
> > > > > + free(vlc_texture);
> > > > > + if (ret < 0)
> > > > > + return NULL;
> > > > > +
> > > > > + if (renderer->b_dump_shaders)
> > > > > + msg_Dbg(renderer->gl, "\n=== Fragment shader for fourcc: %4.4s, colorspace: %d ===\n%s\n",
> > > > > + (const char *) &interop->sw_fmt.i_chroma,
> > > > > + interop->sw_fmt.space, code);
> > > > > +
> > > > > + return code;
> > > > > +}
> > > > > +
> > > > > static int
> > > > > opengl_link_program(struct vlc_gl_renderer *renderer)
> > > > > {
> > > > > @@ -283,10 +324,7 @@ opengl_link_program(struct vlc_gl_renderer *renderer)
> > > > > if (!vertex_shader)
> > > > > return VLC_EGENERIC;
> > > > >
> > > > > - char *fragment_shader =
> > > > > - opengl_fragment_shader_init(renderer, interop->tex_target,
> > > > > - interop->sw_fmt.i_chroma,
> > > > > - interop->sw_fmt.space);
> > > > > + char *fragment_shader = BuildFragmentShader(renderer);
> > > > > if (!fragment_shader)
> > > > > {
> > > > > free(vertex_shader);
> > > > > --
> > > > > 2.26.0.rc2
> > > > >
> > > > > _______________________________________________
> > > > > 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