glupload: provide the output buffer that is rendered into

Allows callers to properly reference count the buffers used for
rendering.

Fixes a redraw race in glimagesink where the previous buffer
(the one used for redraw operations) is freed as soon as the next
buffer is uploaded.

1. glimagesink uploads in _prepare() to texture n
1.1 glupload holds buffer n
2. glimagesink _render()s texture n
3. glimagesink uploads texture n+1
3.1 glupload free previous buffer which deletes texture n
3.2 glupload holds buffer n+1
4. glwindow resize/expose
5. glimagesink redraws with texture n

The race is that the buffer n (the one used for redrawing) is freed as soon as
the buffer n+1 arrives.  There could be any amount of time and number of
redraws between this event and when buffer n+1 is actually rendered and thus
replaces buffer n as the redraw source.

https://bugzilla.gnome.org/show_bug.cgi?id=736740

ext/gl/gstglimagesink.c

@@ -646,6 +646,7 @@ gst_glimage_sink_change_state (GstElement * element, GstStateChange transition)
         glimage_sink->stored_buffer = NULL;
       }
       GST_GLIMAGE_SINK_UNLOCK (glimage_sink);
+      gst_buffer_replace (&glimage_sink->next_buffer, NULL);
 
       if (glimage_sink->upload) {
         gst_object_unref (glimage_sink->upload);
@@ -822,6 +823,7 @@ static GstFlowReturn
 gst_glimage_sink_prepare (GstBaseSink * bsink, GstBuffer * buf)
 {
   GstGLImageSink *glimage_sink;
+  GstBuffer *next_buffer = NULL;
 
   glimage_sink = GST_GLIMAGE_SINK (bsink);
 
@@ -836,9 +838,12 @@ gst_glimage_sink_prepare (GstBaseSink * bsink, GstBuffer * buf)
     return GST_FLOW_NOT_NEGOTIATED;
 
   if (!gst_gl_upload_perform_with_buffer (glimage_sink->upload, buf,
-          &glimage_sink->next_tex))
+          &glimage_sink->next_tex, &next_buffer))
     goto upload_failed;
 
