gst/gstsegment.c: Also accumulate time correctly when doing reverse playback. Fixes...

Original commit message from CVS:
* 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.

diff --git a/ChangeLog b/ChangeLog
index d2fccf7..8ab1b94 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2007-11-09  Wim Taymans  <wim.taymans@gmail.com>
+
+       * 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.
+
 2007-11-07  Tim-Philipp Müller  <tim at centricular dot net>
 
        * gst/gst.c: (init_pre):

diff --git a/gst/gstsegment.c b/gst/gstsegment.c
index 29be652..8ee6ef3 100644 (file)
--- a/gst/gstsegment.c
+++ b/gst/gstsegment.c
@@ -443,12 +443,21 @@ gst_segment_set_newsegment_full (GstSegment * segment, gboolean update,
   g_return_if_fail (segment->format == format);
 
   if (update) {
-    /* an update to the current segment is done, elapsed time is
-     * difference between the old start and new start. */
-    if (start > segment->start)
-      duration = start - segment->start;
-    else
-      duration = 0;
+    if (segment->rate > 0.0) {
+      /* an update to the current segment is done, elapsed time is
+       * difference between the old start and new start. */
+      if (start > segment->start)
+        duration = start - segment->start;
+      else
+        duration = 0;
+    } else {
+      /* for negative rates, the elapsed duration is the diff between the stop
+       * positions */
+      if (stop != -1 && stop < segment->stop)
+        duration = segment->stop - stop;
+      else
+        duration = 0;
+    }
   } else {
     /* the new segment has to be aligned with the old segment.
      * We first update the accumulated time of the previous
@@ -508,7 +517,7 @@ gint64
 gst_segment_to_stream_time (GstSegment * segment, GstFormat format,
     gint64 position)
 {
-  gint64 result;
+  gint64 result, start, stop, time;
   gdouble abs_applied_rate;
 
   g_return_val_if_fail (segment != NULL, -1);
@@ -519,23 +528,33 @@ gst_segment_to_stream_time (GstSegment * segment, GstFormat format,
 
   if (G_UNLIKELY (segment->format == GST_FORMAT_UNDEFINED))
     segment->format = format;
-  else
-    g_return_val_if_fail (segment->format == format, -1);
+
+  /* if we have the position for the same format as the segment, we can compare
+   * the start and stop values, otherwise we assume 0 and -1 */
+  if (segment->format == format) {
+    start = segment->start;
+    stop = segment->stop;
+    time = segment->time;
+  } else {
+    start = 0;
+    stop = -1;
+    time = 0;
+  }
 
   /* outside of the segment boundary stop */
-  if (G_UNLIKELY (segment->stop != -1 && position > segment->stop))
+  if (G_UNLIKELY (stop != -1 && position > stop))
     return -1;
 
   /* before the segment boundary */
-  if (G_UNLIKELY (position < segment->start))
+  if (G_UNLIKELY (position < start))
     return -1;
 
   /* time must be known */
-  if (G_UNLIKELY (segment->time == -1))
+  if (G_UNLIKELY (time == -1))
     return -1;
 
   /* bring to uncorrected position in segment */
-  result = position - segment->start;
+  result = position - start;
 
   abs_applied_rate = ABS (segment->applied_rate);
 
@@ -546,11 +565,11 @@ gst_segment_to_stream_time (GstSegment * segment, GstFormat format,
   /* add or subtract from segment time based on applied rate */
   if (segment->applied_rate > 0.0) {
     /* correct for segment time */
-    result += segment->time;
+    result += time;
   } else {
     /* correct for segment time, clamp at 0 */
-    if (segment->time > result)
-      result = segment->time - result;
+    if (time > result)
+      result = time - result;
     else
       result = 0;
   }
@@ -583,6 +602,7 @@ gst_segment_to_running_time (GstSegment * segment, GstFormat format,
     gint64 position)
 {
   gint64 result;
+  gint64 start, stop, accum;
 
   g_return_val_if_fail (segment != NULL, -1);
 
@@ -591,28 +611,38 @@ gst_segment_to_running_time (GstSegment * segment, GstFormat format,
 
   if (G_UNLIKELY (segment->format == GST_FORMAT_UNDEFINED))
     segment->format = format;
-  else if (segment->accum)
-    g_return_val_if_fail (segment->format == format, -1);
+
+  /* if we have the position for the same format as the segment, we can compare
+   * the start and stop values, otherwise we assume 0 and -1 */
+  if (segment->format == format) {
+    start = segment->start;
+    stop = segment->stop;
+    accum = segment->accum;
+  } else {
+    start = 0;
+    stop = -1;
+    accum = 0;
+  }
 
   /* before the segment boundary */
-  if (G_UNLIKELY (position < segment->start))
+  if (G_UNLIKELY (position < start))
     return -1;
 
   if (segment->rate > 0.0) {
     /* outside of the segment boundary stop */
-    if (G_UNLIKELY (segment->stop != -1 && position > segment->stop))
+    if (G_UNLIKELY (stop != -1 && position > stop))
       return -1;
 
     /* bring to uncorrected position in segment */
-    result = position - segment->start;
+    result = position - start;
   } else {
     /* cannot continue if no stop position set or outside of
      * the segment. */
-    if (G_UNLIKELY (segment->stop == -1 || position > segment->stop))
+    if (G_UNLIKELY (stop == -1 || position > stop))
       return -1;
 
     /* bring to uncorrected position in segment */
-    result = segment->stop - position;
+    result = stop - position;
   }
 
   /* scale based on the rate, avoid division by and conversion to 
@@ -621,7 +651,7 @@ gst_segment_to_running_time (GstSegment * segment, GstFormat format,
     result /= segment->abs_rate;
 
   /* correct for accumulated segments */
-  result += segment->accum;
+  result += accum;
 
   return result;
 }

diff --git a/libs/gst/base/gstbasesink.c b/libs/gst/base/gstbasesink.c
index 120806c..07d57a3 100644 (file)
--- a/libs/gst/base/gstbasesink.c
+++ b/libs/gst/base/gstbasesink.c
@@ -1341,6 +1341,8 @@ gst_base_sink_get_sync_times (GstBaseSink * basesink, GstMiniObject * obj,
   if (G_UNLIKELY (format != GST_FORMAT_TIME)) {
     cstart = start;
     cstop = stop;
+    /* do running and stream time in TIME format */
+    format = GST_FORMAT_TIME;
     goto do_times;
   }
 

diff --git a/tests/check/gst/gstsegment.c b/tests/check/gst/gstsegment.c
index f9fc415..ba4e12e 100644 (file)
--- a/tests/check/gst/gstsegment.c
+++ b/tests/check/gst/gstsegment.c
@@ -1334,6 +1334,165 @@ GST_START_TEST (segment_newsegment_runningtime)
 
 GST_END_TEST;
 
+/* mess with the segment structure in the time format */
+GST_START_TEST (segment_newsegment_accum)
+{
+  GstSegment segment;
+  gint64 result;
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+
+  /***************************
+   * Normal reverse segment
+   ***************************/
+  gst_segment_set_newsegment_full (&segment, FALSE, -1.0, 1.0,
+      GST_FORMAT_TIME, 0, 200, 0);
+
+  fail_unless (segment.rate == -1.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 0);
+  fail_unless (segment.stop == 200);
+  fail_unless (segment.time == 0);
+  fail_unless (segment.accum == 0);
+  fail_unless (segment.last_stop == 0);
+  fail_unless (segment.duration == -1);
+
+  /* invalid time gives invalid result */
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, -1);
+  fail_unless (result == -1);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 200);
+  fail_unless (result == 0);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 150);
+  fail_unless (result == 50);
+
+  /* update segment, this accumulates 50 from the previous segment. */
+  gst_segment_set_newsegment_full (&segment, TRUE, -2.0, 1.0,
+      GST_FORMAT_TIME, 0, 150, 0);
+
+  fail_unless (segment.rate == -2.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 0);
+  fail_unless (segment.stop == 150);
+  fail_unless (segment.time == 0);
+  fail_unless (segment.accum == 50);
+  fail_unless (segment.last_stop == 0);
+  fail_unless (segment.duration == -1);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 150);
+  fail_unless (result == 50);
+
+  /* 50 accumulated + 50 / 2 */
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 100);
+  fail_unless (result == 75);
+
+  /* update segment, this does not accumulate anything. */
+  gst_segment_set_newsegment_full (&segment, TRUE, 1.0, 1.0,
+      GST_FORMAT_TIME, 100, 200, 100);
+
+  fail_unless (segment.rate == 1.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 100);
+  fail_unless (segment.stop == 200);
+  fail_unless (segment.time == 100);
+  fail_unless (segment.accum == 50);
+  fail_unless (segment.last_stop == 100);
+  fail_unless (segment.duration == -1);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 100);
+  fail_unless (result == 50);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 150);
+  fail_unless (result == 100);
+}
+
+GST_END_TEST;
+
+/* mess with the segment structure in the time format */
+GST_START_TEST (segment_newsegment_accum2)
+{
+  GstSegment segment;
+  gint64 result;
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+
+  /***************************
+   * Normal reverse segment
+   ***************************/
+  gst_segment_set_newsegment_full (&segment, FALSE, -1.0, 1.0,
+      GST_FORMAT_TIME, 0, 200, 0);
+
+  fail_unless (segment.rate == -1.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 0);
+  fail_unless (segment.stop == 200);
+  fail_unless (segment.time == 0);
+  fail_unless (segment.accum == 0);
+  fail_unless (segment.last_stop == 0);
+  fail_unless (segment.duration == -1);
+
+  /* invalid time gives invalid result */
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, -1);
+  fail_unless (result == -1);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 200);
+  fail_unless (result == 0);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 150);
+  fail_unless (result == 50);
+
+  /* close segment, this accumulates nothing. */
+  gst_segment_set_newsegment_full (&segment, TRUE, -1.0, 1.0,
+      GST_FORMAT_TIME, 150, 200, 0);
+
+  fail_unless (segment.rate == -1.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 150);
+  fail_unless (segment.stop == 200);
+  fail_unless (segment.time == 0);
+  fail_unless (segment.accum == 0);
+  fail_unless (segment.last_stop == 150);
+  fail_unless (segment.duration == -1);
+
+  /* new segment, this accumulates 50. */
+  gst_segment_set_newsegment_full (&segment, FALSE, 1.0, 1.0,
+      GST_FORMAT_TIME, 150, 300, 150);
+
+  fail_unless (segment.rate == 1.0);
+  fail_unless (segment.applied_rate == 1.0);
+  fail_unless (segment.format == GST_FORMAT_TIME);
+  fail_unless (segment.flags == 0);
+  fail_unless (segment.start == 150);
+  fail_unless (segment.stop == 300);
+  fail_unless (segment.time == 150);
+  fail_unless (segment.accum == 50);
+  fail_unless (segment.last_stop == 150);
+  fail_unless (segment.duration == -1);
+
+  /* invalid time gives invalid result */
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, -1);
+  fail_unless (result == -1);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 150);
+  fail_unless (result == 50);
+
+  result = gst_segment_to_running_time (&segment, GST_FORMAT_TIME, 200);
+  fail_unless (result == 100);
+}
+
+GST_END_TEST;
+
 Suite *
 gst_segment_suite (void)
 {
@@ -1353,6 +1512,8 @@ gst_segment_suite (void)
   tcase_add_test (tc_chain, segment_newsegment_streamtime_applied_rate);
   tcase_add_test (tc_chain, segment_newsegment_streamtime_applied_rate_rate);
   tcase_add_test (tc_chain, segment_newsegment_runningtime);
+  tcase_add_test (tc_chain, segment_newsegment_accum);
+  tcase_add_test (tc_chain, segment_newsegment_accum2);
 
   return s;
 }