GNOME Bugzilla – Bug 754291
videoaggregator: Compositor fails with "reason not-negotiated" when changing the pixel-aspect-ratio during runtime
Last modified: 2015-09-04 08:31:46 UTC
Created attachment 310267 [details] example program The test-program attached to this bug contains a pipeline like this: ` videotestsrc pattern=red ! video/x-raw,width=1920,height=1080,format=I420,framerate=25/1,pixel-aspect-ratio=1/1 ! videoscale ! capsfilter name=caps0 caps=video/x-raw ! mix. compositor name=mix ! video/x-raw,width=1920,height=1080,format=I420,framerate=25/1,pixel-aspect-ratio=1/1 ! xvimagesink ` The caps property of the capsfilter caps0 is, 1 second after the pipeline starts, set to `video/x-raw,width=950,height=534` Because the requested resolution 950:534 is not exactly 16:9 as the input, the videoscaler will produce videoframes with a PAR of 1424/1425 ` video/x-raw, format=(string)I420, width=(int)950, height=(int)534, framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1424/1425, interlace-mode=(string)progressive ` The compositor doesn't allow this change and terminates the pipeline. Using videomixer instead of compositor does not show this behavior and forcing a pixel-aspect-ratio of 1/1 between the videoscaler and the compositor also fixes the issue. What worries me is that this behavior was not visible in 1.5.1
Created attachment 310334 [details] logfile of the compositor failing (level 4)
Does compositor support renegotiation? It seems since 1.5 that videoaggregator (the base class) denies negotiation if the format of pixel-aspect-ratio are different.
It should still allow different format and par. compositor and glvideomixer both can convert between all possible raw formats.
(In reply to Sebastian Dröge (slomo) from comment #3) > It should still allow different format and par. compositor and glvideomixer > both can convert between all possible raw formats. Yes, but is this a blocker? @Peter: Does it work just by reverting gst-plugins-bad to 1.5.1? Or do you need to revert all modules?
(In reply to Thiago Sousa Santos from comment #4) > (In reply to Sebastian Dröge (slomo) from comment #3) > > It should still allow different format and par. compositor and glvideomixer > > both can convert between all possible raw formats. > > Yes, but is this a blocker? The bug report says that it worked in 1.5.1, so I would consider it a blocker :)
Reverting -bad to 1.5.1 works, will try git bisect.
commit c28d90de68cd07a54c9bc21a5f1fde619dc81c55 Author: Thibault Saunier <tsaunier@gnome.org> Date: Mon Jul 6 18:51:07 2015 +0200 videoaggregator: Remove pixel-aspect-ratio field from the caps returned by getcaps Avoiding not negotiated error while negotiating as we anyway force 1/1 as output Bisected to this one, the difference from 1.5.1 to master is that the negotiation in 1.5.1 would be to p-a-r=1/1 and in master is to 1424/1425, which is rejected* * but actually tries to go and asserts ever since commit 6efc106a67ce6ba2a8313150ef919c0be9234da6 Author: Olivier Crête <olivier.crete@collabora.com> Date: Fri Mar 6 19:50:08 2015 -0500 aggregator: Queue "latency" buffers at each sink pad.
Created attachment 310515 [details] [review] videoaggregator: removing restriction on caps format changes videoaggregator was not allowing format, PAR or interlace mode changes in caps, this patch lifts this restriction
Comment on attachment 310515 [details] [review] videoaggregator: removing restriction on caps format changes interlace-mode changes are still not allowed. It can't interlace or deinterlace videos, so would composite interlaced and progressive videos into the same output. Otherwise this makes sense
It seems to be lacking some proper restriction on that. The getcaps seems to report correctly the current interlace-mode but you could have 2 sink pads having its inputs being set to interlaced and non-interlaced before the source pad gets configured, leading to some inconsistency there. Do we want to fix that here or let's postpone that to post-1.6 and just do the simple fix with a variation of the proposed patch?
Is the interlacing part easy/non-intrusive to fix? If not, keep it as is and just fix the PAR problem :)
Created attachment 310555 [details] [review] videoaggregator: lift restriction of changing format and pixel-aspect-ratio The videoaggregator can convert format and PAR, there is no apparent reason for restricting it.
Seems to require some locking and no one complained, I'd defer it to after 1.6 unless someone else wants to fix it now.
commit 8c5b2da124ebe5cf68eaca758dd40c92ab6228b7 Author: Thiago Santos <thiagoss@osg.samsung.com> Date: Wed Sep 2 19:16:56 2015 -0300 videoaggregator: lift restriction of changing pixel-aspect-ratio The videoaggregator can convert PAR, there is no reason for restricting it. https://bugzilla.gnome.org/show_bug.cgi?id=754291