videodecoder: don't take STREAM_LOCK on upstream events

Don't try to take STREAM_LOCK on upstream events such as QOS.
Protect qos-related variables with object lock instead. Fixes
possible deadlock when shutting down in certain situations.

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

1 files changed, 28 insertions, 14 deletions

diff --git a/gst-libs/gst/video/gstvideodecoder.c b/gst-libs/gst/video/gstvideodecoder.c
index 767b73305..d33496545 100644
--- a/gst-libs/gst/video/gstvideodecoder.c
+++ b/gst-libs/gst/video/gstvideodecoder.c
@@ -387,8 +387,9 @@ struct _GstVideoDecoderPrivate
   gboolean output_state_changed;
 
   /* QoS properties */
-  gdouble proportion;
-  GstClockTime earliest_time;
+  gdouble proportion;           /* OBJECT_LOCK */
+  GstClockTime earliest_time;   /* OBJECT_LOCK */
+  GstClockTime qos_frame_duration;      /* OBJECT_LOCK */
   gboolean discont;
   /* qos messages: frames dropped/processed */
   guint dropped;
@@ -1182,7 +1183,6 @@ gst_video_decoder_src_eventfunc (GstVideoDecoder * decoder, GstEvent * event)
       gdouble proportion;
       GstClockTimeDiff diff;
       GstClockTime timestamp;
-      GstClockTime duration;
 
       gst_event_parse_qos (event, &proportion, &diff, &timestamp);
 
@@ -1190,15 +1190,7 @@ gst_video_decoder_src_eventfunc (GstVideoDecoder * decoder, GstEvent * event)
       priv->proportion = proportion;
       if (G_LIKELY (GST_CLOCK_TIME_IS_VALID (timestamp))) {
         if (G_UNLIKELY (diff > 0)) {
-          GST_VIDEO_DECODER_STREAM_LOCK (decoder);
-          if (priv->output_state != NULL && priv->output_state->info.fps_n > 0)
-            duration =
-                gst_util_uint64_scale (GST_SECOND,
-                priv->output_state->info.fps_d, priv->output_state->info.fps_n);
-          else
-            duration = 0;
-          GST_VIDEO_DECODER_STREAM_UNLOCK (decoder);
-          priv->earliest_time = timestamp + 2 * diff + duration;
+          priv->earliest_time = timestamp + 2 * diff + priv->qos_frame_duration;
         } else {
           priv->earliest_time = timestamp + diff;
         }
@@ -1549,6 +1541,11 @@ gst_video_decoder_reset (GstVideoDecoder * decoder, gboolean full)
     if (priv->output_state)
       gst_video_codec_state_unref (priv->output_state);
     priv->output_state = NULL;
+
+    GST_OBJECT_LOCK (decoder);
+    priv->qos_frame_duration = 0;
+    GST_OBJECT_UNLOCK (decoder);
+
     priv->min_latency = 0;
     priv->max_latency = 0;
   }
@@ -1588,8 +1585,10 @@ gst_video_decoder_reset (GstVideoDecoder * decoder, gboolean full)
   priv->bytes_out = 0;
   priv->time = 0;
 
+  GST_OBJECT_LOCK (decoder);
   priv->earliest_time = GST_CLOCK_TIME_NONE;
   priv->proportion = 0.5;
+  GST_OBJECT_UNLOCK (decoder);
 
   priv->reorder_idx_out = priv->reorder_idx_in = 0;
 
@@ -2162,13 +2161,16 @@ gst_video_decoder_drop_frame (GstVideoDecoder * dec, GstVideoCodecFrame * frame)
   dec->priv->dropped++;
 
   /* post QoS message */
-  timestamp = frame->pts;
+  GST_OBJECT_LOCK (dec);
   proportion = dec->priv->proportion;
+  earliest_time = dec->priv->earliest_time;
+  GST_OBJECT_UNLOCK (dec);
+
+  timestamp = frame->pts;
   segment = &dec->output_segment;
   stream_time =
       gst_segment_to_stream_time (segment, GST_FORMAT_TIME, timestamp);
   qostime = gst_segment_to_running_time (segment, GST_FORMAT_TIME, timestamp);
-  earliest_time = dec->priv->earliest_time;
   jitter = GST_CLOCK_DIFF (qostime, earliest_time);
   qos_msg =
       gst_message_new_qos (GST_OBJECT_CAST (dec), FALSE, qostime, stream_time,
@@ -2596,6 +2598,7 @@ gst_video_decoder_set_output_state (GstVideoDecoder * decoder,
 {
   GstVideoDecoderPrivate *priv = decoder->priv;
   GstVideoCodecState *state;
+  GstClockTime qos_frame_duration;
 
   GST_DEBUG_OBJECT (decoder, "fmt:%d, width:%d, height:%d, reference:%p",
       fmt, width, height, reference);
@@ -2609,9 +2612,20 @@ gst_video_decoder_set_output_state (GstVideoDecoder * decoder,
     gst_video_codec_state_unref (priv->output_state);
   priv->output_state = gst_video_codec_state_ref (state);
 
+  if (priv->output_state != NULL && priv->output_state->info.fps_n > 0) {
+    qos_frame_duration =
+        gst_util_uint64_scale (GST_SECOND, priv->output_state->info.fps_d,
+        priv->output_state->info.fps_n);
+  } else {
+    qos_frame_duration = 0;
+  }
   priv->output_state_changed = TRUE;
   GST_VIDEO_DECODER_STREAM_UNLOCK (decoder);
 
+  GST_OBJECT_LOCK (decoder);
+  priv->qos_frame_duration = qos_frame_duration;
+  GST_OBJECT_UNLOCK (decoder);
+
   return state;
 }