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.
This commit is contained in:
Jan Schmidt 2018-08-23 22:34:47 +10:00
parent f87d38c186
commit 260b9791fc
2 changed files with 118 additions and 11 deletions

View file

@ -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 * When 1 is returned, @running_time resulted in a positive position returned
* in @position. * in @position.
* *
* When this function returns -1, the returned @position should be negated * When this function returns -1, the returned @position was < 0, and the value
* to get the real negative segment position. * in the position variable should be negated to get the real negative segment
* position.
* *
* Returns: a 1 or -1 on success, 0 on failure. * 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; *position = base - running_time;
if (G_UNLIKELY (abs_rate != 1.0)) if (G_UNLIKELY (abs_rate != 1.0))
*position = ceil (*position * abs_rate); *position = ceil (*position * abs_rate);
if (start + segment->offset > *position) { if (start + segment->offset >= *position) {
*position -= start + segment->offset; /* The TS is before the segment, but the result is >= 0 */
res = -1;
} else {
*position = start + segment->offset - *position; *position = start + segment->offset - *position;
res = 1; 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 { } else {
@ -1057,15 +1061,24 @@ gst_segment_position_from_running_time_full (const GstSegment * segment,
res = 1; res = 1;
} }
} else { } 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; *position = base - running_time;
if (G_UNLIKELY (abs_rate != 1.0)) if (G_UNLIKELY (abs_rate != 1.0))
*position = ceil (*position * abs_rate); *position = ceil (*position * abs_rate);
if (G_UNLIKELY (stop < segment->offset - *position)) {
*position -= segment->offset - stop; if (G_LIKELY (stop + *position >= segment->offset)) {
res = -1;
} else {
*position = stop + *position - segment->offset; *position = stop + *position - segment->offset;
res = 1; res = 1;
} else {
/* Requested position is still negative because offset is big,
* so negate the result */
*position = segment->offset - *position - stop;
res = -1;
} }
} }
} }

View file

@ -869,11 +869,105 @@ GST_START_TEST (segment_full)
segment.offset = 0; segment.offset = 0;
gst_segment_set_running_time (&segment, GST_FORMAT_TIME, 100); gst_segment_set_running_time (&segment, GST_FORMAT_TIME, 100);
fail_unless_equals_int (segment.base, 100); fail_unless_equals_int (segment.base, 100);
fail_unless (gst_segment_position_from_running_time_full (&segment, 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, fail_unless (gst_segment_position_from_running_time_full (&segment,
GST_FORMAT_TIME, 140, &pos) == 1); GST_FORMAT_TIME, 140, &pos) == 1);
fail_unless_equals_int (pos, 190); 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; GST_END_TEST;