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
This commit is contained in:
Tim-Philipp Müller 2017-01-23 19:08:15 +00:00
parent 83e84d5acf
commit d6c0e9072b
2 changed files with 27 additions and 7 deletions

View file

@ -1018,8 +1018,21 @@ gst_video_rate_query (GstBaseTransform * trans, GstPadDirection direction,
{ {
GstFormat format; GstFormat format;
gint64 duration; 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); 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, GST_LOG_OBJECT (videorate, "upstream duration: %" G_GINT64_FORMAT,
duration); duration);
/* Shouldn't this be a multiplication if the direction is downstream? */
if (GST_CLOCK_TIME_IS_VALID (duration)) { 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_LOG_OBJECT (videorate, "our duration: %" G_GINT64_FORMAT, duration);
gst_query_set_duration (query, format, duration); gst_query_set_duration (query, format, duration);
res = TRUE;
break; break;
} }
case GST_QUERY_POSITION: case GST_QUERY_POSITION:
{ {
GstFormat dst_format; GstFormat dst_format;
gint64 dst_value; gint64 dst_value;
gdouble rate;
gst_query_parse_position (query, &dst_format, &dst_value); GST_OBJECT_LOCK (videorate);
rate = videorate->rate;
GST_OBJECT_UNLOCK (videorate);
gst_query_parse_position (query, &dst_format, NULL);
if (dst_format != GST_FORMAT_TIME) { if (dst_format != GST_FORMAT_TIME) {
GST_DEBUG_OBJECT (videorate, "not TIME format"); GST_DEBUG_OBJECT (videorate, "not TIME format");
break; break;
} }
/* Shouldn't this be a multiplication if the direction is downstream? */
dst_value = dst_value =
(gint64) (gst_segment_to_stream_time (&videorate->segment, (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_LOG_OBJECT (videorate, "our position: %" GST_TIME_FORMAT,
GST_TIME_ARGS (dst_value)); GST_TIME_ARGS (dst_value));
gst_query_set_position (query, dst_format, dst_value); gst_query_set_position (query, dst_format, dst_value);

View file

@ -1287,6 +1287,7 @@ listen_sink_query_duration (GstPad * pad, GstPadProbeInfo * info,
if (GST_QUERY_TYPE (query) == GST_QUERY_DURATION) { if (GST_QUERY_TYPE (query) == GST_QUERY_DURATION) {
gst_query_set_duration (query, GST_FORMAT_TIME, *duration); gst_query_set_duration (query, GST_FORMAT_TIME, *duration);
return GST_PAD_PROBE_HANDLED;
} }
return GST_PAD_PROBE_OK; return GST_PAD_PROBE_OK;
} }
@ -1304,12 +1305,12 @@ GST_START_TEST (test_query_duration)
"could not set to playing"); "could not set to playing");
probe_sink = probe_sink =
gst_pad_add_probe (mysrcpad, 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); (GstPadProbeCallback) listen_sink_query_duration, &duration, NULL);
query = gst_query_new_duration (GST_FORMAT_TIME); query = gst_query_new_duration (GST_FORMAT_TIME);
duration = GST_CLOCK_TIME_NONE; duration = GST_CLOCK_TIME_NONE;
gst_pad_peer_query (mysrcpad, query); gst_pad_peer_query (mysinkpad, query);
gst_query_parse_duration (query, NULL, &duration); gst_query_parse_duration (query, NULL, &duration);
fail_unless_equals_uint64 (duration, GST_CLOCK_TIME_NONE); fail_unless_equals_uint64 (duration, GST_CLOCK_TIME_NONE);
@ -1319,7 +1320,7 @@ GST_START_TEST (test_query_duration)
/* Setting rate to 2.0 */ /* Setting rate to 2.0 */
g_object_set (videorate, "rate", 2.0, NULL); 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); gst_query_parse_duration (query, NULL, &duration);
fail_unless_equals_uint64 (duration, 0.5 * GST_SECOND); fail_unless_equals_uint64 (duration, 0.5 * GST_SECOND);