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 745206 - glimagesink: crash when using meta:GLTextureUpload method
glimagesink: crash when using meta:GLTextureUpload method
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-02-26 12:18 UTC by Víctor Manuel Jáquez Leal
Modified: 2015-08-16 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glupload: set as NULL freed method implementation. (1.67 KB, patch)
2015-02-26 12:25 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
glupload: avoid freeing method implementation twice (1.17 KB, patch)
2015-02-26 15:31 UTC, Víctor Manuel Jáquez Leal
none Details | Review

Description Víctor Manuel Jáquez Leal 2015-02-26 12:18:06 UTC
Using gstreamer-vaapi (plus the patches in bug 744618) the negotiated caps feature is meta:GLTextureUpload the most of the time.

But that uncovered a crash: 

  • #0 __GI_raise
    at ../nptl/sysdeps/unix/sysv/linux/raise.c line 56
  • #1 __GI_abort
    at abort.c line 89
  • #2 __libc_message
    at ../sysdeps/posix/libc_fatal.c line 175
  • #3 malloc_printerr
  • #4 _int_free
    at malloc.c line 3840
  • #5 g_free
    at gmem.c line 190
  • #6 _upload_meta_upload_free
    at gstglupload.c line 584
  • #7 _upload_find_method
    at gstglupload.c line 975
  • #8 gst_gl_upload_perform_with_buffer
    at gstglupload.c line 1042
  • #9 gst_glimage_sink_prepare
    at gstglimagesink.c line 1015
  • #10 gst_base_sink_chain_unlocked
    at gstbasesink.c line 3377
  • #11 gst_base_sink_chain_main
    at gstbasesink.c line 3546

Comment 1 Víctor Manuel Jáquez Leal 2015-02-26 12:25:56 UTC
Created attachment 297967 [details] [review]
glupload: set as NULL freed method implementation.

When trying to render buffers with meta:GLTextureUpload the glimagesink crashes
with a segmentation fault.

This patch workarounds this crash setting to NULL the method implementation
after free.
Comment 2 Sebastian Dröge (slomo) 2015-02-26 12:42:10 UTC
Comment on attachment 297967 [details] [review]
glupload: set as NULL freed method implementation.

If I understand you correctly, there's still another bug elsewhere that causes things to run into this? Do you have a testcase to reproduce it?

commit 75e875b022deaf85b9d1cd46791c36e990627929
Author: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Date:   Thu Feb 26 13:20:26 2015 +0100

    glupload: Set freed method implementation to NULL
    
    When trying to render buffers with meta:GLTextureUpload the glimagesink crashes
    with a segmentation fault.
    
    This patch workarounds this crash setting to NULL the method implementation
    after free.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=745206
Comment 3 Víctor Manuel Jáquez Leal 2015-02-26 14:51:28 UTC
Thanks Sebastian.

(In reply to Sebastian Dröge (slomo) from comment #2)
> Comment on attachment 297967 [details] [review] [review]
> glupload: set as NULL freed method implementation.
> 
> If I understand you correctly, there's still another bug elsewhere that
> causes things to run into this? Do you have a testcase to reproduce it?

Agree.

Using gstreamer-vaapi master and, for example, this media[1], reverting the patch, you can run

$ gst-play-1.0 --interactive ~/300\ -\ Rise\ of\ an\ Empire\ -\ Trailer\ 2.mp4 --videosink=glimagesink --gst-debug="*gl*:5"
 

1. http://www.digital-digest.com/movies/300_Rise_of_an_Empire_1080p_Theatrical_Trailer_2.html
Comment 4 Víctor Manuel Jáquez Leal 2015-02-26 15:31:01 UTC
Created attachment 297997 [details] [review]
glupload: avoid freeing method implementation twice

When uploading a buffer, several methods are checked. If one fails, then
the next is tried.

In case of error the method implementation is freed and _upload_find_method()
is called. Nevertheless, _upload_find_method() also frees the method
implementation, leading to a double free segmentation fault.

This patch removes the free call in gst_gl_upload_perform_with_buffer() when
error, since it is done in _upload_find_method().
Comment 5 Sebastian Dröge (slomo) 2015-02-26 15:43:22 UTC
But this doesn't fix anything, right? Because _upload_find_method() already checks for NULL there.
Comment 6 Víctor Manuel Jáquez Leal 2015-02-26 16:20:31 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> But this doesn't fix anything, right? Because _upload_find_method() already
> checks for NULL there.

Right. Just it explains why the double free was happening.
Comment 7 Sebastian Dröge (slomo) 2015-03-04 09:05:32 UTC
So is there still a problem or not? :)
Comment 8 Víctor Manuel Jáquez Leal 2015-03-17 10:47:08 UTC
(In reply to Sebastian Dröge (slomo) from comment #7)
> So is there still a problem or not? :)

Ooops.. sorry, I overlooked your question.

The first patch fixed the problem. The second patch is just an improvement of the first one, but it does the same. I think we can close the bug.