From 79ef75888c88c5883b8963c9e85d4260fd57f72d Mon Sep 17 00:00:00 2001 From: Julien Isorce Date: Thu, 5 Dec 2013 14:31:25 +0000 Subject: [PATCH] videodec/enc: clear reconfigure flag if negotiate succeeds So that it avoids to send an allocation query twice. One from an early call to gst_video_encoder_negotiate from a subclass, then one from gst_video_encoder_allocate_output_frame. Which means that previously gst_video_encoder_negotiate was not clearing the GST_PAD_FLAG_NEED_RECONFIGURE even on success. Fixes bug https://bugzilla.gnome.org/show_bug.cgi?id=719684 --- gst-libs/gst/video/gstvideodecoder.c | 44 +++++++++++++++++++++------- gst-libs/gst/video/gstvideoencoder.c | 43 +++++++++++++++++++++------ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/gst-libs/gst/video/gstvideodecoder.c b/gst-libs/gst/video/gstvideodecoder.c index 390e7e34d4..a37c29dead 100644 --- a/gst-libs/gst/video/gstvideodecoder.c +++ b/gst-libs/gst/video/gstvideodecoder.c @@ -459,6 +459,8 @@ static gboolean gst_video_decoder_propose_allocation_default (GstVideoDecoder * static gboolean gst_video_decoder_negotiate_default (GstVideoDecoder * decoder); static GstFlowReturn gst_video_decoder_parse_available (GstVideoDecoder * dec, gboolean at_eos, gboolean new_buffer); +static gboolean gst_video_decoder_negotiate_unlocked (GstVideoDecoder * + decoder); /* we can't use G_DEFINE_ABSTRACT_TYPE because we need the klass in the _init * method to get to the padtemplates */ @@ -2461,14 +2463,16 @@ gst_video_decoder_finish_frame (GstVideoDecoder * decoder, GstFlowReturn ret = GST_FLOW_OK; GstVideoDecoderPrivate *priv = decoder->priv; GstBuffer *output_buffer; + gboolean needs_reconfigure = FALSE; GST_LOG_OBJECT (decoder, "finish frame %p", frame); GST_VIDEO_DECODER_STREAM_LOCK (decoder); + needs_reconfigure = gst_pad_check_reconfigure (decoder->srcpad); if (G_UNLIKELY (priv->output_state_changed || (priv->output_state - && gst_pad_check_reconfigure (decoder->srcpad)))) { - if (!gst_video_decoder_negotiate (decoder)) { + && needs_reconfigure))) { + if (!gst_video_decoder_negotiate_unlocked (decoder)) { gst_pad_mark_reconfigure (decoder->srcpad); if (GST_PAD_IS_FLUSHING (decoder->srcpad)) ret = GST_FLOW_FLUSHING; @@ -3183,11 +3187,25 @@ done: return ret; } +static gboolean +gst_video_decoder_negotiate_unlocked (GstVideoDecoder * decoder) +{ + GstVideoDecoderClass *klass = GST_VIDEO_DECODER_GET_CLASS (decoder); + gboolean ret = TRUE; + + if (G_LIKELY (klass->negotiate)) + ret = klass->negotiate (decoder); + + return ret; +} + /** * gst_video_decoder_negotiate: * @decoder: a #GstVideoDecoder * - * Negotiate with downstreame elements to currently configured #GstVideoCodecState. + * Negotiate with downstream elements to currently configured #GstVideoCodecState. + * Unmark GST_PAD_FLAG_NEED_RECONFIGURE in any case. But mark it again if + * negotiate fails. * * Returns: #TRUE if the negotiation succeeded, else #FALSE. */ @@ -3202,8 +3220,12 @@ gst_video_decoder_negotiate (GstVideoDecoder * decoder) klass = GST_VIDEO_DECODER_GET_CLASS (decoder); GST_VIDEO_DECODER_STREAM_LOCK (decoder); - if (klass->negotiate) + gst_pad_check_reconfigure (decoder->srcpad); + if (klass->negotiate) { ret = klass->negotiate (decoder); + if (!ret) + gst_pad_mark_reconfigure (decoder->srcpad); + } GST_VIDEO_DECODER_STREAM_UNLOCK (decoder); return ret; @@ -3227,14 +3249,15 @@ gst_video_decoder_allocate_output_buffer (GstVideoDecoder * decoder) { GstFlowReturn flow; GstBuffer *buffer = NULL; + gboolean needs_reconfigure = FALSE; GST_DEBUG ("alloc src buffer"); GST_VIDEO_DECODER_STREAM_LOCK (decoder); + needs_reconfigure = gst_pad_check_reconfigure (decoder->srcpad); if (G_UNLIKELY (!decoder->priv->output_state - || decoder->priv->output_state_changed - || gst_pad_check_reconfigure (decoder->srcpad))) { - if (!gst_video_decoder_negotiate (decoder)) { + || decoder->priv->output_state_changed || needs_reconfigure)) { + if (!gst_video_decoder_negotiate_unlocked (decoder)) { if (decoder->priv->output_state) { GST_DEBUG_OBJECT (decoder, "Failed to negotiate, fallback allocation"); gst_pad_mark_reconfigure (decoder->srcpad); @@ -3295,6 +3318,7 @@ gst_video_decoder_allocate_output_frame (GstVideoDecoder * GstFlowReturn flow_ret; GstVideoCodecState *state; int num_bytes; + gboolean needs_reconfigure = FALSE; g_return_val_if_fail (decoder->priv->output_state, GST_FLOW_NOT_NEGOTIATED); g_return_val_if_fail (frame->output_buffer == NULL, GST_FLOW_ERROR); @@ -3312,9 +3336,9 @@ gst_video_decoder_allocate_output_frame (GstVideoDecoder * goto error; } - if (G_UNLIKELY (decoder->priv->output_state_changed - || gst_pad_check_reconfigure (decoder->srcpad))) { - if (!gst_video_decoder_negotiate (decoder)) { + needs_reconfigure = gst_pad_check_reconfigure (decoder->srcpad); + if (G_UNLIKELY (decoder->priv->output_state_changed || needs_reconfigure)) { + if (!gst_video_decoder_negotiate_unlocked (decoder)) { GST_DEBUG_OBJECT (decoder, "Failed to negotiate, fallback allocation"); gst_pad_mark_reconfigure (decoder->srcpad); } diff --git a/gst-libs/gst/video/gstvideoencoder.c b/gst-libs/gst/video/gstvideoencoder.c index 32b627fc88..e211fc621f 100644 --- a/gst-libs/gst/video/gstvideoencoder.c +++ b/gst-libs/gst/video/gstvideoencoder.c @@ -232,6 +232,8 @@ static gboolean gst_video_encoder_decide_allocation_default (GstVideoEncoder * static gboolean gst_video_encoder_propose_allocation_default (GstVideoEncoder * encoder, GstQuery * query); static gboolean gst_video_encoder_negotiate_default (GstVideoEncoder * encoder); +static gboolean gst_video_encoder_negotiate_unlocked (GstVideoEncoder * + encoder); /* we can't use G_DEFINE_ABSTRACT_TYPE because we need the klass in the _init * method to get to the padtemplates */ @@ -1570,11 +1572,25 @@ no_decide_allocation: } } +static gboolean +gst_video_encoder_negotiate_unlocked (GstVideoEncoder * encoder) +{ + GstVideoEncoderClass *klass = GST_VIDEO_ENCODER_GET_CLASS (encoder); + gboolean ret = TRUE; + + if (G_LIKELY (klass->negotiate)) + ret = klass->negotiate (encoder); + + return ret; +} + /** * gst_video_encoder_negotiate: * @encoder: a #GstVideoEncoder * * Negotiate with downstream elements to currently configured #GstVideoCodecState. + * Unmark GST_PAD_FLAG_NEED_RECONFIGURE in any case. But mark it again if + * negotiate fails. * * Returns: #TRUE if the negotiation succeeded, else #FALSE. */ @@ -1590,8 +1606,12 @@ gst_video_encoder_negotiate (GstVideoEncoder * encoder) klass = GST_VIDEO_ENCODER_GET_CLASS (encoder); GST_VIDEO_ENCODER_STREAM_LOCK (encoder); - if (klass->negotiate) + gst_pad_check_reconfigure (encoder->srcpad); + if (klass->negotiate) { ret = klass->negotiate (encoder); + if (!ret) + gst_pad_mark_reconfigure (encoder->srcpad); + } GST_VIDEO_ENCODER_STREAM_UNLOCK (encoder); return ret; @@ -1611,16 +1631,17 @@ GstBuffer * gst_video_encoder_allocate_output_buffer (GstVideoEncoder * encoder, gsize size) { GstBuffer *buffer; + gboolean needs_reconfigure = FALSE; g_return_val_if_fail (size > 0, NULL); GST_DEBUG ("alloc src buffer"); GST_VIDEO_ENCODER_STREAM_LOCK (encoder); + needs_reconfigure = gst_pad_check_reconfigure (encoder->srcpad); if (G_UNLIKELY (encoder->priv->output_state_changed - || (encoder->priv->output_state - && gst_pad_check_reconfigure (encoder->srcpad)))) { - if (!gst_video_encoder_negotiate (encoder)) { + || (encoder->priv->output_state && needs_reconfigure))) { + if (!gst_video_encoder_negotiate_unlocked (encoder)) { GST_DEBUG_OBJECT (encoder, "Failed to negotiate, fallback allocation"); gst_pad_mark_reconfigure (encoder->srcpad); goto fallback; @@ -1666,13 +1687,15 @@ GstFlowReturn gst_video_encoder_allocate_output_frame (GstVideoEncoder * encoder, GstVideoCodecFrame * frame, gsize size) { + gboolean needs_reconfigure = FALSE; + g_return_val_if_fail (frame->output_buffer == NULL, GST_FLOW_ERROR); GST_VIDEO_ENCODER_STREAM_LOCK (encoder); + needs_reconfigure = gst_pad_check_reconfigure (encoder->srcpad); if (G_UNLIKELY (encoder->priv->output_state_changed - || (encoder->priv->output_state - && gst_pad_check_reconfigure (encoder->srcpad)))) { - if (!gst_video_encoder_negotiate (encoder)) { + || (encoder->priv->output_state && needs_reconfigure))) { + if (!gst_video_encoder_negotiate_unlocked (encoder)) { GST_DEBUG_OBJECT (encoder, "Failed to negotiate, fallback allocation"); gst_pad_mark_reconfigure (encoder->srcpad); } @@ -1733,6 +1756,7 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder, gboolean send_headers = FALSE; gboolean discont = (frame->presentation_frame_number == 0); GstBuffer *buffer; + gboolean needs_reconfigure = FALSE; encoder_class = GST_VIDEO_ENCODER_GET_CLASS (encoder); @@ -1745,9 +1769,10 @@ gst_video_encoder_finish_frame (GstVideoEncoder * encoder, GST_VIDEO_ENCODER_STREAM_LOCK (encoder); + needs_reconfigure = gst_pad_check_reconfigure (encoder->srcpad); if (G_UNLIKELY (priv->output_state_changed || (priv->output_state - && gst_pad_check_reconfigure (encoder->srcpad)))) { - if (!gst_video_encoder_negotiate (encoder)) { + && needs_reconfigure))) { + if (!gst_video_encoder_negotiate_unlocked (encoder)) { gst_pad_mark_reconfigure (encoder->srcpad); if (GST_PAD_IS_FLUSHING (encoder->srcpad)) ret = GST_FLOW_FLUSHING;