From e921ee132bec0b4dc38ce5ba7bb76e6a951bae84 Mon Sep 17 00:00:00 2001 From: eunhae choi Date: Wed, 20 May 2015 21:18:08 +0900 Subject: [PATCH] downloadbuffer: release lock before posting msg to avoid the deadlock in playbin2, send msg after release the download buffer lock. https://bugzilla.gnome.org/show_bug.cgi?id=749535 --- plugins/elements/gstdownloadbuffer.c | 68 ++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/plugins/elements/gstdownloadbuffer.c b/plugins/elements/gstdownloadbuffer.c index e31633fe15..3b760e2cff 100644 --- a/plugins/elements/gstdownloadbuffer.c +++ b/plugins/elements/gstdownloadbuffer.c @@ -174,7 +174,7 @@ enum G_DEFINE_TYPE_WITH_CODE (GstDownloadBuffer, gst_download_buffer, GST_TYPE_ELEMENT, _do_init); -static void update_buffering (GstDownloadBuffer * dlbuf); +static GstMessage *update_buffering (GstDownloadBuffer * dlbuf); static void gst_download_buffer_finalize (GObject * object); @@ -392,25 +392,19 @@ reset_rate_timer (GstDownloadBuffer * dlbuf) #define AVG_OUT(avg,val) ((avg) * 3.0 + (val)) / 4.0 static void -update_time_level (GstDownloadBuffer * dlbuf) +update_levels (GstDownloadBuffer * dlbuf, guint bytes) { + dlbuf->cur_level.bytes = bytes; + if (dlbuf->byte_in_rate > 0.0) { dlbuf->cur_level.time = dlbuf->cur_level.bytes / dlbuf->byte_in_rate * GST_SECOND; } + GST_DEBUG ("levels: bytes %u/%u, time %" GST_TIME_FORMAT "/%" GST_TIME_FORMAT, dlbuf->cur_level.bytes, dlbuf->max_level.bytes, GST_TIME_ARGS (dlbuf->cur_level.time), GST_TIME_ARGS (dlbuf->max_level.time)); - /* update the buffering */ - update_buffering (dlbuf); -} - -static void -update_levels (GstDownloadBuffer * dlbuf, guint bytes) -{ - dlbuf->cur_level.bytes = bytes; - update_time_level (dlbuf); } static void @@ -568,14 +562,15 @@ get_buffering_stats (GstDownloadBuffer * dlbuf, gint percent, } } -static void +static GstMessage * update_buffering (GstDownloadBuffer * dlbuf) { gint percent; gboolean post = FALSE; + GstMessage *message = NULL; if (!get_buffering_percent (dlbuf, NULL, &percent)) - return; + return NULL; if (dlbuf->is_buffering) { post = TRUE; @@ -599,7 +594,6 @@ update_buffering (GstDownloadBuffer * dlbuf) } if (post) { - GstMessage *message; GstBufferingMode mode; gint avg_in, avg_out; gint64 buffering_left; @@ -611,9 +605,9 @@ update_buffering (GstDownloadBuffer * dlbuf) (gint) percent); gst_message_set_buffering_stats (message, mode, avg_in, avg_out, buffering_left); - - gst_element_post_message (GST_ELEMENT_CAST (dlbuf), message); } + + return message; } static gboolean @@ -1053,6 +1047,8 @@ gst_download_buffer_handle_sink_event (GstPad * pad, GstObject * parent, } default: if (GST_EVENT_IS_SERIALIZED (event)) { + GstMessage *msg = NULL; + /* serialized events go in the buffer */ GST_DOWNLOAD_BUFFER_MUTEX_LOCK_CHECK (dlbuf, dlbuf->sinkresult, out_flushing); @@ -1063,6 +1059,8 @@ gst_download_buffer_handle_sink_event (GstPad * pad, GstObject * parent, * filled and we can read all data from the dlbuf. */ /* update the buffering status */ update_levels (dlbuf, dlbuf->max_level.bytes); + /* update the buffering */ + msg = update_buffering (dlbuf); /* wakeup the waiter and let it recheck */ GST_DOWNLOAD_BUFFER_SIGNAL_ADD (dlbuf, -1); break; @@ -1080,6 +1078,8 @@ gst_download_buffer_handle_sink_event (GstPad * pad, GstObject * parent, } gst_event_unref (event); GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); } else { /* non-serialized events are passed upstream. */ ret = gst_pad_push_event (dlbuf->srcpad, event); @@ -1129,6 +1129,7 @@ gst_download_buffer_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer) guint64 offset; gsize res, available; GError *error = NULL; + GstMessage *msg = NULL; dlbuf = GST_DOWNLOAD_BUFFER (parent); @@ -1210,8 +1211,14 @@ gst_download_buffer_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer) update_levels (dlbuf, 0); } + /* update the buffering */ + msg = update_buffering (dlbuf); + GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); + return GST_FLOW_OK; /* ERRORS */ @@ -1253,12 +1260,18 @@ completed: dlbuf->write_pos = dlbuf->upstream_size; dlbuf->filling = FALSE; update_levels (dlbuf, dlbuf->max_level.bytes); + msg = update_buffering (dlbuf); + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), gst_message_new_element (GST_OBJECT_CAST (dlbuf), gst_structure_new ("GstCacheDownloadComplete", "location", G_TYPE_STRING, dlbuf->temp_location, NULL))); + GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); + return GST_FLOW_EOS; } } @@ -1271,6 +1284,7 @@ gst_download_buffer_loop (GstPad * pad) GstDownloadBuffer *dlbuf; GstFlowReturn ret; GstBuffer *buffer = NULL; + GstMessage *msg = NULL; dlbuf = GST_DOWNLOAD_BUFFER (GST_PAD_PARENT (pad)); @@ -1290,9 +1304,15 @@ gst_download_buffer_loop (GstPad * pad) if (ret != GST_FLOW_OK) goto out_flushing; + /* update the buffering */ + msg = update_buffering (dlbuf); + g_atomic_int_set (&dlbuf->downstream_may_block, 1); GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); + ret = gst_pad_push (dlbuf->srcpad, buffer); g_atomic_int_set (&dlbuf->downstream_may_block, 0); @@ -1591,14 +1611,21 @@ gst_download_buffer_get_range (GstPad * pad, GstObject * parent, guint64 offset, { GstDownloadBuffer *dlbuf; GstFlowReturn ret; + GstMessage *msg = NULL; dlbuf = GST_DOWNLOAD_BUFFER_CAST (parent); GST_DOWNLOAD_BUFFER_MUTEX_LOCK_CHECK (dlbuf, dlbuf->srcresult, out_flushing); /* FIXME - function will block when the range is not yet available */ ret = gst_download_buffer_read_buffer (dlbuf, offset, length, buffer); + /* update the buffering */ + msg = update_buffering (dlbuf); + GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); + return ret; /* ERRORS */ @@ -1837,6 +1864,7 @@ gst_download_buffer_set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec) { GstDownloadBuffer *dlbuf = GST_DOWNLOAD_BUFFER (object); + GstMessage *msg = NULL; /* someone could change levels here, and since this * affects the get/put funcs, we need to lock for safety. */ @@ -1845,11 +1873,11 @@ gst_download_buffer_set_property (GObject * object, switch (prop_id) { case PROP_MAX_SIZE_BYTES: dlbuf->max_level.bytes = g_value_get_uint (value); - CAPACITY_CHANGE (dlbuf); + msg = CAPACITY_CHANGE (dlbuf); break; case PROP_MAX_SIZE_TIME: dlbuf->max_level.time = g_value_get_uint64 (value); - CAPACITY_CHANGE (dlbuf); + msg = CAPACITY_CHANGE (dlbuf); break; case PROP_LOW_PERCENT: dlbuf->low_percent = g_value_get_int (value); @@ -1869,6 +1897,10 @@ gst_download_buffer_set_property (GObject * object, } GST_DOWNLOAD_BUFFER_MUTEX_UNLOCK (dlbuf); + + if (msg != NULL) + gst_element_post_message (GST_ELEMENT_CAST (dlbuf), msg); + } static void