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

Romain Vimont rom1v at videolabs.io
Tue Mar 24 11:58:03 CET 2020


OK, I edited the commit message:
https://code.videolan.org/rom1v/vlc/-/commit/2b29f92c40a5b6501e9093a139255ba2a82c803b?merge_request_iid=6

On 3/24/20 11:22 AM, Alexandre Janniaux wrote:
> 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
> _______________________________________________
> 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