[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