segment: Correct stream_time calc for negative applied rate

Updated gst_segment_position_from_stream_time and gst_segment_to_stream_time to reflect correct calculations for the case when the applied rate is negative.

Pasting from design docs:

===============================
Stream time is calculated using the buffer times and the preceding SEGMENT
event as follows:

    stream_time = (B.timestamp - S.start) * ABS (S.applied_rate) + S.time

For negative rates, B.timestamp will go backwards from S.stop to S.start,
making the stream time go backwards.
===============================

Therefore, the calculation for applied_rate < 0 should be:

    stream_time = (S.stop - B.timestamp) * ABS (S.applied_rate) + S.time

and the reverse:

    B.timestamp = S.stop - (stream_time - S.time) / ABS (S.applied_rate)

https://bugzilla.gnome.org/show_bug.cgi?id=756810

diff --git a/gst/gstsegment.c b/gst/gstsegment.c
index c8ebae8..2307ded 100644 (file)
--- a/gst/gstsegment.c
+++ b/gst/gstsegment.c
@@ -426,27 +426,29 @@ gst_segment_to_stream_time (const GstSegment * segment, GstFormat format,
   if (G_UNLIKELY (time == -1))
     return -1;
 
-  /* bring to uncorrected position in segment */
-  stream_time = position - start;
-
   abs_applied_rate = ABS (segment->applied_rate);
 
-  /* correct for applied rate if needed */
-  if (G_UNLIKELY (abs_applied_rate != 1.0))
-    stream_time *= abs_applied_rate;
-
   /* add or subtract from segment time based on applied rate */
   if (G_LIKELY (segment->applied_rate > 0.0)) {
+    if (G_UNLIKELY (position < start))
+      return -1;
+    /* bring to uncorrected position in segment */
+    stream_time = position - start;
+    /* correct for applied rate if needed */
+    if (G_UNLIKELY (abs_applied_rate != 1.0))
+      stream_time *= abs_applied_rate;
     /* correct for segment time */
     stream_time += time;
   } else {
     /* correct for segment time, clamp at 0. Streams with a negative
      * applied_rate have timestamps between start and stop, as usual, but have
      * the time member starting high and going backwards.  */
-    if (G_LIKELY (time > stream_time))
-      stream_time = time - stream_time;
-    else
-      stream_time = 0;
+    if (G_UNLIKELY (position > stop))
+      return -1;
+    stream_time = stop - position;
+    if (G_UNLIKELY (abs_applied_rate != 1.0))
+      stream_time *= abs_applied_rate;
+    stream_time += time;
   }
 
   return stream_time;
@@ -490,27 +492,34 @@ gst_segment_position_from_stream_time (const GstSegment * segment,
   if (G_UNLIKELY (time == -1))
     return -1;
 
-  if (G_LIKELY (segment->applied_rate > 0.0)) {
-    position = stream_time - time;
-  } else {
-    if (G_LIKELY (time > stream_time))
-      position = time - stream_time;
-    else
-      return -1;
-  }
+  position = stream_time - time;
 
   abs_applied_rate = ABS (segment->applied_rate);
+  stop = segment->stop;
 
   /* correct for applied rate if needed */
   if (G_UNLIKELY (abs_applied_rate != 1.0))
     position /= abs_applied_rate;
 
-  position += start;
+  if (G_LIKELY (segment->applied_rate > 0.0)) {
+    position += start;
+    /* outside of the segment boundary stop */
+    if (G_UNLIKELY (stop != -1 && position > stop))
+      return -1;
+  } else {
+    /* cannot calculate without known boundary stop */
+    if (G_UNLIKELY (stop == -1))
+      return -1;
+    /* outside segment boundary */
+    if (G_UNLIKELY (position > stop))
+      return -1;
+
+    position = stop - position;
 
-  stop = segment->stop;
-  /* outside of the segment boundary stop */
-  if (G_UNLIKELY (stop != -1 && position > stop))
-    return -1;
+    /* position before segment start */
+    if (G_UNLIKELY (position < start))
+      return -1;
+  }
 
   return position;
 }

diff --git a/tests/check/gst/gstsegment.c b/tests/check/gst/gstsegment.c
index 8b05351..78e0688 100644 (file)
--- a/tests/check/gst/gstsegment.c
+++ b/tests/check/gst/gstsegment.c
@@ -798,6 +798,86 @@ GST_START_TEST (segment_full)
 
 GST_END_TEST;
 
+GST_START_TEST (segment_negative_rate)
+{
+  GstSegment segment;
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+
+  segment.start = 50;
+  segment.position = 150;
+  segment.stop = 200;
+  segment.time = 0;
+  segment.applied_rate = -1;
+  segment.rate = -1;
+
+  /* somewhere in the middle */
+  check_times (&segment, 100, 100, 100);
+  /* after stop */
+  check_times (&segment, 220, -1, -1);
+  /* before start */
+  check_times (&segment, 10, -1, -1);
+  /* at segment start */
+  check_times (&segment, 50, 150, 150);
+  /* another place in the middle */
+  check_times (&segment, 150, 50, 50);
+  /* at segment stop */
+  check_times (&segment, 200, 0, 0);
+
+  segment.time = 100;
+  segment.base = 100;
+  /* somewhere in the middle */
+  check_times (&segment, 100, 200, 200);
+  /* at segment start */
+  check_times (&segment, 50, 250, 250);
+  /* another place in the middle */
+  check_times (&segment, 150, 150, 150);
+  /* at segment stop */
+  check_times (&segment, 200, 100, 100);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (segment_negative_applied_rate)
+{
+  GstSegment segment;
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+
+  segment.start = 50;
+  segment.position = 150;
+  segment.stop = 200;
+  segment.time = 0;
+  segment.applied_rate = -1;
+  segment.rate = 1;
+
+  /* somewhere in the middle */
+  check_times (&segment, 100, 100, 50);
+  /* after stop */
+  check_times (&segment, 220, -1, -1);
+  /* before start */
+  check_times (&segment, 10, -1, -1);
+  /* at segment start */
+  check_times (&segment, 50, 150, 0);
+  /* another place in the middle */
+  check_times (&segment, 150, 50, 100);
+  /* at segment stop */
+  check_times (&segment, 200, 0, 150);
+
+  segment.time = 100;
+  segment.base = 100;
+  /* somewhere in the middle */
+  check_times (&segment, 100, 200, 150);
+  /* at segment start */
+  check_times (&segment, 50, 250, 100);
+  /* another place in the middle */
+  check_times (&segment, 150, 150, 200);
+  /* at segment stop */
+  check_times (&segment, 200, 100, 250);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_segment_suite (void)
 {
@@ -815,6 +895,8 @@ gst_segment_suite (void)
   tcase_add_test (tc_chain, segment_seek_noupdate);
   tcase_add_test (tc_chain, segment_offset);
   tcase_add_test (tc_chain, segment_full);
+  tcase_add_test (tc_chain, segment_negative_rate);
+  tcase_add_test (tc_chain, segment_negative_applied_rate);
 
   return s;
 }