GNOME Bugzilla – Bug 754851
aggregator: Use the whole aggregator latency for the queue size, not just the latency property
Last modified: 2015-09-18 10:19:46 UTC
See commit message
Created attachment 311100 [details] [review] aggregator: Use the whole aggregator latency for the queue size, not just the latency property This includes the subclass latency, and makes pipelines like this work properly without garbled output: gst-launch-1.0 audiotestsrc is-live=true ! audiomixer name=mixer ! pulsesink audiotestsrc is-live=true ! valve drop=true ! mixer.
Created attachment 311101 [details] [review] That feels wrong, because the upstream latency is time where the upstream elements have already queued the buffer, so we're effectively adding a queue with max-size-time=upstream-latency. The attached patch basically updated the max in our response to ref I wonder if the problem is just about CPU scheduling, the "latency" from the latency property is basically how much leeway the scheduler has before the source thread has been scheduled enough to give us buffers and I think this is kind of arbitrary (it's a deadline, not something inherent to the pipeline), but I'd love to be proven wrong. But if I am right, then maybe it is independant of the output buffer size, the subclass latency or the upstream latency.
Created attachment 311589 [details] [review] aggregator: Keep at least two buffers in the queue in live mode When in live mode, the queue needs to hold the currently processed buffer and one more at least. This means that the max latency is a bit more than announced, but I think this is what we do everywhere else also. It also fixes your usecase even if the latency property is set to 0.
Just for more context. Most issues happens when the input buffer size are slightly bigger then the queue size. The aggregator will slowly consume the data in the buffer, but will leave the buffer in the queue, which causes upstream to stay blocked. When we reach the end of that buffer, we process the last bits, remove the buffer and wait for the timeout. This gives very little time for upstream to send the next buffer (which was most likely ready before we decided to unblock). I thought initially that the solution was to take the buffer out of the queue immediately, but Olivier's patch has exactly the same effect and is simpler to implement. With this patch, using only the output buffer duration as latency is now sufficient for the case your sources have no jitter. Low-latency live mixing is now possible also.
Review of attachment 311589 [details] [review]: Makes sense, let's get this in :) ::: gst-libs/gst/base/gstaggregator.c @@ +2054,3 @@ + /* We also want at least two buffers, one is being processed and one is ready + * for the next iteration when we operate in live mode. */ + if (self->priv->peer_latency_live && aggpad->priv->num_buffers < 2) Why not g_queue_get_length() ?
Comment on attachment 311589 [details] [review] aggregator: Keep at least two buffers in the queue in live mode ... because we also queue events.
commit 473145197404c02b00f659d3a303561d81ffce36 Author: Olivier Crête <olivier.crete@collabora.com> Date: Thu Sep 17 19:42:34 2015 -0400 aggregator: Keep at least two buffers in the queue in live mode When in live mode, the queue needs to hold the currently processed buffer and one more at least. https://bugzilla.gnome.org/show_bug.cgi?id=754851