GNOME Bugzilla – Bug 494245
[basesink] doesn't sync correctly if segment is in BYTES and a stop position is set
Last modified: 2007-11-09 11:56:04 UTC
Consider this pipeline: gst-launch-0.10 filesrc location=/some/file blocksize=100 ! identity datarate=100 ! fakesink sync=true -v Despite timestamps and durations being set correctly, fakesink will not sync against the clock correctly from the second buffer onwards. This, however, will work just fine: gst-launch-0.10 fakesrc sizemax=100 sizetype=fixed ! identity datarate=100 ! fakesink sync=true -v The reason for this is that filesrc sends a newsegment event in BYTES format with a stop position that equals the filesize. Now, gst_base_sink_get_sync_times() in do_times calls gst_segment_to_stream_time() and gst_segment_to_running_time() with the timestamps and GST_FORMAT_BYTES as format. gst_segment_to_*_time(), however, will not pay attention to the fact that segment->format != format and goes on to return -1 here: if (segment->rate > 0.0) { /* outside of the segment boundary stop */ if (G_UNLIKELY (segment->stop != -1 && position > segment->stop)) return -1; because the timestamp integer value is > the file size in bytes, so basesink gets unsyncable positions returned and skips the sync. (This can all be made to work by using single-segment=true on identity, but that's not the point).
Some pointers: - when identity knows the datarate, it can convert a bytes segment to time. - the GstSegment code should not try to compare values of different formats.
Created attachment 98767 [details] [review] proposed patch Remove some return_if_fail when the formats are different. Instead, don't try to compare values in different formats but assume the default values.
Is this a regression?
This is also addressed by the patch in #488201
* gst/gstsegment.c: (gst_segment_set_newsegment_full), (gst_segment_to_stream_time), (gst_segment_to_running_time): Also accumulate time correctly when doing reverse playback. Fixes #488201, When converting to running and stream time, use default values for start/stop/time/accum when comparing different formats. Fixes #494245. * libs/gst/base/gstbasesink.c: (gst_base_sink_get_sync_times): Do running/stream time in TIME format. * tests/check/gst/gstsegment.c: (GST_START_TEST), (gst_segment_suite): 2 new unit tests for segment accumulation.