From 260b9791fc7ea524ad949b5cfdd81688d0a3013e Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Thu, 23 Aug 2018 22:34:47 +1000 Subject: [PATCH] gstsegment: Handle positions before the segment properly Fixes for gst_segment_position_from_running_time_full() when converting running_times that precede the segment start (or stop in a negative rate segment) The return value was incorrectly negated in those cases. Add some more unit test checks for those cases, and especially for segments with offsets. --- gst/gstsegment.c | 33 ++++++++++----- tests/check/gst/gstsegment.c | 96 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 11 deletions(-) diff --git a/gst/gstsegment.c b/gst/gstsegment.c index e09d75e..6aa1ce2 100644 --- a/gst/gstsegment.c +++ b/gst/gstsegment.c @@ -992,8 +992,9 @@ gst_segment_position_from_running_time (const GstSegment * segment, * When 1 is returned, @running_time resulted in a positive position returned * in @position. * - * When this function returns -1, the returned @position should be negated - * to get the real negative segment position. + * When this function returns -1, the returned @position was < 0, and the value + * in the position variable should be negated to get the real negative segment + * position. * * Returns: a 1 or -1 on success, 0 on failure. * @@ -1036,12 +1037,15 @@ gst_segment_position_from_running_time_full (const GstSegment * segment, *position = base - running_time; if (G_UNLIKELY (abs_rate != 1.0)) *position = ceil (*position * abs_rate); - if (start + segment->offset > *position) { - *position -= start + segment->offset; - res = -1; - } else { + if (start + segment->offset >= *position) { + /* The TS is before the segment, but the result is >= 0 */ *position = start + segment->offset - *position; res = 1; + } else { + /* The TS is before the segment, and the result is < 0 + * so negate the return result */ + *position = *position - (start + segment->offset); + res = -1; } } } else { @@ -1057,15 +1061,24 @@ gst_segment_position_from_running_time_full (const GstSegment * segment, res = 1; } } else { + /* This case is tricky. Requested running time precedes the + * segment base, so in a reversed segment where rate < 0, that + * means it's before the alignment point of (stop - offset). + * Before = always bigger than (stop-offset), which is usually +ve, + * but could be -ve is offset is big enough. -ve position implies + * that the offset has clipped away the entire segment anyway */ *position = base - running_time; if (G_UNLIKELY (abs_rate != 1.0)) *position = ceil (*position * abs_rate); - if (G_UNLIKELY (stop < segment->offset - *position)) { - *position -= segment->offset - stop; - res = -1; - } else { + + if (G_LIKELY (stop + *position >= segment->offset)) { *position = stop + *position - segment->offset; res = 1; + } else { + /* Requested position is still negative because offset is big, + * so negate the result */ + *position = segment->offset - *position - stop; + res = -1; } } } diff --git a/tests/check/gst/gstsegment.c b/tests/check/gst/gstsegment.c index bba31ce..f160462 100644 --- a/tests/check/gst/gstsegment.c +++ b/tests/check/gst/gstsegment.c @@ -869,11 +869,105 @@ GST_START_TEST (segment_full) segment.offset = 0; gst_segment_set_running_time (&segment, GST_FORMAT_TIME, 100); fail_unless_equals_int (segment.base, 100); + fail_unless (gst_segment_position_from_running_time_full (&segment, - GST_FORMAT_TIME, 70, &pos) == -1); + GST_FORMAT_TIME, 70, &pos) == 1); + fail_unless_equals_int (pos, 120); + fail_unless (gst_segment_position_from_running_time_full (&segment, GST_FORMAT_TIME, 140, &pos) == 1); fail_unless_equals_int (pos, 190); + + /* Test a non-1.0 rate that lands right before the segment, but still +ve */ + segment.rate = 1.1; + segment.start = 100; + segment.offset = 0; + segment.stop = 500; + segment.position = 40; + segment.base = 150; + segment.time = 10000; + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 140, &pos) == 1); + fail_unless (pos == 89); + /* And now one that should give a position < 0 */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 0, &pos) == -1); + fail_unless (pos == 65); + + /* Test a non-1.0 negative rate that lands right after the (reversed) segment, but still +ve position */ + segment.rate = -2.0; + segment.start = 100; + segment.offset = 0; + segment.stop = 500; + segment.position = 150; + segment.base = 133; + segment.time = 10000; + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 200 + 133 + 20, &pos) == 1); + fail_unless (pos == 60); + /* And now one that should give a position < 0, reported as a negated value */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 200 + 133 + 70, &pos) == -1); + fail_unless (pos == 40); + + /* Test gst_segment_position_from_running_time_full() with offsets */ + segment.rate = 2.0; + segment.start = 100; + segment.offset = 100; + segment.stop = 500; + segment.position = 150; + segment.base = 175; + segment.time = 10000; + /* Position before the segment but >= 0 */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 75, &pos) == 1); + fail_unless (pos == 0); + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 75) == -1); + + /* Position before the segment and < 0 */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 65, &pos) == -1); + fail_unless (pos == 20); /* Actually -20 */ + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 65) == -1); + + /* After the segment */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 175 + 150 + 10, &pos) == 1); + fail_unless (pos == 520); + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 175 + 150 + 10) == -1); + + /* And with negative rate, so the segment is reversed */ + segment.rate = -2.0; + + /* Before the segment */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 75, &pos) == 1); + fail_unless (pos == 600); + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 75) == -1); + + /* Position after the segment and < 0 */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 400, &pos) == -1); + fail_unless (pos == 50); /* Actually -50 */ + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 400) == -1); + + /* Position after the segment and >= 0 */ + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 325 + 10, &pos) == 1); + fail_unless (pos == 80); + fail_unless (gst_segment_position_from_running_time (&segment, + GST_FORMAT_TIME, 325 + 10) == -1); + + /* Big offset can clip away an entire reversed segment and produce a negative position anyway */ + segment.offset = 1000; + fail_unless (gst_segment_position_from_running_time_full (&segment, + GST_FORMAT_TIME, 75, &pos) == -1); + fail_unless (pos == 300); /* Actually -300 */ } GST_END_TEST; -- 2.7.4