GNOME Bugzilla – Bug 629410
GstBaseTransform: position query refers to sink pad, not source pad
Last modified: 2010-10-29 13:45:56 UTC
Created attachment 170072 [details] [review] Proposed patch In order to answer position queries, GstBaseTransform stores the (timestamp + duration) of the last received buffer on the sink pad. For elements that have a fixed delay or history -- for instance, an FIR or IIR filter -- this is not necessarily the same as the (timestamp + duration) of the buffers that get pushed on the source pad. One consequence of this is confusing output from the progressreport element. The attached patch takes the position from the buffers that are pushed on the source pad, instead of the buffers that are received on the sink pad.
This is partly correct. For the sinkpad you should still return this value as position but for the srcpad your new value is more correct
Instead of tracking timestamps on both sides it would also be possible to always take the input timestamps and return this as the position of the sinkpad and return this minus the latency as the position of the srcpad. Do you want to provide a patch for that?
(In reply to comment #2) > Instead of tracking timestamps on both sides it would also be possible to > always take the input timestamps and return this as the position of the sinkpad > and return this minus the latency as the position of the srcpad. Do you want to > provide a patch for that? That sounds sensible to me. I'm a little bit bogged down right now; it might be up to a week until I can put forth a new patch. I don't mind if someone else does it before then.
Setting NEEDINFO, awaiting updated patch.
Too bad that we can't really get the latency from inside basetransform and instead should do the doubled timestamp tracking commit 14023fff89d073b5758ce8c46abf15951f6607f7 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Sun Oct 10 18:14:40 2010 +0200 basetransform: Report the output position on POSITION queries on the srcpad There can be a difference between input and output last_stop. Fixes bug #629410.
This breaks rhythmbox a tiny bit. When playback is paused, we get the playback position by doing a position query on a volume element which is part of a bin that (when paused) is not linked to the audio sink. Immediately after pausing, we do a flushing seek back about half a second to compensate for the fade out duration, which now results in last_stop_out being set to GST_CLOCK_TIME_NONE, so the position queries return that rather than the actual position. Using segment.last_stop when last_stop_out is GST_CLOCK_TIME_NONE makes things work again for me.
Created attachment 173397 [details] [review] use input position instead of reporting GST_CLOCK_TIME_NONE I think I misread the previous condition in three separate ways while trying to fix this, so it's entirely possible this isn't right.
Comment on attachment 173397 [details] [review] use input position instead of reporting GST_CLOCK_TIME_NONE Makes sense... let's get this into the next pre-release.
sounds like a good idea.
Comment on attachment 173397 [details] [review] use input position instead of reporting GST_CLOCK_TIME_NONE Ok, let's get this in then. It's not ideal though, basetransform should always have 1:1 input-output timestamp mapping, there shuouldn't really be any lantency (says wim)..
Well, even for something simple like audioresample you have latency... or any other FIR filter. commit 754c3038be87e182b7c71b1b402ed430a877ad58 Author: Jonathan Matthew <jonathan@d14n.org> Date: Thu Oct 28 23:28:15 2010 +1000 basetransform: use input position for queries if we have no output position