After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784210 - gl: [regression] GstGLUploadMeta feature doesn't show frame in GLX
gl: [regression] GstGLUploadMeta feature doesn't show frame in GLX
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.12.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-26 10:57 UTC by Víctor Manuel Jáquez Leal
Modified: 2017-06-29 09:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glmemory: reset the draw buffer to GL_BACK or GL_COLOR_ATTACHMENT0 (1.11 KB, patch)
2017-06-27 09:40 UTC, Hyunjun Ko
none Details | Review
glmemory: reset the draw buffer to GL_BACK (902 bytes, patch)
2017-06-28 03:22 UTC, Hyunjun Ko
committed Details | Review

Description Víctor Manuel Jáquez Leal 2017-06-26 10:57:12 UTC
This pipeline doesn't show any frame:

GST_GL_PLATFORM=glx GST_GL_API=opengl \
   gst-play-1.0 big_buck_bunny_1080p_h264.mov --videosink=glimagesink

When the negotatied caps with glupload are 

video/x-raw(meta:GstVideoGLTextureUploadMeta), format=(string)RGBA, width=(int)1920, height=(int)1080, framerate=(fraction)24/1, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive

I did a bisect and the culprit is commit 0730a55 from bug #782376

Commenting out this it works again

diff --git a/gst-libs/gst/gl/gstglmemory.c b/gst-libs/gst/gl/gstglmemory.c
index 02848f0b7..2212ecbfd 100644
--- a/gst-libs/gst/gl/gstglmemory.c
+++ b/gst-libs/gst/gl/gstglmemory.c
@@ -745,8 +745,8 @@ gst_gl_memory_copy_teximage (GstGLMemory * src, guint tex_id,

     gl->DeleteFramebuffers (n_fbos, &fbo[0]);

-    if (gl->DrawBuffer)
-      gl->DrawBuffer (GL_NONE);
+    /* if (gl->DrawBuffer) */
+    /*   gl->DrawBuffer (GL_NONE); */
   }
Comment 1 Matthew Waters (ystreet00) 2017-06-26 13:20:46 UTC
Then something's not setting the required GL state for it's drawing ;)
Comment 2 Víctor Manuel Jáquez Leal 2017-06-26 17:58:22 UTC
(In reply to Matthew Waters (ystreet00) from comment #1)
> Then something's not setting the required GL state for it's drawing ;)

I'll need to spend more time on it since I'm clueless :'(
Comment 3 Matthew Waters (ystreet00) 2017-06-27 04:01:20 UTC
Actually, this may be a gstgl bug.  The default for glDrawBuffer is GL_BACK or GL_COLOR_ATTACHMENT0 depending on if a framebuffer is current or not.  Does changing the enum to glDrawBuffer fix this?
Comment 4 Hyunjun Ko 2017-06-27 05:38:49 UTC
(In reply to Matthew Waters (ystreet00) from comment #3)
> Actually, this may be a gstgl bug.  The default for glDrawBuffer is GL_BACK
> or GL_COLOR_ATTACHMENT0 depending on if a framebuffer is current or not. 
> Does changing the enum to glDrawBuffer fix this?

Yes, it fixes it by chaning either GL_BACK or GL_COLOR_ATTACHMENT0 :)
Comment 5 Matthew Waters (ystreet00) 2017-06-27 07:52:26 UTC
(In reply to Hyunjun Ko from comment #4)
> (In reply to Matthew Waters (ystreet00) from comment #3)
> > Actually, this may be a gstgl bug.  The default for glDrawBuffer is GL_BACK
> > or GL_COLOR_ATTACHMENT0 depending on if a framebuffer is current or not. 
> > Does changing the enum to glDrawBuffer fix this?
> 
> Yes, it fixes it by chaning either GL_BACK or GL_COLOR_ATTACHMENT0 :)

Ok, then that is the correct approach for that. Do you want to provide a patch?

I think there are a few other cases where this happens that you may as well fix too.

The draw buffer should reset to GL_BACK if there is no framebuffer bound and GL_COLOR_ATTACHMENT0 if there is a framebuffer bound.
Comment 6 Hyunjun Ko 2017-06-27 09:40:44 UTC
Created attachment 354550 [details] [review]
glmemory: reset the draw buffer to GL_BACK or  GL_COLOR_ATTACHMENT0

The draw buffer should be reset to GL_BACK if there is no framebuffer bound
and GL_COLOR_ATTACHMENT0 if there is a framebuffer bound.
Comment 7 Matthew Waters (ystreet00) 2017-06-27 10:19:44 UTC
Review of attachment 354550 [details] [review]:

Not quite :)

::: gst-libs/gst/gl/gstglmemory.c
@@ +748,3 @@
+    if (gl->DrawBuffer) {
+      gint is_bound;
+      gl->GetIntegerv (GL_DRAW_FRAMEBUFFER_BINDING, &is_bound);

This is not necessary, you can infer this from the previous GL calls.
Comment 8 Hyunjun Ko 2017-06-28 03:22:29 UTC
Created attachment 354602 [details] [review]
glmemory: reset the draw buffer to GL_BACK

OdicThe draw buffer should be reset to GL_BACK since the framebuffer is unbound.
Comment 9 Matthew Waters (ystreet00) 2017-06-29 05:00:32 UTC
commit effda87a0b16dd55df7ce38d56b5557766643cc8
Author: Matthew Waters <matthew@centricular.com>
Date:   Wed Jun 28 14:45:18 2017 +1000

    gl: reset gl->DrawBuffer to the necessary values
    
    GL_COLOR_ATTACHMENT0 when a framebuffer is bound
    GL_BACK if no framebuffer is bound
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784210

commit 02ef39f48aa9e261c8fd9a4986f986d2ec047cf7
Author: Hyunjun Ko <zzoon@igalia.com>
Date:   Wed Jun 28 12:17:37 2017 +0900

    glmemory: reset the draw buffer to GL_BACK
    
    The draw buffer should be reset to GL_BACK since the framebuffer is already
    unbound.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784210
Comment 10 Matthew Waters (ystreet00) 2017-06-29 09:34:48 UTC
and 1.12
1922548721e5f8ff9d86c69615cf060eca7d0913
e9bcddb7c61d681ef24cc901bff25fdc2b3ac555