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 729542 - glimagesink: pool may never be activated, which leads to crash
glimagesink: pool may never be activated, which leads to crash
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 1.3.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-05-04 22:46 UTC by Nicolas Dufresne (ndufresne)
Modified: 2014-05-24 20:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] gl: Ensure buffer activatio (846 bytes, patch)
2014-05-04 22:50 UTC, Nicolas Dufresne (ndufresne)
none Details | Review

Description Nicolas Dufresne (ndufresne) 2014-05-04 22:46:37 UTC
since "gl: use the bufferpool's upload when available", it is likely that the pool upload object get used without the pool being activated. If there is no video meta, this leads to an upload->info.finfo being NULL, and mapping that frame lead to crash.

I'm not sure what approach we want, we could force pool activation, or find an in between to that change. The comment says "when available", but in fact it's always using it, an error ?
Comment 1 Nicolas Dufresne (ndufresne) 2014-05-04 22:50:46 UTC
Created attachment 275852 [details] [review]
[PATCH] gl: Ensure buffer activatio

 ext/gl/gstglimagesink.c | 3 +++
 1 file changed, 3 insertions(+)
Comment 2 Nicolas Dufresne (ndufresne) 2014-05-04 22:52:17 UTC
(In reply to comment #1)
> Created an attachment (id=275852) [details] [review]
> [PATCH] gl: Ensure buffer activatio

This is in case we want to enforce buffer pool activation, note this will take a lock each time it's called (if that matters ?)
Comment 3 Nicolas Dufresne (ndufresne) 2014-05-06 19:41:07 UTC
btw, to reproduce the crash:

gst-launch-1.0 v4l2src ! glimagesink
Comment 4 Matthew Waters (ystreet00) 2014-05-06 23:13:19 UTC
commit 6f4fd7086745720c39cc1d6bfd7a1c4c845caf99
Author: Matthew Waters <ystreet00@gmail.com>
Date:   Wed May 7 09:11:25 2014 +1000

    gl/sink: make sure we always initialize the upload object
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729542
Comment 5 Julien Isorce 2014-05-07 07:57:22 UTC
1- There is no pool of textures in gst-launch-1.0 v4l2src ! glimagesink  ?

2- I guess we also need to apply this change in gstglfilter ? (gst-launch-1.0 v4l2src ! gleffects ! glimagesink) 

3- Would it be better to start the gl buffer pool if not already active ?

if (G_UNLIKELY (!gst_buffer_pool_is_active (pool)) {
  ret = gst_buffer_pool_set_active (pool);
  if (!ret)
    goto error;
}
Comment 6 Matthew Waters (ystreet00) 2014-05-07 08:11:54 UTC
(In reply to comment #5)
> 1- There is no pool of textures in gst-launch-1.0 v4l2src ! glimagesink  ?

No

> 2- I guess we also need to apply this change in gstglfilter ? (gst-launch-1.0
> v4l2src ! gleffects ! glimagesink) 

No, the upload is created regardless of whether it is activated.  http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst-libs/gst/gl/gstglfilter.c#n1116

> 3- Would it be better to start the gl buffer pool if not already active ?
> 
> if (G_UNLIKELY (!gst_buffer_pool_is_active (pool)) {
>   ret = gst_buffer_pool_set_active (pool);
>   if (!ret)
>     goto error;
> }

The extra textures are created/destroyed by GstGLUpload.
Comment 7 Nicolas Dufresne (ndufresne) 2014-05-07 14:26:11 UTC
Thanks works, for the reference

commit 6f4fd7086745720c39cc1d6bfd7a1c4c845caf99
Author: Matthew Waters <ystreet00@gmail.com>
Date:   Wed May 7 09:11:25 2014 +1000

    gl/sink: make sure we always initialize the upload object
    
    https://bugzilla.gnome.org/show_bug.cgi?id=729542
Comment 8 Julien Isorce 2014-05-08 08:18:22 UTC
Hey, is c2cdac278e043e091f21a6a796231dd26ec75ddd for #6  -2 ? :)
Comment 9 Nicolas Dufresne (ndufresne) 2014-05-24 20:41:26 UTC
(In reply to comment #8)
> Hey, is c2cdac278e043e091f21a6a796231dd26ec75ddd for #6  -2 ? :)

That one fixes that same issue but in gleffects I think.