GNOME Bugzilla – Bug 788431
tracers: Latency tracer should not include sink synchronization pause
Last modified: 2017-10-30 19:48:55 UTC
As of today, we calculate the latency after the buffer has been pushed out of the peer of the sink sinkpad. As a side effect, if the sink is doing synchronization on the clock, we will wait for a variable about of time, time that gets included into the measure latency. This makes the result quite unreliable (specially on video). The intention with this trace is to measure the effective latency (including processing time) inside the pipeline, ignoring the sources and sink latency, which cannot be measured effectively. To illustrace better, let's take two camera sources. For both the latency have been estimated to 1 frame (let's assume 17ms). Then I build a supposedly zero-latency pipeline like this: videosource1 ! videosink videosource2 ! videosink Both sources produce proper timestamps. If videosource1 effectively had 16ms of latency, the latency tracer would 1ms latency, while if the videosource2 only take 2ms of real latency, the latency tracer report 14ms. So basically, a lower latency camera reports more latency through the tracer. This is the case for v4l2src which currently just assume 1 frame latency since the API does not explicitly give back this latency. I'm working on adding a proper latency estimation in that element, but still, would like this to not affect the value from the latency tracer as it's counter intuitive.
What's the proper way to fix this? Just switching to the "pad-push-(list)-pre" hooks on the sink's pad?
I think so, but you also need to move some stuff to the sink sinkpad peer, otherwise you'll get a race and then it's worst, the result includes the last buffer duration (was the result of my quick and dirty attempt).
Created attachment 361332 [details] [review] latency-tracer: Exclude synchronization time The goal of this tracer is to measure the processing latency between a src and a sink. In push mode, the time was read after the chain function have returned. As the amount of time we wait to get synched is reverse to the amount of latency the source introduced, the result was quite surprising. This patch moves the latency calculation in the pre-push hook. When there is no processing in a a pipeline (e.g. fakesrc ! fakesink), the latency will now be 0 as it's supposed to. For pull mode, the code was already correct. When GstBaseSink operate in pull mode, the processing time is done durring the pull, so pull-post is the right hook. The synchronization will happen after the pull has ended. Note that GstBaseSink rarely operate in pull mode.
Review of attachment 361332 [details] [review]: Typo in commit msg: "synched". Looks good. Maybe add a comment in the code about the push/pull hooks distinction as it's not that trivial?
Review of attachment 361332 [details] [review]: Sounds good to me. +1 for expanding the doc-comment to explain the reasoning about the hooks.
Attachment 361332 [details] pushed as 41e35c3 - latency-tracer: Exclude synchronization time