GNOME Bugzilla – Bug 635001
basesink: fix position reporting in PAUSED
Last modified: 2010-12-15 17:50:07 UTC
Currently, if you play a stream at e.g. 3x speed, query the position, pause the pipeline, then query the position again, the second position is earlier than the first. This is pretty bogus; in my app that results in time seemingly going backwards a bit when playback resumes after a flushing seek to adjust rate back to 1.0, but to the same timestamp. The solution is to unify the logic for basesink position queries in PAUSED and PLAYING. In PAUSED instead of using the base time and the clock, given that we have neither, we use the start_time instead, in combination with the current segment. The two attached patches implement this refactoring.
Created attachment 174618 [details] [review] gst_base_sink_get_position refactor
Created attachment 174619 [details] [review] fix position reporting in PAUSED
+ if (oformat == tformat) + running_time = GST_ELEMENT_START_TIME (basesink); + else { + gint64 rt; + + rt = gst_segment_to_running_time (segment, oformat, segment->last_stop); + + /* convert values from segment to time */ + if (!gst_pad_query_convert (basesink->sinkpad, oformat, accum, &tformat, + &accum)) + goto convert_failed; + if (!gst_pad_query_convert (basesink->sinkpad, oformat, duration, &tformat, + &duration)) + goto convert_failed; + if (!gst_pad_query_convert (basesink->sinkpad, oformat, time, &tformat, + &time)) + goto convert_failed; + if (!gst_pad_query_convert (basesink->sinkpad, oformat, rt, &tformat, &rt)) + goto convert_failed; + + running_time = (rt < 0) ? 0 : rt; } Even if the segment format is not in time, we can use the START_TIME of the element. The only reason you need to use the last_stop is when you don't have a valid start_time.
What do you mean by "start_time"? segment.start? This code only gets run when we actually have a new segment, and I don't see the case where segment.start could be -1. Did you mean segment.time?
I meant GST_ELEMENT_START_TIME() in the above code it's not used when the segment format is not in GST_FORMAT_TIME.
You said "The only reason you need to use the last_stop is when you don't have a valid start_time", but the GST_ELEMENT_START_TIME() doesn't have an invalid marker; i.e. it's never -1, AFAIK.
The start_time can be -1 when distribution of base_time is disabled with gst_pipeline_set_new_stream_time() or when the application calls gst_element_set_start_time() with GST_CLOCK_TIME_NONE.
Wim says this is fixed in the recent basesink patches, between 25 Nov and 14 Dec. Closing provisionally; will re-open if issue reappears.