GNOME Bugzilla – Bug 757610
compositor: negotation failure when mixing alpha and non alpha branches
Last modified: 2016-02-25 15:25:58 UTC
The fix for https://bugzilla.gnome.org/show_bug.cgi?id=754465 has introduced what I believe is a regression. The alpha logic change that was committed breaks the following pipeline: compositor name=mixer ! videoconvert ! autovideosink \ ksvideosrc ! queue ! mixer. \ udpsrc port=9000 ! textrender ! videorate drop-only=true ! video/x-raw, framerate=10/1 ! queue ! mixer. To be correct, the pipeline breaks if the ksvideosrc is the first to push a frame to the compositor. In that case, the textrender fails to negotiate because all alpha format are filtered out. If the textrender is the first to push a frame, then the pipeline works. If I revert the fix then the pipeline always works no matter which branch first reaches the compositor. Note that if I remove the downstream videoconvert, then the pipeline fails again but when the textrender is first… But this is expected and documented : ("At least one of the input pads contains alpha, but downstream can't support alpha."), ("Either convert your inputs to not contain alpha or add a videoconvert after the aggregator"));
Minor correction : when removing the downstream videoconvert, the pipeline always fails whichever branch is first :)
In https://bugzilla.gnome.org/show_bug.cgi?id=754465 there it is mentioned that the fix might be hiding another bug elsewhere. My hunch is that it was true and the "other bug" has been solved since.
Can you share the exact formats that those elements are feeding the compositor? I'm writing some unit tests and so far it has been working for alpha and non-alpha formats in both orders. A GST_DEBUG=6 log might also help.
I have just reproduced the issue with gstreamer 1.6.3 on windows. To reproduce the issue: 1/ Run this pipeline: compositor name=mixer ! videoconvert ! autovideosink \ ksvideosrc ! queue ! mixer. \ udpsrc port=9000 ! textrender ! videorate drop-only=true ! video/x-raw, framerate=10/1 ! queue ! mixer. It will start fine and show the ksvideosrc output. 2/ Send any udp message to localhost:9000. You can use "Packet Sender" on windows or do the following on linux: echo -n '<span foreground="red">Hello</span>' | nc -4u -w1 127.0.0.1 9000 The pipeline will then fail with this error: ERROR: from element /GstPipeline:pipeline0/GstTextRender:textrender0: GStreamer error: negotiation problem. Additional debug info: ../../../gst-plugins-base-1.6.3/ext/pango/gsttextrender.c(511): gst_text_render_chain (): /GstPipeline:pipeline0/GstTextRender:textrender0 I attached a log (with GST_DEBUG=6) that shows the issue.
I don't see the debug log attached. Unfortunately I don't have a windows machine setup with GStreamer to reproduce this but the log should be enough to understand what is happening.
Created attachment 321698 [details] log of test case
The issue should be reproducible on linux too. Just change ksvideosrc with v4l2src.
As to what is happening, my understanding: - the fix for https://bugzilla.gnome.org/show_bug.cgi?id=754465 was an attempt at fixing a issue downstream of compositor when the compositor is compositing both alpha and non alpha sources. If the first source to get through is has no alpha then the compositor will reject any further source if it has alpha. - the bug fix in question was debated and finally accepted although it was probably due to a bug "elsewhere". - reverting the fix allows the compositor to properly composite a mix of alpha and non alpha sources.
Created attachment 321967 [details] [review] videoaggregator: fix caps queries to allow proper renegotiation Please check if this patch solves your issue
Review of attachment 321967 [details] [review]: ::: gst-libs/gst/video/gstvideoaggregator.c @@ +893,3 @@ GST_DEBUG_OBJECT (pad, "Get caps with filter: %" GST_PTR_FORMAT, filter); + srccaps = gst_pad_get_allowed_caps (srcpad); get_allowed_caps() always returns NULL (instead of the template caps) if there is no peer. Is that a problem here?
Created attachment 322000 [details] [review] videoaggregator: fix caps queries to allow proper renegotiation It shouldn't cause any issues but it made me realize the code could be simplified a bit as the get_allowed_caps() would be doing the same as just a query with the template as filter. So just do it directly.
I'll test the change asap. Please note that with the proposed change, the code introduced to fix https://bugzilla.gnome.org/show_bug.cgi?id=754465 is probably obsolete (i.e. gst_videoaggregator_caps_has_alpha (GstCaps * caps), _get_non_alpha_caps_from_template(), etc...)
Philippe, any news here? Thiago, do you think this is safe to go?
I built just gstreamer 1.6.3 (was a bit away from that project). Will now test the proposed fix.
I built 1.6.3 and was able to reproduce the issue with this pipeline : compositor name=mixer ! videoconvert ! autovideosink videotestsrc ! queue ! mixer. udpsrc port=9000 ! textrender ! videorate drop-only=true ! video/x-raw, framerate=10/1 ! queue ! mixer. Then rebuilt with the proposed fix and was not able to reproduce it anymore :). Please note that I feel that the whole has_alpha stuff is not needed ;)
(In reply to Philippe Renon from comment #15) > I built 1.6.3 and was able to reproduce the issue with this pipeline : > > compositor name=mixer ! videoconvert ! autovideosink videotestsrc ! queue ! > mixer. udpsrc port=9000 ! textrender ! videorate drop-only=true ! > video/x-raw, framerate=10/1 ! queue ! mixer. > > Then rebuilt with the proposed fix and was not able to reproduce it anymore > :). Thanks for testing. > > Please note that I feel that the whole has_alpha stuff is not needed ;) It is needed because videoaggregator needs to identify if downstream supports alpha formats or not as videoaggregator can't convert those. Otherwise it might accept alpha formats and then fail as downstream won't support those.
commit f231598370ebc4364d16eab1c7e3dbdd7001668a Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Thu Feb 18 10:57:51 2016 -0300 videoaggregator: fix caps queries to allow proper renegotiation When caps are already negotiated it should be possible to select formats other than the one that was negotiated. If downstream allows alpha video caps and it has already negotiated to a non-alpha format, caps queries should still return the alpha caps as a possible format as caps renegotiation can happen. Includes tests (for compositor) to check that caps queries done after a caps has been negotiated returns complete results https://bugzilla.gnome.org/show_bug.cgi?id=757610
Comment on attachment 322000 [details] [review] videoaggregator: fix caps queries to allow proper renegotiation Committed with some minor modifications on the tests to make valgrind happy.