summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGeorge Kiagiadakis <george.kiagiadakis@collabora.com>2014-12-19 11:19:55 +0200
committerSebastian Dröge <sebastian@centricular.com>2015-03-04 09:51:03 +0100
commit0ef1e90b34082b88fdef3fd53dfe25fbf9002209 (patch)
tree3a08b15334581f79e2dae3bbd15a9612c0f58c75
parentdf58d8baac0a647cac33e3e273c8c1f9c3ce2274 (diff)
downloadgst-omx-0ef1e90b34082b88fdef3fd53dfe25fbf9002209.tar.gz
omx: call handle_messages() only once in acquire_buffer() to avoid potential deadlock
There is one rare case where calling handle_messages() more than once can cause a deadlock in the video decoder element: - sink pad thread starts the src pad task (gst_omx_video_dec_loop()) - _video_dec_loop() calls gst_omx_port_acquire_buffer() on dec_out_port - blocks in gst_omx_component_wait_message() releasing comp->lock and comp->messages_lock (initially, there are no buffers configured on that port, so it waits for OMX_EventPortSettingsChanged) - the sink pad thread pushes a buffer to the decoder with gst_omx_port_release_buffer() - _release_buffer() grabs comp->lock and sends the buffer to OMX, which consumes it immediately - EmptyBufferDone gets called at this point, which signals _wait_message() to unblock - the message from EmptyBufferDone is processed in gst_omx_component_handle_messages() called from gst_omx_port_release_buffer() - gst_omx_port_release_buffer releases comp->lock - the src pad thread now gets to run, grabbing comp->lock while it exits from _wait_message() - _acquire_buffer() calls the _handle_messages() on the next line after _wait_message(), which does nothing (no pending messages) - then it goes to "retry:" and calls _handle_messages() again, which also does nothing (still no pending messages) - scheduler switches to a videocore thread that calls EventHandler, informing us about the OMX_EventPortSettingsChanged event that just arrived - EventHandler graps comp->messages_lock, but not comp->lock, so it can run in parallel at this point just fine. - scheduler switches back to the src pad thread (which is in the middle of _acquire_buffer()) - the next _handle_messages() which is right before if (g_queue_is_empty (&port->pending_buffers)) processes the OMX_EventPortSettingsChanged - the buffer queue is still empty, so that thread blocks again in _wait_message() - the sink pad thread tries to acquire the next input port buffer - _acquire_buffer() also blocks this thread in: if (comp->pending_reconfigure_outports) { ... _wait_message() ... } - DEADLOCK. gstreamer is waiting for omx to do something, omx waits for gstreamer to do something. By removing those extra _handle_messages() calls, we can ensure that all the checks of _acquire_buffer() will re-run. In the above case, after the scheduler switches back to the middle of _acquire_buffer(), the code will enter _wait_message(), which will see that there are pending messages and will return immediately, going back to "retry:" and re-doing all the checks properly. https://bugzilla.gnome.org/show_bug.cgi?id=741854
-rw-r--r--omx/gstomx.c2
1 files changed, 0 insertions, 2 deletions
diff --git a/omx/gstomx.c b/omx/gstomx.c
index 2108acf..e439a0e 100644
--- a/omx/gstomx.c
+++ b/omx/gstomx.c
@@ -1326,12 +1326,10 @@ retry:
* arrives, an error happens, the port is flushing
* or the port needs to be reconfigured.
*/
- gst_omx_component_handle_messages (comp);
if (g_queue_is_empty (&port->pending_buffers)) {
GST_DEBUG_OBJECT (comp->parent, "Queue of %s port %u is empty",
comp->name, port->index);
gst_omx_component_wait_message (comp, GST_CLOCK_TIME_NONE);
- gst_omx_component_handle_messages (comp);
/* And now check everything again and maybe get a buffer */
goto retry;