GNOME Bugzilla – Bug 735800
textoverlay: Two textoverlay in sequence fail to negotiate (regression)
Last modified: 2014-09-01 18:41:30 UTC
Created attachment 284990 [details] GST_DEBUG=LOG With gst-plugins-base 1.4.1, the following pipeline now deadlocks: gst-launch-1.0 videotestsrc ! textoverlay ! textoverlay ! fakesink It tries to recursively lock the class' pango_lock. The (sparse) stack for this is: g_mutex_lock gst_base_text_overlay_setcaps gst_base_text_overlay_video_event (second textoverlay) gst_pad_push_event gst_pad_set_caps gst_base_text_overlay_setcaps gst_base_text_overlay_video_event (first textoverlay) With e783a36 applied (which fixes the deadlock), the pipeline fails to negotiate: ERROR: from element /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: Internal data flow error. Additional debug info: gstbasesrc.c(2933): gst_base_src_loop (): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0: streaming task paused, reason not-negotiated (-4) ERROR: pipeline doesn't want to preroll. Log attached.
Another thing that doesn't work any more, but worked in 1.2: gst-launch-1.0 videotestsrc num-buffers=1 ! textoverlay ! textoverlay ! fakesink (works with a real video sink, but not fakesink).
This is related to bug #733916 btw
Created attachment 285028 [details] [review] basetextoverlay: properly fallback to non-overlay caps When downstream claims to accept the overlay meta but fails to provide it in the allocation query, properly fallback to setting a new caps without the overlay meta as that is not going to be used. Only do this if the original caps doesn't have the overlay already, otherwise there isn't much that can be done.
Comment on attachment 285028 [details] [review] basetextoverlay: properly fallback to non-overlay caps This smells like a good opportunity for many unit tests ;)
There were a few unit tests with caps features scenarios, added one more with the missing case that fakesink causes. master: commit 2157497405b5151d8fb7ee59fb10c809df6f0007 Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Mon Sep 1 15:23:27 2014 -0300 tests: textoverlay: add test to reproduce fakesink scenario Adds a new test to textoverlay to make sure it can properly handle elements that have ANY caps but fail to add the overlay meta in the allocation query. This test verifies that textoverlay won't use the caps features even knowing that the overlay meta is accepted when querying the downstream caps because it also needs downstream to confirm by putting the meta in the allocation query. https://bugzilla.gnome.org/show_bug.cgi?id=735800 commit a65b3073497fe92140d045561cc6808b789657d7 Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Mon Sep 1 12:38:02 2014 -0300 basetextoverlay: properly fallback to non-overlay caps When downstream claims to accept the overlay meta but fails to provide it in the allocation query, properly fallback to setting a new caps without the overlay meta as that is not going to be used. Only do this if the original caps doesn't have the overlay already, otherwise there isn't much that can be done. https://bugzilla.gnome.org/show_bug.cgi?id=735800 commit e783a366cb514561cc6b929b93d75ea834132f75 Author: Sebastian Dröge <sebastian@centricular.com> Date: Mon Sep 1 12:28:24 2014 +0300 textoverlay: Don't hold any mutexes while calling negotiate It's not done in any other code calling negotiate and will cause deadlocks as it is sending events and queries in the pipeline. Specifically this pipeline was deadlocking: gst-launch-1.0 videotestsrc ! textoverlay ! textoverlay ! fakesink 1.4: commit 18f01194fd67c9dbfa676f3885a15fbe20537a50 commit 0b8125326ba3f3648085f68ae670dd61659e371e commit 382f4b7eaa401c5ac8d6be1ea2a33d41f4e0089b