+  gst_buffer_replace (&glimage_sink->next_buffer, next_buffer);
+  gst_buffer_unref (next_buffer);
+
   if (glimage_sink->window_id != glimage_sink->new_window_id) {
     GstGLWindow *window = gst_gl_context_get_window (glimage_sink->context);
 
@@ -878,10 +883,8 @@ gst_glimage_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
   GST_GLIMAGE_SINK_LOCK (glimage_sink);
   glimage_sink->redisplay_texture = glimage_sink->next_tex;
   stored_buffer = glimage_sink->stored_buffer;
-  glimage_sink->stored_buffer = gst_buffer_ref (buf);
+  glimage_sink->stored_buffer = gst_buffer_ref (glimage_sink->next_buffer);
   GST_GLIMAGE_SINK_UNLOCK (glimage_sink);
-  if (stored_buffer)
-    gst_buffer_unref (stored_buffer);
 
   /* Ask the underlying window to redraw its content */
   if (!gst_glimage_sink_redisplay (glimage_sink))
@@ -889,6 +892,9 @@ gst_glimage_sink_show_frame (GstVideoSink * vsink, GstBuffer * buf)
 
   GST_TRACE ("post redisplay");
 
+  if (stored_buffer)
+    gst_buffer_unref (stored_buffer);
+
   if (g_atomic_int_get (&glimage_sink->to_quit) != 0) {
     GST_ELEMENT_ERROR (glimage_sink, RESOURCE, NOT_FOUND,
         ("%s", gst_gl_context_get_error ()), (NULL));

ext/gl/gstglimagesink.h

@@ -68,6 +68,7 @@ struct _GstGLImageSink
 
     GstGLUpload *upload;
     guint      next_tex;
+    GstBuffer *next_buffer;
 
     volatile gint to_quit;
     gboolean keep_aspect_ratio;

ext/gl/gstglmixer.c

@@ -882,7 +882,7 @@ gst_gl_mixer_process_textures (GstGLMixer * mix, GstBuffer * outbuf)
       }
 
       if (!gst_gl_upload_perform_with_buffer (pad->upload,
-              vaggpad->buffer, &in_tex)) {
+              vaggpad->buffer, &in_tex, NULL)) {
         ++array_index;
         pad->mapped = FALSE;
         continue;

gst-libs/gst/gl/gstglfilter.c

@@ -1175,7 +1175,7 @@ gst_gl_filter_filter_texture (GstGLFilter * filter, GstBuffer * inbuf,
 
   filter_class = GST_GL_FILTER_GET_CLASS (filter);
 
-  if (!gst_gl_upload_perform_with_buffer (filter->upload, inbuf, &in_tex))
+  if (!gst_gl_upload_perform_with_buffer (filter->upload, inbuf, &in_tex, NULL))
     return FALSE;
 
   if (!gst_video_frame_map (&out_frame, &filter->out_info, outbuf,

gst-libs/gst/gl/gstglupload.c

@@ -50,7 +50,8 @@
 static gboolean _upload_memory (GstGLUpload * upload);
 static gboolean _init_upload (GstGLUpload * upload);
 static gboolean _gst_gl_upload_perform_with_data_unlocked (GstGLUpload * upload,
-    GLuint * texture_id, gpointer data[GST_VIDEO_MAX_PLANES]);
+    GLuint * texture_id, gpointer data[GST_VIDEO_MAX_PLANES],
+    GstBuffer ** outbuf);
 static void _do_upload_with_meta (GstGLContext * context, GstGLUpload * upload);
 static void gst_gl_upload_reset (GstGLUpload * upload);
 
@@ -223,6 +224,7 @@ gst_gl_upload_get_format (GstGLUpload * upload)
  * @upload: a #GstGLUpload
  * @buffer: a #GstBuffer
  * @tex_id: resulting texture
+ * @outbuf: (allow-none): resulting buffer
  *
  * Uploads @buffer to the texture given by @tex_id.  @tex_id is valid
  * until gst_gl_upload_release_buffer() is called.
@@ -231,7 +233,7 @@ gst_gl_upload_get_format (GstGLUpload * upload)
  */
 gboolean
 gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer,
-    guint * tex_id)
+    guint * tex_id, GstBuffer ** outbuf)
 {
   GstMemory *mem;
   GstVideoGLTextureUploadMeta *gl_tex_upload_meta;
@@ -257,6 +259,10 @@ gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer,
       gst_memory_unmap (mem, &map_info);
 
       *tex_id = ((GstGLMemory *) mem)->tex_id;
+
+      if (outbuf)
+        *outbuf = gst_buffer_ref (buffer);
+
       return TRUE;
     }
 
@@ -271,6 +277,10 @@ gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer,
     for (i = 0; i < GST_VIDEO_INFO_N_PLANES (&upload->in_info); i++) {
       upload->in_tex[i] = NULL;
     }
+
+    if (ret && outbuf != NULL)
+      *outbuf = gst_buffer_ref (upload->priv->outbuf);
+
     return ret;
   }
 #if GST_GL_HAVE_PLATFORM_EGL
@@ -292,7 +302,7 @@ gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer,
     texture_ids[0] = upload->priv->tex_id;
 
     if (!gst_gl_upload_perform_with_gl_texture_upload_meta (upload,
-            gl_tex_upload_meta, texture_ids)) {
+            gl_tex_upload_meta, texture_ids, outbuf)) {
       GST_DEBUG_OBJECT (upload, "Upload with GstVideoGLTextureUploadMeta "
           "failed");
     } else {
@@ -315,7 +325,7 @@ gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer,
   gst_gl_upload_set_format (upload, &upload->priv->frame.info);
 
   if (!gst_gl_upload_perform_with_data (upload, tex_id,
-          upload->priv->frame.data)) {
+          upload->priv->frame.data, outbuf)) {
     return FALSE;
   }
 
@@ -364,6 +374,7 @@ error:
  * @upload: a #GstGLUpload
  * @meta: a #GstVideoGLTextureUploadMeta
  * @texture_id: resulting GL textures to place the data into.
