[vlc-devel] [PATCH 2/2] opengl: move fragment shader creation to renderer

Alexandre Janniaux ajanni at videolabs.io
Tue Mar 24 09:55:11 CET 2020


Sorry, might question might have been confusing.

Creating a sampler (from sampler.c) will be shared by
sub_renderer and renderer? 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.

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


More information about the vlc-devel mailing list