Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
videoaggregator: ensure peek_next_sample() uses the correct caps
gst_pad_get_current_caps() may be wrong when there is a renegotiation in
progress for the pad and we have not yet received or selected the buffer
with different caps yet.

Fix by storing the caps through in a similar way to the existing code
for buffer/video-info selection machinery.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/850>
  • Loading branch information
ystreet authored and tp-m committed Oct 4, 2020
1 parent ba39a54 commit 6c7cd66
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions gst-libs/gst/video/gstvideoaggregator.c
Expand Up @@ -68,6 +68,7 @@ enum
struct _GstVideoAggregatorPadPrivate
{
GstBuffer *buffer;
GstCaps *caps;
GstVideoFrame prepared_frame;

/* properties */
Expand All @@ -83,6 +84,7 @@ struct _GstVideoAggregatorPadPrivate
GstClockTime end_time;

GstVideoInfo pending_vinfo;
GstCaps *pending_caps;
};


Expand Down Expand Up @@ -161,6 +163,7 @@ _flush_pad (GstAggregatorPad * aggpad, GstAggregator * aggregator)

gst_video_aggregator_reset_qos (vagg);
gst_buffer_replace (&pad->priv->buffer, NULL);
gst_caps_replace (&pad->priv->caps, NULL);
pad->priv->start_time = -1;
pad->priv->end_time = -1;

Expand Down Expand Up @@ -221,9 +224,8 @@ gst_video_aggregator_peek_next_sample (GstAggregator * agg,
GstSample *res = NULL;

if (vaggpad->priv->buffer) {
GstCaps *caps = gst_pad_get_current_caps (GST_PAD (aggpad));
res = gst_sample_new (vaggpad->priv->buffer, caps, &aggpad->segment, NULL);
gst_caps_unref (caps);
res = gst_sample_new (vaggpad->priv->buffer, vaggpad->priv->caps,
&aggpad->segment, NULL);
}

return res;
Expand Down Expand Up @@ -1147,12 +1149,14 @@ gst_video_aggregator_pad_sink_setcaps (GstPad * pad, GstObject * parent,
* that this pad is getting configured; configure immediately to avoid
* problems with the initial negotiation */
vaggpad->info = info;
gst_caps_replace (&vaggpad->priv->caps, caps);
gst_pad_mark_reconfigure (GST_AGGREGATOR_SRC_PAD (vagg));
} else {
/* this pad already had caps but received new ones; keep the new caps
* pending until we pick the next buffer from the queue, otherwise we
* might use an old buffer with the new caps and crash */
vaggpad->priv->pending_vinfo = info;
gst_caps_replace (&vaggpad->priv->pending_caps, caps);
GST_DEBUG_OBJECT (pad, "delaying caps change");
}
ret = TRUE;
Expand Down Expand Up @@ -1414,6 +1418,7 @@ gst_video_aggregator_reset (GstVideoAggregator * vagg)
GstVideoAggregatorPad *p = l->data;

gst_buffer_replace (&p->priv->buffer, NULL);
gst_caps_replace (&p->priv->caps, NULL);
p->priv->start_time = -1;
p->priv->end_time = -1;

Expand Down Expand Up @@ -1490,6 +1495,8 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
"output_start_running_time. Discarding old buffer");
gst_buffer_replace (&pad->priv->buffer, buf);
if (pad->priv->pending_vinfo.finfo) {
gst_caps_replace (&pad->priv->caps, pad->priv->pending_caps);
gst_caps_replace (&pad->priv->pending_caps, NULL);
pad->info = pad->priv->pending_vinfo;
need_reconfigure = TRUE;
pad->priv->pending_vinfo.finfo = NULL;
Expand All @@ -1504,6 +1511,8 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
buf = gst_aggregator_pad_pop_buffer (bpad);
gst_buffer_replace (&pad->priv->buffer, buf);
if (pad->priv->pending_vinfo.finfo) {
gst_caps_replace (&pad->priv->caps, pad->priv->pending_caps);
gst_caps_replace (&pad->priv->pending_caps, NULL);
pad->info = pad->priv->pending_vinfo;
need_reconfigure = TRUE;
pad->priv->pending_vinfo.finfo = NULL;
Expand Down Expand Up @@ -1564,6 +1573,8 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
GST_TIME_ARGS (start_time));
gst_buffer_replace (&pad->priv->buffer, buf);
if (pad->priv->pending_vinfo.finfo) {
gst_caps_replace (&pad->priv->caps, pad->priv->pending_caps);
gst_caps_replace (&pad->priv->pending_caps, NULL);
pad->info = pad->priv->pending_vinfo;
need_reconfigure = TRUE;
pad->priv->pending_vinfo.finfo = NULL;
Expand All @@ -1582,6 +1593,8 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
} else {
gst_buffer_replace (&pad->priv->buffer, buf);
if (pad->priv->pending_vinfo.finfo) {
gst_caps_replace (&pad->priv->caps, pad->priv->pending_caps);
gst_caps_replace (&pad->priv->pending_caps, NULL);
pad->info = pad->priv->pending_vinfo;
need_reconfigure = TRUE;
pad->priv->pending_vinfo.finfo = NULL;
Expand Down Expand Up @@ -1614,20 +1627,23 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
pad->priv->max_last_buffer_repeat) {
pad->priv->start_time = pad->priv->end_time = -1;
gst_buffer_replace (&pad->priv->buffer, NULL);
gst_caps_replace (&pad->priv->caps, NULL);
}
} else {
pad->priv->start_time = pad->priv->end_time = -1;
}
need_more_data = TRUE;
} else {
gst_buffer_replace (&pad->priv->buffer, NULL);
gst_caps_replace (&pad->priv->caps, NULL);
pad->priv->start_time = pad->priv->end_time = -1;
}
} else if (is_eos) {
eos = FALSE;
}
} else if (is_eos) {
gst_buffer_replace (&pad->priv->buffer, NULL);
gst_caps_replace (&pad->priv->caps, NULL);
} else if (pad->priv->start_time != -1) {
/* When the current buffer didn't have a duration, but
* max-last-buffer-repeat was set, we use start_time as
Expand All @@ -1639,6 +1655,7 @@ gst_video_aggregator_fill_queues (GstVideoAggregator * vagg,
pad->priv->max_last_buffer_repeat) {
pad->priv->start_time = pad->priv->end_time = -1;
gst_buffer_replace (&pad->priv->buffer, NULL);
gst_caps_replace (&pad->priv->caps, NULL);
}
}
}
Expand Down Expand Up @@ -2290,6 +2307,8 @@ gst_video_aggregator_release_pad (GstElement * element, GstPad * pad)
gst_video_aggregator_reset (vagg);

gst_buffer_replace (&vaggpad->priv->buffer, NULL);
gst_caps_replace (&vaggpad->priv->caps, NULL);
gst_caps_replace (&vaggpad->priv->pending_caps, NULL);

GST_ELEMENT_CLASS (gst_video_aggregator_parent_class)->release_pad
(GST_ELEMENT (vagg), pad);
Expand Down

0 comments on commit 6c7cd66

Please sign in to comment.