[vlc-commits] opengl: converter: refactor pf_init

Thomas Guillem git at videolan.org
Thu Feb 2 09:52:48 CET 2017


vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | Tue Jan 31 18:18:38 2017 +0100| [065aeb5fd11b61241dc584bf7524c7d775276f70] | committer: Thomas Guillem

opengl: converter: refactor pf_init

Return directly the fragment_shader from pf_init. The vout_helper is now
responsible of deleting fragment shaders. pf_release if not mandatory anymore.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=065aeb5fd11b61241dc584bf7524c7d775276f70
---

 modules/video_output/opengl/converter_android.c | 16 +++---
 modules/video_output/opengl/converters.c        | 65 ++++++++++++-------------
 modules/video_output/opengl/internal.h          | 19 +++-----
 modules/video_output/opengl/vout_helper.c       | 47 ++++++++----------
 4 files changed, 66 insertions(+), 81 deletions(-)

diff --git a/modules/video_output/opengl/converter_android.c b/modules/video_output/opengl/converter_android.c
index b651597..7075c04 100644
--- a/modules/video_output/opengl/converter_android.c
+++ b/modules/video_output/opengl/converter_android.c
@@ -176,8 +176,6 @@ tc_anop_prepare_shader(const opengl_tex_converter_t *tc,
 static void
 tc_anop_release(const opengl_tex_converter_t *tc)
 {
-    tc->api->DeleteShader(tc->fragment_shader);
-
     struct priv *priv = tc->priv;
     if (priv->stex != NULL)
         SurfaceTexture_release(priv->stex);
@@ -185,16 +183,16 @@ tc_anop_release(const opengl_tex_converter_t *tc)
     free(priv);
 }
 
-int
+GLuint
 opengl_tex_converter_anop_init(const video_format_t *fmt,
                                opengl_tex_converter_t *tc)
 {
     if (fmt->i_chroma != VLC_CODEC_ANDROID_OPAQUE)
-        return VLC_EGENERIC;
+        return 0;
 
     tc->priv = malloc(sizeof(struct priv));
     if (unlikely(tc->priv == NULL))
-        return VLC_ENOMEM;
+        return 0;
 
     struct priv *priv = tc->priv;
     priv->stex = NULL;
@@ -263,9 +261,9 @@ opengl_tex_converter_anop_init(const video_format_t *fmt,
         "{ "
         "  gl_FragColor = texture2D(sTexture, (uSTMatrix * TexCoord0).xy);"
         "}";
-    tc->fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
-    tc->api->ShaderSource(tc->fragment_shader, 1, &code, NULL);
-    tc->api->CompileShader(tc->fragment_shader);
+    GLuint fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
+    tc->api->ShaderSource(fragment_shader, 1, &code, NULL);
+    tc->api->CompileShader(fragment_shader);
 
-    return VLC_SUCCESS;
+    return fragment_shader;
 }
diff --git a/modules/video_output/opengl/converters.c b/modules/video_output/opengl/converters.c
index 5e0a03e..a8631e4 100644
--- a/modules/video_output/opengl/converters.c
+++ b/modules/video_output/opengl/converters.c
@@ -421,9 +421,6 @@ tc_common_update(const opengl_tex_converter_t *tc, GLuint *textures,
 static void
 tc_common_release(const opengl_tex_converter_t *tc)
 {
-    if (tc->fragment_shader != 0)
-        tc->api->DeleteShader(tc->fragment_shader);
-
     struct priv *priv = tc->priv;
     free(priv->texture_temp_buf);
 
@@ -499,16 +496,16 @@ tc_rgba_prepare_shader(const opengl_tex_converter_t *tc,
     tc->api->Uniform4f(priv->uloc.FillColor, 1.0f, 1.0f, 1.0f, alpha);
 }
 
-int
+GLuint
 opengl_tex_converter_rgba_init(const video_format_t *fmt,
                                opengl_tex_converter_t *tc)
 {
     if (fmt->i_chroma != VLC_CODEC_RGBA && fmt->i_chroma != VLC_CODEC_RGB32)
-        return VLC_EGENERIC;
+        return 0;
 
     if (common_init(tc, sizeof(struct priv), VLC_CODEC_RGBA,
                     GL_RGBA, GL_RGBA, GL_UNSIGNED_BYTE) != VLC_SUCCESS)
-        return VLC_ENOMEM;
+        return 0;
 
     tc->pf_fetch_locations = tc_rgba_fetch_locations;
     tc->pf_prepare_shader = tc_rgba_prepare_shader;
@@ -538,15 +535,15 @@ opengl_tex_converter_rgba_init(const video_format_t *fmt,
         "  gl_FragColor = texture2D(Texture0, TexCoord0.st) * FillColor;"
         "}";
 
-    tc->fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
-    if (tc->fragment_shader == 0)
+    GLuint fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
+    if (fragment_shader == 0)
     {
         tc_common_release(tc);
-        return VLC_EGENERIC;
+        return 0;
     }
-    tc->api->ShaderSource(tc->fragment_shader, 1, &code, NULL);
-    tc->api->CompileShader(tc->fragment_shader);
-    return VLC_SUCCESS;
+    tc->api->ShaderSource(fragment_shader, 1, &code, NULL);
+    tc->api->CompileShader(fragment_shader);
+    return fragment_shader;
 }
 
 #if !defined(USE_OPENGL_ES2)
@@ -605,18 +602,18 @@ tc_yuv_prepare_shader(const opengl_tex_converter_t *tc,
     tc->api->Uniform1i(priv->uloc.Texture2, 2);
 }
 
-int
+GLuint
 opengl_tex_converter_yuv_init(const video_format_t *fmt,
                               opengl_tex_converter_t *tc)
 {
     if (!vlc_fourcc_IsYUV(fmt->i_chroma))
-        return VLC_EGENERIC;
+        return 0;
 
     GLint max_texture_units = 0;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &max_texture_units);
 
     if (max_texture_units < 3)
-        return VLC_EGENERIC;
+        return 0;
 
 #if !defined(USE_OPENGL_ES2)
     const unsigned char *ogl_version = glGetString(GL_VERSION);
@@ -638,7 +635,7 @@ opengl_tex_converter_yuv_init(const video_format_t *fmt,
             if (common_init(tc, sizeof(struct yuv_priv), *list,
                             yuv_plane_texformat, yuv_plane_texformat,
                             GL_UNSIGNED_BYTE) != VLC_SUCCESS)
-                return VLC_ENOMEM;
+                return 0;
 
             yuv_range_correction = 1.0;
             break;
@@ -652,7 +649,7 @@ opengl_tex_converter_yuv_init(const video_format_t *fmt,
             if (common_init(tc, sizeof(struct yuv_priv), *list,
                             yuv_plane_texformat_16, yuv_plane_texformat,
                             GL_UNSIGNED_SHORT) != VLC_SUCCESS)
-                return VLC_ENOMEM;
+                return 0;
 
             yuv_range_correction = (float)((1 << 16) - 1)
                                  / ((1 << dsc->pixel_bits) - 1);
@@ -662,7 +659,7 @@ opengl_tex_converter_yuv_init(const video_format_t *fmt,
         list++;
     }
     if (!*list)
-        return VLC_EGENERIC;
+        return 0;
 
     tc->pf_fetch_locations = tc_yuv_fetch_locations;
     tc->pf_prepare_shader = tc_yuv_prepare_shader;
@@ -731,7 +728,7 @@ opengl_tex_converter_yuv_init(const video_format_t *fmt,
                  swap_uv ? 'y' : 'z') < 0)
     {
         tc_common_release(tc);
-        return VLC_ENOMEM;
+        return 0;
     }
 
     for (int i = 0; i < 4; i++) {
@@ -743,18 +740,18 @@ opengl_tex_converter_yuv_init(const video_format_t *fmt,
             local_value[i*4+j] = j < 3 ? correction * matrix[j*4+i] : 0.f;
     }
 
-    tc->fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
-    if (tc->fragment_shader == 0)
+    GLuint fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
+    if (fragment_shader == 0)
     {
         tc_common_release(tc);
         free(code);
-        return VLC_EGENERIC;
+        return 0;
     }
-    tc->api->ShaderSource(tc->fragment_shader, 1, (const char **)&code, NULL);
-    tc->api->CompileShader(tc->fragment_shader);
+    tc->api->ShaderSource(fragment_shader, 1, (const char **)&code, NULL);
+    tc->api->CompileShader(fragment_shader);
     free(code);
 
-    return VLC_SUCCESS;
+    return fragment_shader;
 }
 
 static int
@@ -775,16 +772,16 @@ tc_xyz12_prepare_shader(const opengl_tex_converter_t *tc,
     tc->api->Uniform1i(priv->uloc.Texture0, 0);
 }
 
-int
+GLuint
 opengl_tex_converter_xyz12_init(const video_format_t *fmt,
                                 opengl_tex_converter_t *tc)
 {
     if (fmt->i_chroma != VLC_CODEC_XYZ12)
-        return VLC_EGENERIC;
+        return 0;
 
     if (common_init(tc, sizeof(struct priv), VLC_CODEC_XYZ12,
                     GL_RGB, GL_RGB, GL_UNSIGNED_SHORT) != VLC_SUCCESS)
-        return VLC_ENOMEM;
+        return 0;
 
     tc->pf_fetch_locations = tc_xyz12_fetch_locations;
     tc->pf_prepare_shader = tc_xyz12_prepare_shader;
@@ -821,13 +818,13 @@ opengl_tex_converter_xyz12_init(const video_format_t *fmt,
         " gl_FragColor = v_out;"
         "}";
 
-    tc->fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
-    if (tc->fragment_shader == 0)
+    GLuint fragment_shader = tc->api->CreateShader(GL_FRAGMENT_SHADER);
+    if (fragment_shader == 0)
     {
         tc_common_release(tc);
-        return VLC_EGENERIC;
+        return 0;
     }
-    tc->api->ShaderSource(tc->fragment_shader, 1, &code, NULL);
-    tc->api->CompileShader(tc->fragment_shader);
-    return VLC_SUCCESS;
+    tc->api->ShaderSource(fragment_shader, 1, &code, NULL);
+    tc->api->CompileShader(fragment_shader);
+    return fragment_shader;
 }
diff --git a/modules/video_output/opengl/internal.h b/modules/video_output/opengl/internal.h
index c8b880c..19733c7 100644
--- a/modules/video_output/opengl/internal.h
+++ b/modules/video_output/opengl/internal.h
@@ -148,8 +148,8 @@ typedef struct opengl_tex_converter_t opengl_tex_converter_t;
  * \param fc OpenGL tex converter that needs to be filled on success
  * \return VLC_SUCCESS or a VLC error
  */
-typedef int (*opengl_tex_converter_init_cb)(const video_format_t *fmt,
-                                            opengl_tex_converter_t *fc);
+typedef GLuint (*opengl_tex_converter_init_cb)(const video_format_t *fmt,
+                                               opengl_tex_converter_t *fc);
 
 /*
  * Structure that is filled by an opengl_tex_converter_init_cb function
@@ -173,9 +173,6 @@ struct opengl_tex_converter_t
     /* Texture mapping (usually: GL_TEXTURE_2D), cannot be 0 */
     GLenum tex_target;
 
-    /* The compiled fragment shader, cannot be 0 */
-    GLuint fragment_shader;
-
     /* Private context */
     void *priv;
 
@@ -261,26 +258,26 @@ struct opengl_tex_converter_t
                               float alpha);
 
     /*
-     * Callback to release the shader and the private context
+     * Callback to release the private context
      *
-     * This function pointer cannot be NULL.
+     * This function pointer can be NULL.
      * \param fc OpenGL tex converter
      */
     void (*pf_release)(const opengl_tex_converter_t *fc);
 };
 
-extern int
+extern GLuint
 opengl_tex_converter_rgba_init(const video_format_t *,
                                opengl_tex_converter_t *);
-extern int
+extern GLuint
 opengl_tex_converter_yuv_init(const video_format_t *,
                               opengl_tex_converter_t *);
-extern int
+extern GLuint
 opengl_tex_converter_xyz12_init(const video_format_t *,
                                 opengl_tex_converter_t *);
 
 #ifdef __ANDROID__
-extern int
+extern GLuint
 opengl_tex_converter_anop_init(const video_format_t *,
                                opengl_tex_converter_t *);
 #endif
diff --git a/modules/video_output/opengl/vout_helper.c b/modules/video_output/opengl/vout_helper.c
index ae12343..a312bf4 100644
--- a/modules/video_output/opengl/vout_helper.c
+++ b/modules/video_output/opengl/vout_helper.c
@@ -545,6 +545,7 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
     vgl->fmt.i_gmask  = 0x0000ff00;
     vgl->fmt.i_bmask  = 0x00ff0000;
 #   endif
+    GLuint fragment_shader = 0, sub_fragment_shader;
     opengl_tex_converter_t tex_conv;
     opengl_tex_converter_t sub_tex_conv = {
         .parent = VLC_OBJECT(vgl->gl),
@@ -554,17 +555,16 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
     };
 
     /* RGBA is needed for subpictures or for non YUV pictures */
-    if (opengl_tex_converter_rgba_init(&vgl->fmt, &sub_tex_conv) != VLC_SUCCESS)
+    sub_fragment_shader = opengl_tex_converter_rgba_init(&vgl->fmt, &sub_tex_conv);
+    if (sub_fragment_shader == 0)
     {
         msg_Err(gl, "RGBA shader failed");
         free(vgl);
         return NULL;
     }
-    assert(sub_tex_conv.fragment_shader != 0);
 
     const video_format_t *fmts[2] = { fmt, &vgl->fmt };
-    int ret = VLC_EGENERIC;
-    for (size_t i = 0; i < 2 && ret != VLC_SUCCESS; ++i)
+    for (size_t i = 0; i < 2 && fragment_shader == 0; ++i)
     {
         /* Try first the untouched fmt, then the rgba fmt */
         for (size_t j = 0; j < ARRAY_SIZE(opengl_tex_converter_init_cbs); ++j)
@@ -575,47 +575,37 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
                 .glexts = extensions,
                 .orientation = fmt->orientation,
             };
-            ret = opengl_tex_converter_init_cbs[j](fmts[i], &tex_conv);
-            if (ret == VLC_SUCCESS)
+            fragment_shader = opengl_tex_converter_init_cbs[j](fmts[i], &tex_conv);
+            if (fragment_shader != 0)
             {
                 assert(tex_conv.chroma != 0 && tex_conv.tex_target != 0 &&
-                       tex_conv.fragment_shader != 0 &&
                        tex_conv.pf_update != NULL &&
                        tex_conv.pf_fetch_locations != NULL &&
-                       tex_conv.pf_prepare_shader != NULL &&
-                       tex_conv.pf_release != NULL);
+                       tex_conv.pf_prepare_shader != NULL);
                 vgl->fmt = *fmt;
                 vgl->fmt.i_chroma = tex_conv.chroma;
                 break;
             }
         }
     }
-    if (ret != VLC_SUCCESS)
+    if (fragment_shader == 0)
     {
         msg_Err(gl, "could not init tex converter");
-        sub_tex_conv.pf_release(&sub_tex_conv);
+        if (sub_tex_conv.pf_release != NULL)
+            sub_tex_conv.pf_release(&sub_tex_conv);
+        vgl->api.DeleteShader(sub_fragment_shader);
         free(vgl);
         return NULL;
     }
-    assert(tex_conv.fragment_shader != 0);
 
     /* Build program if needed */
     GLuint vertex_shader, sub_vertex_shader;
     vertex_shader = BuildVertexShader(vgl, tex_conv.desc->plane_count);
     sub_vertex_shader = BuildVertexShader(vgl, sub_tex_conv.desc->plane_count);
-    if (vertex_shader == 0 || sub_vertex_shader == 0)
-    {
-        vgl->api.DeleteShader(vertex_shader);
-        vgl->api.DeleteShader(sub_vertex_shader);
-        tex_conv.pf_release(&tex_conv);
-        sub_tex_conv.pf_release(&sub_tex_conv);
-        free(vgl);
-        return NULL;
-    }
 
-    GLuint shaders[] = {
-        tex_conv.fragment_shader,
-        sub_tex_conv.fragment_shader,
+    const GLuint shaders[] = {
+        fragment_shader,
+        sub_fragment_shader,
         vertex_shader,
         sub_vertex_shader
     };
@@ -642,7 +632,7 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
     vgl->prgm = &vgl->prgms[0];
     vgl->prgm->tc = tex_conv;
     vgl->prgm->id = vgl->api.CreateProgram();
-    vgl->api.AttachShader(vgl->prgm->id, tex_conv.fragment_shader);
+    vgl->api.AttachShader(vgl->prgm->id, fragment_shader);
     vgl->api.AttachShader(vgl->prgm->id, vertex_shader);
     vgl->api.LinkProgram(vgl->prgm->id);
 
@@ -650,12 +640,14 @@ vout_display_opengl_t *vout_display_opengl_New(video_format_t *fmt,
     vgl->sub_prgm = &vgl->prgms[1];
     vgl->sub_prgm->tc = sub_tex_conv;
     vgl->sub_prgm->id = vgl->api.CreateProgram();
-    vgl->api.AttachShader(vgl->sub_prgm->id, sub_tex_conv.fragment_shader);
+    vgl->api.AttachShader(vgl->sub_prgm->id, sub_fragment_shader);
     vgl->api.AttachShader(vgl->sub_prgm->id, sub_vertex_shader);
     vgl->api.LinkProgram(vgl->sub_prgm->id);
 
     vgl->api.DeleteShader(vertex_shader);
+    vgl->api.DeleteShader(fragment_shader);
     vgl->api.DeleteShader(sub_vertex_shader);
+    vgl->api.DeleteShader(sub_fragment_shader);
 
     /* Check program messages */
     for (GLuint i = 0; i < 2; i++) {
@@ -820,7 +812,8 @@ void vout_display_opengl_Delete(vout_display_opengl_t *vgl)
     {
         vgl->api.DeleteProgram(vgl->prgms[i].id);
         opengl_tex_converter_t *tc = &vgl->prgms[i].tc;
-        tc->pf_release(tc);
+        if (tc->pf_release != NULL)
+            tc->pf_release(tc);
     }
     vgl->api.DeleteBuffers(1, &vgl->vertex_buffer_object);
     vgl->api.DeleteBuffers(1, &vgl->index_buffer_object);



More information about the vlc-commits mailing list