GNOME Bugzilla – Bug 748405
glimagesink: balance change_state bufferpool/other_context ref/unref
Last modified: 2015-04-27 08:24:50 UTC
The glbufferpool is now unreffed on PAUSED=>READY same as the GstGlContext. The other_context is now unreffed on READY=>NULL as it should (since it is allocated in NULL=>READY. It fixes an issues (having two windows) when going from PLAYING=>READY=>PLAYING with the following pipeline videotestsrc ! glimagesink.
Created attachment 302281 [details] [review] glimagesink: balance change_state bufferpool/other_context ref/unref
Note that this patch is causing a (re-)negotiation error with the following pipeline: videotestsrc ! glupload ! glcolorconvert ! gltransformation ! glimagesink when doing the following scenario: PLAYING => READY => PLAYING. I'm currently investigating the issue.
Here is the logs: http://0x5c.me/output.txt (grep for "we got return not-negotiated") After the re-negotiation is done (PLAYING=>READY=>PLAYING), the first buffer travels through the pipeline reaching the gltransformation element. At this point, gst_pad_push returns not-negotiated (looks like the buffer the element allocates does not match what has been negotiated for some reason). Any ideas ? gltransformation is not configured to perform any transformations in my pipeline.
The following patch linked by Matthew on IRC fixes the the above issue: http://hastebin.com/ehuvomoyeg.coffee There is another issue with the following pipeline: videotestsrc ! glupload ! glcolorconvert ! gltransformation ! insertbin ! glimagesink Adding an identity element to insertbin and then going to the READY state is causing the original glwindow not to be destroyed and the glcontext is not deactivated/freed. The log shows that all glbufferpools have been deactivated. Going back to PLAYING, adding a new identity element and then going back to READY causes the second glwindow to be destroyed correctly and the context is deactivated. The original glwindow is still there.
commit 1fce7dc228678c981fd2de1c6f89787149e984a4 Author: Matthew Waters <matthew@centricular.com> Date: Sun Apr 26 20:33:41 2015 +0200 glbasefilter: Unref other context in finalize, and display in READY->NULL https://bugzilla.gnome.org/show_bug.cgi?id=748405
Second half of the previous commit. commit bd940327ef9a5d7809c61c6ec138ed12fd5c5863 Author: Matthew Waters <matthew@centricular.com> Date: Mon Apr 27 15:20:56 2015 +1000 gl: unref display/other-context in the correct place Otherwise state changes from PLAYING->READY->PAUSED will cause there to to be no display configured on the element. https://bugzilla.gnome.org/show_bug.cgi?id=748405
(In reply to Matthieu Bouron from comment #4) > There is another issue with the following pipeline: > > videotestsrc ! glupload ! glcolorconvert ! gltransformation ! insertbin ! > glimagesink > > Adding an identity element to insertbin and then going to the READY state is > causing the original glwindow not to be destroyed and the glcontext is not > deactivated/freed. > The log shows that all glbufferpools have been deactivated. Going to READY from what state? >=PAUSED or NULL? Have all the pools been freed? > Going back to PLAYING, adding a new identity element and then going back to > READY causes the second glwindow to be destroyed correctly and the context > is deactivated. > > The original glwindow is still there. Does it occur without glimagesink? Does it occur without 'glupload ! glcolorconvert ! gltransformation'? By 'there' you mean on screen visible or just sticking around as a result of some ref still being held?
Created attachment 302412 [details] [review] glimagesink: unref the pool at the correct time The pool is currently unreffed in GstBaseSink::stop () which is called on READY->NULL so the context is still be alive through the pool when changing down to READY.
(In reply to Matthew Waters from comment #7) > (In reply to Matthieu Bouron from comment #4) > > There is another issue with the following pipeline: > > > > videotestsrc ! glupload ! glcolorconvert ! gltransformation ! insertbin ! > > glimagesink > > > > Adding an identity element to insertbin and then going to the READY state is > > causing the original glwindow not to be destroyed and the glcontext is not > > deactivated/freed. > > The log shows that all glbufferpools have been deactivated. > > Going to READY from what state? >=PAUSED or NULL? > Have all the pools been freed? The scenario is having the pipeline PLAYING, then adding an identity element to the insertbin. At this point everything work. then setting the pipeline to READY does not destroy the glwindow (as the gstglcontext is held). The issue is triggered by the original patch i linked to the bug. Having the pool in glimagesink freed in PAUSED=>READY + othercontext freed in PAUSE=>READY. [...] > > Does it occur without glimagesink? > Does it occur without 'glupload ! glcolorconvert ! gltransformation'? I haven't tested without glimagesink and the minimal scenario to trigger the bug is having glupload ! glcolorconvert ! gltransformation. If gltransformation is not in the pipeline the issue is not there. > By 'there' you mean on screen visible or just sticking around as a result of > some ref still being held? It's a visible window sticking around as a result of some ref still being held. The original gstglcontext is not unreffed (ie: the log does not show "activate: 0"). All the gl pools are deactivated. (In reply to Matthew Waters from comment #8) > Created attachment 302412 [details] [review] [review] > glimagesink: unref the pool at the correct time > > The pool is currently unreffed in GstBaseSink::stop () which is called on > READY->NULL so the context is still be alive through the pool when changing > down to READY. Is there a difference with the original patch I linked to the bug ?
To be a bit more clear, all the issues I've described here are the result of having the original " glimagesink: balance change_state bufferpool/other_context ref/unref" patch I linked to the bug applied.
commit 05109be4a082ed8574f34b8ed87005a8397107c6 Author: Matthew Waters <matthew@centricular.com> Date: Mon Apr 27 16:04:50 2015 +1000 glimagesink: unref the pool in the correct place Otherwise we could hold a pool to a context that is never going to be used. https://bugzilla.gnome.org/show_bug.cgi?id=748405