GNOME Bugzilla – Bug 745206
glimagesink: crash when using meta:GLTextureUpload method
Last modified: 2015-08-16 13:37:12 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:
+ Trace 234716
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 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
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
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().
But this doesn't fix anything, right? Because _upload_find_method() already checks for NULL there.
(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.
So is there still a problem or not? :)
(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.