[vlc-devel] [vlc-commits] vlc_opengl: refactor vlc_gl_t creation
Rémi Denis-Courmont
remi at remlab.net
Mon Dec 19 17:02:43 CET 2016
On December 19, 2016 5:41:26 PM GMT+02:00, Thomas Guillem <thomas at gllm.fr> wrote:
>
>
>On Mon, Dec 19, 2016, at 15:59, Rémi Denis-Courmont wrote:
>> On December 19, 2016 4:04:07 PM GMT+02:00, git at videolan.org wrote:
>> >vlc | branch: master | Filip Roséen <filip at atch.se> | Mon Dec 19
>> >03:07:43 2016 +0100| [79b2187631f2786f46a8225b538f3ec7ae7d1c9e] |
>> >committer: Thomas Guillem
>> >
>> >vlc_opengl: refactor vlc_gl_t creation
>> >
>> >Given that there are places in the codebase that just require a
>> >created object of type vlc_gl_t ("gl"), these changes split the
>object
>> >creation into two functions:
>> >
>> > - vlc_gl_Create: a simple wrapper around vlc_custom_create
>> > - vlc_gl_ModuleCreate: creates a vlc_gl_t with an associated
>> > module/surface.
>> >
>> >refs #17795
>> >
>> >Signed-off-by: Thomas Guillem <thomas at gllm.fr>
>> >
>> >>
>>
>>http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=79b2187631f2786f46a8225b538f3ec7ae7d1c9e
>> >---
>> >
>> > include/vlc_opengl.h | 22 ++++++++++++++-
>> > modules/video_output/opengl/display.c | 2 +-
>> > src/libvlccore.sym | 1 +
>> >src/video_output/opengl.c | 51
>> >++++++++++++++++-------------------
>> > 4 files changed, 46 insertions(+), 30 deletions(-)
>> >
>> >diff --git a/include/vlc_opengl.h b/include/vlc_opengl.h
>> >index 53d73d0..f7704bc 100644
>> >--- a/include/vlc_opengl.h
>> >+++ b/include/vlc_opengl.h
>> >@@ -58,7 +58,27 @@ enum {
>> > VLC_OPENGL_ES2,
>> > };
>> >
>> >-VLC_API vlc_gl_t *vlc_gl_Create(struct vout_window_t *, unsigned,
>> >const char *) VLC_USED;
>> >+/**
>> >+ * Create a VLC OpenGL object
>> >+ *
>> >+ * @note The allocated resource, if any, shall be released
>> >+ * with a call to \ref vlc_gl_Destroy.
>> >+ * @return a created object on success, NULL on failure.
>> >+ **/
>> >+VLC_API vlc_gl_t *vlc_gl_Create(vlc_object_t* parent);
>> >+
>> >+/**
>> >+ * Creates a VLC OpenGL object with associated surface and module
>> >+ *
>> >+ * @note In most cases, you should vlc_gl_MakeCurrent() afterward.
>> >+ *
>> >+ * @param wnd window to use as OpenGL surface
>> >+ * @param flags OpenGL context type
>> >+ * @param name module name (or NULL for auto)
>> >+ * @return a new context, or NULL on failure
>> >+ */
>> >+VLC_API vlc_gl_t *vlc_gl_ModuleCreate(struct vout_window_t *wnd,
>> >unsigned flags,
>> >+ const char *name) VLC_USED;
>> > VLC_API void vlc_gl_Destroy(vlc_gl_t *);
>> >
>> > static inline int vlc_gl_MakeCurrent(vlc_gl_t *gl)
>> >diff --git a/modules/video_output/opengl/display.c
>> >b/modules/video_output/opengl/display.c
>> >index eefb5cf..68418b3 100644
>> >--- a/modules/video_output/opengl/display.c
>> >+++ b/modules/video_output/opengl/display.c
>> >@@ -107,7 +107,7 @@ static int Open (vlc_object_t *obj)
>> > goto error;
>> > }
>> >
>> >- sys->gl = vlc_gl_Create (surface, API, "$" MODULE_VARNAME);
>> >+ sys->gl = vlc_gl_ModuleCreate (surface, API, "$"
>MODULE_VARNAME);
>> > if (sys->gl == NULL)
>> > goto error;
>> >
>> >diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>> >index 7b9657a..b65f538 100644
>> >--- a/src/libvlccore.sym
>> >+++ b/src/libvlccore.sym
>> >@@ -691,6 +691,7 @@ vlc_fifo_GetCount
>> > vlc_fifo_GetBytes
>> > vlc_gl_Create
>> > vlc_gl_Destroy
>> >+vlc_gl_ModuleCreate
>> > vlc_gl_surface_Create
>> > vlc_gl_surface_CheckSize
>> > vlc_gl_surface_Destroy
>> >diff --git a/src/video_output/opengl.c b/src/video_output/opengl.c
>> >index ac16c3f..6459cbf 100644
>> >--- a/src/video_output/opengl.c
>> >+++ b/src/video_output/opengl.c
>> >@@ -29,47 +29,43 @@
>> > #include <vlc_opengl.h>
>> > #include "libvlc.h"
>> > #include <vlc_modules.h>
>> >+#include <vlc_vout_window.h>
>> >
>> >-#undef vlc_gl_Create
>> >-/**
>> >- * Creates an OpenGL context (and its underlying surface).
>> >- *
>> >- * @note In most cases, you should vlc_gl_MakeCurrent() afterward.
>> >- *
>> >- * @param wnd window to use as OpenGL surface
>> >- * @param flags OpenGL context type
>> >- * @param name module name (or NULL for auto)
>> >- * @return a new context, or NULL on failure
>> >- */
>> >-vlc_gl_t *vlc_gl_Create(struct vout_window_t *wnd, unsigned flags,
>> >- const char *name)
>> >+vlc_gl_t *vlc_gl_Create(vlc_object_t* parent)
>> > {
>> >- vlc_object_t *parent = (vlc_object_t *)wnd;
>> >- vlc_gl_t *gl;
>> >- const char *type;
>> >+ return vlc_custom_create(parent, sizeof(vlc_gl_t), "gl");
>> >+}
>> >
>> >- switch (flags /*& VLC_OPENGL_API_MASK*/)
>> >+vlc_gl_t *vlc_gl_ModuleCreate(struct vout_window_t *wnd, unsigned
>> >flags,
>> >+ const char *module_name)
>> >+{
>> >+ const char *module_type;
>> >+
>> >+ switch (flags)
>> > {
>> > case VLC_OPENGL:
>> >- type = "opengl";
>> >+ module_type = "opengl";
>> > break;
>> > case VLC_OPENGL_ES:
>> >- type = "opengl es";
>> >+ module_type = "opengl es";
>> > break;
>> > case VLC_OPENGL_ES2:
>> >- type = "opengl es2";
>> >+ module_type = "opengl es2";
>> > break;
>> >+
>> > default:
>> > return NULL;
>> > }
>> >
>> >- gl = vlc_custom_create(parent, sizeof (*gl), "gl");
>> >- if (unlikely(gl == NULL))
>> >+ vlc_gl_t *gl = vlc_gl_Create(VLC_OBJECT(wnd));
>> >+
>> >+ if( unlikely( !gl ) )
>> > return NULL;
>> >
>> > gl->surface = wnd;
>> >- gl->module = module_need(gl, type, name, true);
>> >- if (gl->module == NULL)
>> >+ gl->module = module_need(gl, module_type, module_name, true);
>> >+
>> >+ if (gl->module == NULL )
>> > {
>> > vlc_object_release(gl);
>> > return NULL;
>> >@@ -80,12 +76,11 @@ vlc_gl_t *vlc_gl_Create(struct vout_window_t
>*wnd,
>> >unsigned flags,
>> >
>> > void vlc_gl_Destroy(vlc_gl_t *gl)
>> > {
>> >- module_unneed(gl, gl->module);
>> >+ if(gl->module)
>> >+ module_unneed(gl, gl->module);
>> > vlc_object_release(gl);
>> > }
>> >
>> >-#include <vlc_vout_window.h>
>> >-
>> > typedef struct vlc_gl_surface
>> > {
>> > int width;
>> >@@ -130,7 +125,7 @@ vlc_gl_t *vlc_gl_surface_Create(vlc_object_t
>*obj,
>> > *wp = surface;
>> >
>> > /* TODO: support ES? */
>> >- vlc_gl_t *gl = vlc_gl_Create(surface, VLC_OPENGL, NULL);
>> >+ vlc_gl_t *gl = vlc_gl_ModuleCreate(surface, VLC_OPENGL, NULL);
>> > if (gl == NULL) {
>> > vout_window_Delete(surface);
>> > return NULL;
>> >
>> >_______________________________________________
>> >vlc-commits mailing list
>> >vlc-commits at videolan.org
>> >https://mailman.videolan.org/listinfo/vlc-commits
>>
>> This API makes no sense whatsoever, please revert.
>
>Why does it make no sense ? Do you have an other solution in mind ?
>
>> --
>> Rémi Denis-Courmont
>> _______________________________________________
>> 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
Making an API less logical and more complex when you can achieve the same with the existing API does not make sense.
--
Rémi Denis-Courmont
More information about the vlc-devel
mailing list