GNOME Bugzilla – Bug 781928
videoaggregator: issues with live mode when the input framerates are different from the output framerate
Last modified: 2018-01-23 19:20:31 UTC
How to reproduce: Get wondershaper (available on Fedora at least). ``` sudo wondershaper wlp6s0 1000000 100 GST_DEBUG=*videoaggregator*:6 gst-launch-1.0 videotestsrc pattern=ball is-live=true ! videorate ! video/x-raw, framerate=30/1 ! identity ! compositor name=comp sink_0::alpha=0.5 sink_1::alpha=0.5 sink_2::alpha=0.5 ! video/x-raw, framerate=5/1 ! queue ! xvimagesink uridecodebin uri=https://media.w3.org/2010/05/sintel/trailer.webm ! videorate ! comp. 2>&1 | grep "Producing buffer\|replacing old buffer\|Taking new buffer" ``` Observed behaviour: The ball moves at 5fps as expected, but its movement is 6 times slower than it would otherwise be. The timestamps that are mixed progressively drift from the output timestamps. Expected behaviour: The ball should move at 5fps, but 5 out of 6 frames should be skipped, thus maintaining the speed of its movement. Of course this is a simple example to highlight the problem, with actual sources the decoder very quickly ends up dropping frames because of QOS.
Created attachment 361662 [details] [review] API: GstAggregatorPad.skip_buffer virtual method Allows subclasses to prevent buffers from being queued. GstVideoAggregatorPad implements the virtual method and skips buffers from sources with a framerate higher than its output framerate. Fixes
Aggregator is in core now, so patch needs to be split, sorry :) I haven't really looked at the implementation, but was just wondering: this means we would basically drop the latest buffer, right? I wonder if anyone would ever want a mode where any older (already queued?) buffer should be dropped instead?
Also wonder if this could be unit-tested without too much ėffort somehow.
Created attachment 366043 [details] [review] API: GstAggregatorPad.skip_buffer virtual method Allows subclasses to prevent buffers from being queued.
Created attachment 366044 [details] [review] videoaggregatorpad: implement skip_buffer Skip buffers from sources with a framerate higher than the output framerate.
Easier way to test this: gst-launch-1.0 videotestsrc pattern=ball is-live=true ! videorate ! video/x-raw, framerate=30/1 ! identity ! compositor name=comp sink_0::alpha=0.5 sink_1::alpha=0.5 sink_2::alpha=0.5 ! video/x-raw, framerate=5/1 ! queue ! xvimagesink uridecodebin uri=https://media.w3.org/2010/05/sintel/trailer.webm ! identity sleep-time=2000000 ! videorate ! comp. Observed and expected results are the same as before.
(In reply to Tim-Philipp Müller from comment #2) > Aggregator is in core now, so patch needs to be split, sorry :) > > I haven't really looked at the implementation, but was just wondering: this > means we would basically drop the latest buffer, right? I wonder if anyone > would ever want a mode where any older (already queued?) buffer should be > dropped instead? No, we drop exactly the same buffers as would be dropped later on if the other input wasn't late, as we compute "output_start_running_time" and compare the buffer's PTS + duration
What's up with these patches? Are they good to go? Are you waiting for more review?
Yes, they are, we should definitely push them
Comment on attachment 366043 [details] [review] API: GstAggregatorPad.skip_buffer virtual method Attachment 366043 [details] pushed as 9f69034 - API: GstAggregatorPad.skip_buffer virtual method
Attachment 366044 [details] pushed as 99f646b - videoaggregatorpad: implement skip_buffer