videorate: fix duration and position query handling

Duration query would return TRUE and duration=-1. This
worked in the unit test because the unit test implementation
was a bit broken.

Both queries need to access rate with a lock.

Fix broken duration query test as well. It relied on broken
behaviour by the videorate query handler, and also it was
implemented as a downstream query rather than an upstream
query. And we must return HANDLED from the probe so that the
query we intercept actually returns TRUE.

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

diff --git a/gst/videorate/gstvideorate.c b/gst/videorate/gstvideorate.c
index 86b4d89d2bff5d7486bbce6bbccd919248799b9b..8b5c9943ccb85ebf5e85d9a56f78e3ab97f8d8bb 100644 (file)
--- a/gst/videorate/gstvideorate.c
+++ b/gst/videorate/gstvideorate.c
@@ -1018,8 +1018,21 @@ gst_video_rate_query (GstBaseTransform * trans, GstPadDirection direction,
     {
       GstFormat format;
       gint64 duration;
+      gdouble rate;
 
+      res =
+          GST_BASE_TRANSFORM_CLASS (parent_class)->query (trans, direction,
+          query);
 
+      if (!res)
+        break;
+
+      GST_OBJECT_LOCK (videorate);
+      rate = videorate->rate;
+      GST_OBJECT_UNLOCK (videorate);
+
+      if (rate == 1.0)
+        break;
 
       gst_query_parse_duration (query, &format, &duration);
 
@@ -1029,28 +1042,34 @@ gst_video_rate_query (GstBaseTransform * trans, GstPadDirection direction,
       }
       GST_LOG_OBJECT (videorate, "upstream duration: %" G_GINT64_FORMAT,
           duration);
+      /* Shouldn't this be a multiplication if the direction is downstream? */
       if (GST_CLOCK_TIME_IS_VALID (duration)) {
-        duration = (gint64) (duration / videorate->rate);
+        duration = (gint64) (duration / rate);
       }
       GST_LOG_OBJECT (videorate, "our duration: %" G_GINT64_FORMAT, duration);
       gst_query_set_duration (query, format, duration);
-      res = TRUE;
       break;
     }
     case GST_QUERY_POSITION:
     {
       GstFormat dst_format;
       gint64 dst_value;
+      gdouble rate;
+
+      GST_OBJECT_LOCK (videorate);
+      rate = videorate->rate;
+      GST_OBJECT_UNLOCK (videorate);
 
-      gst_query_parse_position (query, &dst_format, &dst_value);
+      gst_query_parse_position (query, &dst_format, NULL);
 
       if (dst_format != GST_FORMAT_TIME) {
         GST_DEBUG_OBJECT (videorate, "not TIME format");
         break;
       }
+      /* Shouldn't this be a multiplication if the direction is downstream? */
       dst_value =
           (gint64) (gst_segment_to_stream_time (&videorate->segment,
-              GST_FORMAT_TIME, videorate->last_ts / videorate->rate));
+              GST_FORMAT_TIME, videorate->last_ts / rate));
       GST_LOG_OBJECT (videorate, "our position: %" GST_TIME_FORMAT,
           GST_TIME_ARGS (dst_value));
       gst_query_set_position (query, dst_format, dst_value);

diff --git a/tests/check/elements/videorate.c b/tests/check/elements/videorate.c
index 132f9c1adaadbefd1eb4ef85df697d30d3732235..7db55199ab7bf481a1e804acfa86d497581c90bd 100644 (file)
--- a/tests/check/elements/videorate.c
+++ b/tests/check/elements/videorate.c
@@ -1287,6 +1287,7 @@ listen_sink_query_duration (GstPad * pad, GstPadProbeInfo * info,
 
   if (GST_QUERY_TYPE (query) == GST_QUERY_DURATION) {
     gst_query_set_duration (query, GST_FORMAT_TIME, *duration);
+    return GST_PAD_PROBE_HANDLED;
   }
   return GST_PAD_PROBE_OK;
 }
@@ -1304,12 +1305,12 @@ GST_START_TEST (test_query_duration)
       "could not set to playing");
   probe_sink =
       gst_pad_add_probe (mysrcpad,
-      GST_PAD_PROBE_TYPE_QUERY_DOWNSTREAM | GST_PAD_PROBE_TYPE_PUSH,
+      GST_PAD_PROBE_TYPE_QUERY_UPSTREAM,
       (GstPadProbeCallback) listen_sink_query_duration, &duration, NULL);
 
   query = gst_query_new_duration (GST_FORMAT_TIME);
   duration = GST_CLOCK_TIME_NONE;
-  gst_pad_peer_query (mysrcpad, query);
+  gst_pad_peer_query (mysinkpad, query);
   gst_query_parse_duration (query, NULL, &duration);
   fail_unless_equals_uint64 (duration, GST_CLOCK_TIME_NONE);
 
@@ -1319,7 +1320,7 @@ GST_START_TEST (test_query_duration)
   /* Setting rate to 2.0 */
   g_object_set (videorate, "rate", 2.0, NULL);
 
-  gst_pad_peer_query (mysrcpad, query);
+  gst_pad_peer_query (mysinkpad, query);
   gst_query_parse_duration (query, NULL, &duration);
   fail_unless_equals_uint64 (duration, 0.5 * GST_SECOND);