Commit 637106e2 authored by Sebastian Dröge's avatar Sebastian Dröge 🍵

audioaggregator: Fix mixup of running times and segment positions

We have to queue buffers based on their running time, not based on
the segment position.

Also return running time from GstAggregator::get_next_time() instead of
a segment position, as required by the API.

Also only update the segment position after we pushed a buffer, otherwise
we're going to push down a segment event with the next position already.

https://bugzilla.gnome.org/show_bug.cgi?id=753196
parent 97fe89f3
......@@ -53,11 +53,12 @@ struct _GstAudioAggregatorPadPrivate
cached values. */
guint position, size;
guint64 output_offset; /* Offset in output segment that
collect.pos refers to in the
guint64 output_offset; /* Sample offset in output segment relative to
segment.start that collect.pos refers to in the
current buffer. */
guint64 next_offset; /* Next expected offset in the input segment */
guint64 next_offset; /* Next expected sample offset in the input segment
relative to segment.start */
/* Last time we noticed a discont */
GstClockTime discont_time;
......@@ -145,7 +146,8 @@ struct _GstAudioAggregatorPrivate
/* counters to keep track of timestamps */
/* Readable with object lock, writable with both aag lock and object lock */
gint64 offset;
gint64 offset; /* Sample offset starting from 0 at segment.start */
};
#define GST_AUDIO_AGGREGATOR_LOCK(self) g_mutex_lock (&(self)->priv->mutex);
......@@ -195,10 +197,16 @@ gst_audio_aggregator_get_next_time (GstAggregator * agg)
GstClockTime next_time;
GST_OBJECT_LOCK (agg);
if (agg->segment.position == -1)
if (agg->segment.position == -1 || agg->segment.position < agg->segment.start)
next_time = agg->segment.start;
else
next_time = agg->segment.position;
if (agg->segment.stop != -1 && next_time > agg->segment.stop)
next_time = agg->segment.stop;
next_time =
gst_segment_to_running_time (&agg->segment, GST_FORMAT_TIME, next_time);
GST_OBJECT_UNLOCK (agg);
return next_time;
......@@ -742,6 +750,7 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
guint64 start_offset, end_offset;
gint rate, bpf;
GstAggregator *agg = GST_AGGREGATOR (aagg);
GstAggregatorPad *aggpad = GST_AGGREGATOR_PAD (pad);
g_assert (pad->priv->buffer == NULL);
......@@ -767,7 +776,12 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
start_time + gst_util_uint64_scale_ceil (pad->priv->size, GST_SECOND,
rate);
start_offset = gst_util_uint64_scale (start_time, rate, GST_SECOND);
/* Clipping should've ensured this */
g_assert (start_time >= aggpad->segment.start);
start_offset =
gst_util_uint64_scale (start_time - aggpad->segment.start, rate,
GST_SECOND);
end_offset = start_offset + pad->priv->size;
if (GST_BUFFER_IS_DISCONT (inbuf)
......@@ -822,8 +836,8 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
if (pad->priv->output_offset == -1) {
GstClockTime start_running_time;
GstClockTime end_running_time;
guint64 start_running_time_offset;
guint64 end_running_time_offset;
guint64 start_output_offset;
guint64 end_output_offset;
start_running_time =
gst_segment_to_running_time (&aggpad->segment,
......@@ -831,12 +845,40 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
end_running_time =
gst_segment_to_running_time (&aggpad->segment,
GST_FORMAT_TIME, end_time);
start_running_time_offset =
gst_util_uint64_scale (start_running_time, rate, GST_SECOND);
end_running_time_offset =
gst_util_uint64_scale (end_running_time, rate, GST_SECOND);
if (end_running_time_offset < aagg->priv->offset) {
/* Convert to position in the output segment */
start_output_offset =
gst_segment_to_position (&agg->segment, GST_FORMAT_TIME,
start_running_time);
if (start_output_offset != -1)
start_output_offset =
gst_util_uint64_scale (start_output_offset - agg->segment.start, rate,
GST_SECOND);
end_output_offset =
gst_segment_to_position (&agg->segment, GST_FORMAT_TIME,
end_running_time);
if (end_output_offset != -1)
end_output_offset =
gst_util_uint64_scale (end_output_offset - agg->segment.start, rate,
GST_SECOND);
if (start_output_offset == -1 && end_output_offset == -1) {
/* Outside output segment, drop */
gst_buffer_unref (inbuf);
pad->priv->buffer = NULL;
pad->priv->position = 0;
pad->priv->size = 0;
pad->priv->output_offset = -1;
GST_DEBUG_OBJECT (pad, "Buffer outside output segment");
return FALSE;
}
/* Calculate end_output_offset if it was outside the output segment */
if (end_output_offset == -1)
end_output_offset = start_output_offset + pad->priv->size;
if (end_output_offset < aagg->priv->offset) {
/* Before output segment, drop */
gst_buffer_unref (inbuf);
pad->priv->buffer = NULL;
......@@ -845,12 +887,25 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
pad->priv->output_offset = -1;
GST_DEBUG_OBJECT (pad,
"Buffer before segment or current position: %" G_GUINT64_FORMAT " < %"
G_GUINT64_FORMAT, end_running_time_offset, aagg->priv->offset);
G_GUINT64_FORMAT, end_output_offset, aagg->priv->offset);
return FALSE;
}
if (start_running_time_offset < aagg->priv->offset) {
guint diff = aagg->priv->offset - start_running_time_offset;
if (start_output_offset == -1 || start_output_offset < aagg->priv->offset) {
guint diff;
if (start_output_offset == -1 && end_output_offset < pad->priv->size) {
diff = pad->priv->size - end_output_offset + aagg->priv->offset;
} else if (start_output_offset == -1) {
start_output_offset = end_output_offset - pad->priv->size;
if (start_output_offset < aagg->priv->offset)
diff = aagg->priv->offset - start_output_offset;
else
diff = 0;
} else {
diff = aagg->priv->offset - start_output_offset;
}
pad->priv->position += diff;
if (pad->priv->position >= pad->priv->size) {
......@@ -862,14 +917,16 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,
pad->priv->output_offset = -1;
GST_DEBUG_OBJECT (pad,
"Buffer before segment or current position: %" G_GUINT64_FORMAT
" < %" G_GUINT64_FORMAT, end_running_time_offset,
aagg->priv->offset);
" < %" G_GUINT64_FORMAT, end_output_offset, aagg->priv->offset);
return FALSE;
}
}
pad->priv->output_offset =
MAX (start_running_time_offset, aagg->priv->offset);
if (start_output_offset == -1 || start_output_offset < aagg->priv->offset)
pad->priv->output_offset = aagg->priv->offset;
else
pad->priv->output_offset = start_output_offset;
GST_DEBUG_OBJECT (pad,
"Buffer resynced: Pad offset %" G_GUINT64_FORMAT
", current audio aggregator offset %" G_GUINT64_FORMAT,
......@@ -1066,13 +1123,10 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout)
GST_OBJECT_UNLOCK (agg);
gst_aggregator_set_src_caps (agg, aagg->current_caps);
GST_OBJECT_LOCK (agg);
aagg->priv->offset = gst_util_uint64_scale (agg->segment.position,
GST_AUDIO_INFO_RATE (&aagg->info), GST_SECOND);
aagg->priv->send_caps = FALSE;
}
rate = GST_AUDIO_INFO_RATE (&aagg->info);
bpf = GST_AUDIO_INFO_BPF (&aagg->info);
......@@ -1090,7 +1144,9 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout)
next_offset = aagg->priv->offset - blocksize;
}
next_timestamp = gst_util_uint64_scale (next_offset, GST_SECOND, rate);
next_timestamp =
agg->segment.start + gst_util_uint64_scale (next_offset, GST_SECOND,
rate);
if (aagg->priv->current_buffer == NULL) {
GST_OBJECT_UNLOCK (agg);
......@@ -1248,7 +1304,9 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout)
"Last buffer is incomplete: %" G_GUINT64_FORMAT " <= %"
G_GUINT64_FORMAT, max_offset, next_offset);
next_offset = max_offset;
next_timestamp = gst_util_uint64_scale (next_offset, GST_SECOND, rate);
next_timestamp =
agg->segment.start + gst_util_uint64_scale (next_offset, GST_SECOND,
rate);
if (next_offset > aagg->priv->offset)
gst_buffer_resize (outbuf, 0, (next_offset - aagg->priv->offset) * bpf);
......@@ -1269,6 +1327,23 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout)
GST_BUFFER_DURATION (outbuf) = agg->segment.position - next_timestamp;
}
GST_OBJECT_UNLOCK (agg);
/* send it out */
GST_LOG_OBJECT (aagg,
"pushing outbuf %p, timestamp %" GST_TIME_FORMAT " offset %"
G_GINT64_FORMAT, outbuf, GST_TIME_ARGS (GST_BUFFER_PTS (outbuf)),
GST_BUFFER_OFFSET (outbuf));
GST_AUDIO_AGGREGATOR_UNLOCK (aagg);
ret = gst_aggregator_finish_buffer (agg, aagg->priv->current_buffer);
aagg->priv->current_buffer = NULL;
GST_LOG_OBJECT (aagg, "pushed outbuf, result = %s", gst_flow_get_name (ret));
GST_AUDIO_AGGREGATOR_LOCK (aagg);
GST_OBJECT_LOCK (agg);
aagg->priv->offset = next_offset;
agg->segment.position = next_timestamp;
......@@ -1285,22 +1360,9 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout)
GST_OBJECT_UNLOCK (pad);
}
}
GST_OBJECT_UNLOCK (agg);
/* send it out */
GST_LOG_OBJECT (aagg,
"pushing outbuf %p, timestamp %" GST_TIME_FORMAT " offset %"
G_GINT64_FORMAT, outbuf, GST_TIME_ARGS (GST_BUFFER_PTS (outbuf)),
GST_BUFFER_OFFSET (outbuf));
GST_AUDIO_AGGREGATOR_UNLOCK (aagg);
ret = gst_aggregator_finish_buffer (agg, aagg->priv->current_buffer);
aagg->priv->current_buffer = NULL;
GST_LOG_OBJECT (aagg, "pushed outbuf, result = %s", gst_flow_get_name (ret));
return ret;
/* ERRORS */
not_negotiated:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment