[vlc-devel] [vlc-commits] vlc_opengl: refactor vlc_gl_t creation
Thomas Guillem
thomas at gllm.fr
Mon Dec 19 17:50:38 CET 2016
On Mon, Dec 19, 2016, at 17:02, Rémi Denis-Courmont wrote:
> 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.
I don't see how to do it differently. Some video outputs don't have a
valid window and can't use the legacy vlc_gl_Create() function (unless
we do an ugly cast).
> --
> Rémi Denis-Courmont
> _______________________________________________
> 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