+ * @outbuf: (allow-none): resulting buffer
  *
  * Uploads @meta into @texture_id.
  *
@@ -371,7 +382,8 @@ error:
  */
 gboolean
 gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload * upload,
-    GstVideoGLTextureUploadMeta * meta, guint texture_id[4])
+    GstVideoGLTextureUploadMeta * meta, guint texture_id[4],
+    GstBuffer ** outbuf)
 {
   gboolean ret;
 
@@ -403,6 +415,9 @@ gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload * upload,
 
   ret = upload->priv->result;
 
+  if (ret && outbuf != NULL)
+    *outbuf = gst_buffer_ref (upload->priv->outbuf);
+
   GST_OBJECT_UNLOCK (upload);
 
   return ret;
@@ -413,6 +428,7 @@ gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload * upload,
  * @upload: a #GstGLUpload
  * @texture_id: (out): the texture id to upload into
  * @data: where the downloaded data should go
+ * @outbuf: (allow-none): resulting buffer
  *
  * Uploads @data into @texture_id. data size and format is specified by
  * the #GstVideoInfo<!--  -->s passed to gst_gl_upload_set_format() 
@@ -421,14 +437,16 @@ gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload * upload,
  */
 gboolean
 gst_gl_upload_perform_with_data (GstGLUpload * upload, GLuint * texture_id,
-    gpointer data[GST_VIDEO_MAX_PLANES])
+    gpointer data[GST_VIDEO_MAX_PLANES], GstBuffer ** outbuf)
 {
   gboolean ret;
 
   g_return_val_if_fail (upload != NULL, FALSE);
 
   GST_OBJECT_LOCK (upload);
-  ret = _gst_gl_upload_perform_with_data_unlocked (upload, texture_id, data);
+  ret =
+      _gst_gl_upload_perform_with_data_unlocked (upload, texture_id, data,
+      outbuf);
   GST_OBJECT_UNLOCK (upload);
 
   return ret;
@@ -436,7 +454,8 @@ gst_gl_upload_perform_with_data (GstGLUpload * upload, GLuint * texture_id,
 
 static gboolean
 _gst_gl_upload_perform_with_data_unlocked (GstGLUpload * upload,
-    GLuint * texture_id, gpointer data[GST_VIDEO_MAX_PLANES])
+    GLuint * texture_id, gpointer data[GST_VIDEO_MAX_PLANES],
+    GstBuffer ** outbuf)
 {
   gboolean ret;
   guint i;
@@ -459,6 +478,9 @@ _gst_gl_upload_perform_with_data_unlocked (GstGLUpload * upload,
   ret = _upload_memory (upload);
   *texture_id = upload->out_tex->tex_id;
 
+  if (ret && outbuf != NULL)
+    *outbuf = gst_buffer_ref (upload->priv->outbuf);
+
   return ret;
 }
 

gst-libs/gst/gl/gstglupload.h

@@ -78,12 +78,12 @@ GstGLUpload * gst_gl_upload_new            (GstGLContext * context);
 void           gst_gl_upload_set_format    (GstGLUpload * upload, GstVideoInfo * in_info);
 GstVideoInfo * gst_gl_upload_get_format    (GstGLUpload * upload);
 
-gboolean gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer, guint * tex_id);
+gboolean gst_gl_upload_perform_with_buffer (GstGLUpload * upload, GstBuffer * buffer, guint * tex_id, GstBuffer ** outbuf);
 void gst_gl_upload_release_buffer (GstGLUpload * upload);
 gboolean gst_gl_upload_perform_with_data          (GstGLUpload * upload, GLuint * texture_id,
-                                                   gpointer data[GST_VIDEO_MAX_PLANES]);
+                                                   gpointer data[GST_VIDEO_MAX_PLANES], GstBuffer ** outbuf);
 
-gboolean gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload *upload, GstVideoGLTextureUploadMeta *meta, guint texture_id[4]);
+gboolean gst_gl_upload_perform_with_gl_texture_upload_meta (GstGLUpload *upload, GstVideoGLTextureUploadMeta *meta, guint texture_id[4], GstBuffer ** outbuf);
 
 G_END_DECLS