[vlc-devel] [PATCH 2/2] opengl: move fragment shader creation to renderer
Romain Vimont
rom1v at videolabs.io
Tue Mar 24 11:15:21 CET 2020
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
>
More information about the vlc-devel
mailing list