From 940151bcd605fe5cdfc61a6fce11abfbd53eb7db Mon Sep 17 00:00:00 2001 From: Julien Isorce Date: Wed, 28 Aug 2013 10:07:24 +0100 Subject: [PATCH] [799/906] glimagesink: fix asynchrone drawing Simplify gst_glimage_sink_redisplay which is there only to ask the window for a redraw. Put a lock to make sure we are not realeasing the stored buffer while still drawing the corresponding texture --- gst/gl/gstglimagesink.c | 114 +++++++++++++++++++++++++--------------- gst/gl/gstglimagesink.h | 10 ++-- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/gst/gl/gstglimagesink.c b/gst/gl/gstglimagesink.c index 9741622dca..f438e57618 100644 --- a/gst/gl/gstglimagesink.c +++ b/gst/gl/gstglimagesink.c @@ -90,6 +90,13 @@ GST_DEBUG_CATEGORY (gst_debug_glimage_sink); #define GST_CAT_DEFAULT gst_debug_glimage_sink +#define GST_GLIMAGE_SINK_GET_LOCK(glsink) \ + (GST_GLIMAGE_SINK(glsink)->drawing_lock) +#define GST_GLIMAGE_SINK_LOCK(glsink) \ + (g_mutex_lock(&GST_GLIMAGE_SINK_GET_LOCK (glsink))) +#define GST_GLIMAGE_SINK_UNLOCK(glsink) \ + (g_mutex_unlock(&GST_GLIMAGE_SINK_GET_LOCK (glsink))) + #define USING_OPENGL(display) (display->gl_api & GST_GL_API_OPENGL) #define USING_OPENGL3(display) (display->gl_api & GST_GL_API_OPENGL3) #define USING_GLES(display) (display->gl_api & GST_GL_API_GLES) @@ -100,12 +107,10 @@ GST_DEBUG_CATEGORY (gst_debug_glimage_sink); static void gst_glimage_sink_thread_init_redisplay (GstGLImageSink * gl_sink); #endif static void gst_glimage_sink_on_close (GstGLImageSink * gl_sink); -static void gst_glimage_sink_on_resize (GstGLImageSink * gl_sink, gint width, - gint height); -static void gst_glimage_sink_on_draw (GstGLImageSink * gl_sink); -static gboolean gst_glimage_sink_redisplay (GstGLImageSink * gl_sink, - GLuint texture, gint gl_width, gint gl_height, gint window_width, - gint window_height, gboolean keep_aspect_ratio); +static void gst_glimage_sink_on_resize (const GstGLImageSink * gl_sink, + gint width, gint height); +static void gst_glimage_sink_on_draw (const GstGLImageSink * gl_sink); +static gboolean gst_glimage_sink_redisplay (GstGLImageSink * gl_sink); static void gst_glimage_sink_finalize (GObject * object); static void gst_glimage_sink_set_property (GObject * object, guint prop_id, @@ -254,13 +259,17 @@ gst_glimage_sink_init (GstGLImageSink * glimage_sink) glimage_sink->window_id = 0; glimage_sink->new_window_id = 0; glimage_sink->display = NULL; - glimage_sink->stored_buffer = NULL; glimage_sink->clientReshapeCallback = NULL; glimage_sink->clientDrawCallback = NULL; glimage_sink->client_data = NULL; glimage_sink->keep_aspect_ratio = FALSE; glimage_sink->par_n = 0; glimage_sink->par_d = 1; + glimage_sink->pool = NULL; + glimage_sink->stored_buffer = NULL; + glimage_sink->redisplay_texture = 0; + + g_mutex_init (&glimage_sink->drawing_lock); } static void @@ -326,6 +335,8 @@ gst_glimage_sink_finalize (GObject * object) glimage_sink->pool = NULL; } + g_mutex_clear (&glimage_sink->drawing_lock); + g_free (glimage_sink->display_name); GST_DEBUG ("finalized"); @@ -574,6 +585,7 @@ gst_glimage_sink_set_caps (GstBaseSink * bsink, GstCaps * caps) if (glimage_sink->tex_id) gst_gl_display_del_texture (glimage_sink->display, &glimage_sink->tex_id); + //FIXME: this texture seems to be never deleted when going to STATE_NULL gst_gl_display_gen_texture (glimage_sink->display, &glimage_sink->tex_id, GST_VIDEO_INFO_FORMAT (&vinfo), width, height); @@ -686,7 +698,8 @@ gst_glimage_sink_render (GstBaseSink * bsink, GstBuffer * buf) return GST_FLOW_ERROR; } } - + //FIXME: here stored_buffer is not usefull + // so here we have to use 2 textures (stored_texture and tex) if (!gst_gl_upload_perform_with_data (glimage_sink->upload, glimage_sink->tex_id, frame.data)) { GST_ELEMENT_ERROR (glimage_sink, RESOURCE, NOT_FOUND, @@ -708,21 +721,20 @@ gst_glimage_sink_render (GstBaseSink * bsink, GstBuffer * buf) gst_object_unref (context); } - gst_buffer_replace (&glimage_sink->stored_buffer, buf); - GST_TRACE ("redisplay texture:%u of size:%ux%u, window size:%ux%u", tex_id, GST_VIDEO_INFO_WIDTH (&glimage_sink->info), GST_VIDEO_INFO_HEIGHT (&glimage_sink->info), GST_VIDEO_SINK_WIDTH (glimage_sink), GST_VIDEO_SINK_HEIGHT (glimage_sink)); - //redisplay opengl scene - if (!gst_glimage_sink_redisplay (glimage_sink, - tex_id, GST_VIDEO_INFO_WIDTH (&glimage_sink->info), - GST_VIDEO_INFO_HEIGHT (&glimage_sink->info), - GST_VIDEO_SINK_WIDTH (glimage_sink), - GST_VIDEO_SINK_HEIGHT (glimage_sink), - glimage_sink->keep_aspect_ratio)) + /* Avoid to release the texture while drawing */ + GST_GLIMAGE_SINK_LOCK (glimage_sink); + glimage_sink->redisplay_texture = tex_id; + gst_buffer_replace (&glimage_sink->stored_buffer, buf); + GST_GLIMAGE_SINK_UNLOCK (glimage_sink); + + /* Ask the underlying window to redraw its content */ + if (!gst_glimage_sink_redisplay (glimage_sink)) goto redisplay_failed; GST_TRACE ("post redisplay"); @@ -790,8 +802,7 @@ gst_glimage_sink_expose (GstVideoOverlay * overlay) gst_object_unref (context); } - gst_glimage_sink_redisplay (glimage_sink, 0, 0, 0, 0, 0, - glimage_sink->keep_aspect_ratio); + gst_glimage_sink_redisplay (glimage_sink); } } @@ -905,8 +916,12 @@ gst_glimage_sink_thread_init_redisplay (GstGLImageSink * gl_sink) #endif static void -gst_glimage_sink_on_resize (GstGLImageSink * gl_sink, gint width, gint height) +gst_glimage_sink_on_resize (const GstGLImageSink * gl_sink, gint width, + gint height) { + /* Here gl_sink members (ex:gl_sink->info) have a life time of set_caps. + * It means that they cannot not change between two set_caps + */ const GstGLFuncs *gl = gl_sink->display->gl_vtable; GST_TRACE ("GL Window resized to %ux%u", width, height); @@ -922,8 +937,8 @@ gst_glimage_sink_on_resize (GstGLImageSink * gl_sink, gint width, gint height) src.x = 0; src.y = 0; - src.w = gl_sink->redisplay_texture_width; - src.h = gl_sink->redisplay_texture_height; + src.w = GST_VIDEO_INFO_WIDTH (&gl_sink->info); + src.h = GST_VIDEO_INFO_HEIGHT (&gl_sink->info); dst.x = 0; dst.y = 0; @@ -948,13 +963,28 @@ gst_glimage_sink_on_resize (GstGLImageSink * gl_sink, gint width, gint height) static void -gst_glimage_sink_on_draw (GstGLImageSink * gl_sink) +gst_glimage_sink_on_draw (const GstGLImageSink * gl_sink) { - const GstGLFuncs *gl = gl_sink->display->gl_vtable; + /* Here gl_sink members (ex:gl_sink->info) have a life time of set_caps. + * It means that they cannot not change between two set_caps as well as + * for the redisplay_texture size. + * Whereas redisplay_texture id changes every sink_render + */ + + const GstGLFuncs *gl = NULL; + + g_return_if_fail (GST_IS_GLIMAGE_SINK (gl_sink)); + + gl = gl_sink->display->gl_vtable; + + GST_GLIMAGE_SINK_LOCK (gl_sink); /* check if texture is ready for being drawn */ - if (!gl_sink->redisplay_texture) + if (!gl_sink->redisplay_texture) { + GST_GLIMAGE_SINK_UNLOCK (gl_sink); return; + } + /* opengl scene */ GST_TRACE ("redrawing texture:%u", gl_sink->redisplay_texture); @@ -973,8 +1003,8 @@ gst_glimage_sink_on_draw (GstGLImageSink * gl_sink) gboolean doRedisplay = gl_sink->clientDrawCallback (gl_sink->redisplay_texture, - gl_sink->redisplay_texture_width, - gl_sink->redisplay_texture_height, + GST_VIDEO_INFO_WIDTH (&gl_sink->info), + GST_VIDEO_INFO_HEIGHT (&gl_sink->info), gl_sink->client_data); if (doRedisplay) { @@ -982,7 +1012,8 @@ gst_glimage_sink_on_draw (GstGLImageSink * gl_sink) GstGLWindow *window = gst_gl_context_get_window (context); gst_gl_window_draw_unlocked (window, - gl_sink->redisplay_texture_width, gl_sink->redisplay_texture_height); + GST_VIDEO_INFO_WIDTH (&gl_sink->info), + GST_VIDEO_INFO_HEIGHT (&gl_sink->info)); gst_object_unref (window); gst_object_unref (context); @@ -997,11 +1028,11 @@ gst_glimage_sink_on_draw (GstGLImageSink * gl_sink) -1.0f, -1.0f, 1.0f, -1.0f }; - GLfloat texcoords[8] = { gl_sink->redisplay_texture_width, 0, + GLfloat texcoords[8] = { GST_VIDEO_INFO_WIDTH (&gl_sink->info), 0, 0, 0, - 0, gl_sink->redisplay_texture_height, - gl_sink->redisplay_texture_width, - gl_sink->redisplay_texture_height + 0, GST_VIDEO_INFO_HEIGHT (&gl_sink->info), + GST_VIDEO_INFO_WIDTH (&gl_sink->info), + GST_VIDEO_INFO_HEIGHT (&gl_sink->info) }; gl->Clear (GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); @@ -1061,6 +1092,8 @@ gst_glimage_sink_on_draw (GstGLImageSink * gl_sink) } #endif } /* end default opengl scene */ + + GST_GLIMAGE_SINK_UNLOCK (gl_sink); } static void @@ -1072,9 +1105,7 @@ gst_glimage_sink_on_close (GstGLImageSink * gl_sink) } static gboolean -gst_glimage_sink_redisplay (GstGLImageSink * gl_sink, GLuint texture, - gint gl_width, gint gl_height, gint window_width, gint window_height, - gboolean keep_aspect_ratio) +gst_glimage_sink_redisplay (GstGLImageSink * gl_sink) { GstGLContext *context; GstGLWindow *window; @@ -1094,14 +1125,11 @@ gst_glimage_sink_redisplay (GstGLImageSink * gl_sink, GLuint texture, } #endif - if (texture) { - gl_sink->redisplay_texture = texture; - gl_sink->redisplay_texture_width = gl_width; - gl_sink->redisplay_texture_height = gl_height; - } - gl_sink->keep_aspect_ratio = keep_aspect_ratio; - if (window) - gst_gl_window_draw (window, window_width, window_height); + /* Drawing is asynchrone: gst_gl_window_draw is not blocking + * It means that it does not wait for stuff being executed in other threads + */ + gst_gl_window_draw (window, GST_VIDEO_SINK_WIDTH (gl_sink), + GST_VIDEO_SINK_HEIGHT (gl_sink)); } alive = gst_gl_window_is_running (window); gst_object_unref (window); diff --git a/gst/gl/gstglimagesink.h b/gst/gl/gstglimagesink.h index 6da610bd18..637166649e 100644 --- a/gst/gl/gstglimagesink.h +++ b/gst/gl/gstglimagesink.h @@ -61,7 +61,6 @@ struct _GstGLImageSink GstVideoInfo info; GstGLDisplay *display; - GstBuffer *stored_buffer; GstGLUpload *upload; GLuint tex_id; @@ -75,10 +74,11 @@ struct _GstGLImageSink GstBufferPool *pool; - /* action redisplay */ - GLuint redisplay_texture; - GLuint redisplay_texture_width; - GLuint redisplay_texture_height; + /* avoid replacing the stored_buffer while drawing */ + GMutex drawing_lock; + GstBuffer *stored_buffer; + GLuint redisplay_texture; + #if GST_GL_HAVE_GLES2 GstGLShader *redisplay_shader; GLint redisplay_attr_position_loc;