After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 494245 - [basesink] doesn't sync correctly if segment is in BYTES and a stop position is set
[basesink] doesn't sync correctly if segment is in BYTES and a stop position ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.15
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on: 488201
Blocks:
 
 
Reported: 2007-11-06 17:04 UTC by Tim-Philipp Müller
Modified: 2007-11-09 11:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (4.02 KB, patch)
2007-11-08 13:11 UTC, Wim Taymans
committed Details | Review

Description Tim-Philipp Müller 2007-11-06 17:04:16 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).
Comment 1 Wim Taymans 2007-11-08 12:29:21 UTC
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.
Comment 2 Wim Taymans 2007-11-08 13:11:56 UTC
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.
Comment 3 Jan Schmidt 2007-11-09 11:11:24 UTC
Is this a regression? 
Comment 4 Jan Schmidt 2007-11-09 11:26:58 UTC
This is also addressed by the patch in #488201
Comment 5 Wim Taymans 2007-11-09 11:56:04 UTC
        * 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.