From 3d50c1f99ce1c9af5e4a2b8687a1c95eaebc94cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Wed, 6 Jul 2011 10:29:54 +0200 Subject: [PATCH] omx: Improve error handling and reporting --- omx/gstomx.c | 411 +++++++++++++++++++++++++++++++-------------------- omx/gstomx.h | 5 +- 2 files changed, 251 insertions(+), 165 deletions(-) diff --git a/omx/gstomx.c b/omx/gstomx.c index fbf55abe5a..c1aaa58584 100644 --- a/omx/gstomx.c +++ b/omx/gstomx.c @@ -179,7 +179,7 @@ EventHandler (OMX_HANDLETYPE hComponent, OMX_PTR pAppData, OMX_EVENTTYPE eEvent, * we can continue */ g_mutex_lock (port->port_lock); - /* If this is ever called when the port + /* FIXME: If this is ever called when the port * was not set to flushing something went * wrong but it happens for some reason. */ @@ -187,7 +187,8 @@ EventHandler (OMX_HANDLETYPE hComponent, OMX_PTR pAppData, OMX_EVENTTYPE eEvent, port->flushed = TRUE; g_cond_broadcast (port->port_cond); } else { - g_debug ("Port %u is not flushing\n", (guint32) port->index); + GST_ERROR_OBJECT (comp->parent, "Port %u was not flushing", + port->index); } g_mutex_unlock (port->port_lock); break; @@ -217,36 +218,16 @@ EventHandler (OMX_HANDLETYPE hComponent, OMX_PTR pAppData, OMX_EVENTTYPE eEvent, } case OMX_EventError: { - gint i, n; OMX_ERRORTYPE err = nData1; if (err == OMX_ErrorNone) break; - GST_ERROR_OBJECT (comp->parent, "Got error %d\n", err); + GST_ERROR_OBJECT (comp->parent, "Got error %d", err); - /* Error events are always fatal, notify all - * condition variables that something went - * wrong - */ - g_mutex_lock (comp->state_lock); - comp->last_error = err; - g_cond_broadcast (comp->state_cond); - g_mutex_unlock (comp->state_lock); + /* Error events are always fatal */ + gst_omx_component_set_last_error (comp, err); - /* Now notify all ports, no locking needed - * here because the ports are allocated in the - * very beginning and never change again until - * component destruction. - */ - n = comp->ports->len; - for (i = 0; i < n; i++) { - GstOMXPort *tmp = g_ptr_array_index (comp->ports, i); - - g_mutex_lock (tmp->port_lock); - g_cond_broadcast (tmp->port_cond); - g_mutex_unlock (tmp->port_lock); - } break; } case OMX_EventPortSettingsChanged: @@ -309,18 +290,21 @@ EmptyBufferDone (OMX_HANDLETYPE hComponent, OMX_PTR pAppData, OMX_BUFFERHEADERTYPE * pBuffer) { GstOMXBuffer *buf = pBuffer->pAppPrivate; + GstOMXPort *port = buf->port; + GstOMXComponent *comp = port->comp; g_assert (buf->omx_buf == pBuffer); /* Input buffer is empty again and can * be used to contain new input */ - g_mutex_lock (buf->port->port_lock); - GST_DEBUG_OBJECT (buf->port->comp->parent, "Port %u emptied buffer %p", - buf->port->index, buf); + g_mutex_lock (port->port_lock); + GST_DEBUG_OBJECT (comp->parent, "Port %u emptied buffer %p", + port->index, buf); buf->used = FALSE; - g_queue_push_tail (buf->port->pending_buffers, buf); - g_cond_broadcast (buf->port->port_cond); - g_mutex_unlock (buf->port->port_lock); + g_queue_push_tail (port->pending_buffers, buf); + g_cond_broadcast (port->port_cond); + g_mutex_unlock (port->port_lock); + return OMX_ErrorNone; } @@ -329,18 +313,20 @@ FillBufferDone (OMX_HANDLETYPE hComponent, OMX_PTR pAppData, OMX_BUFFERHEADERTYPE * pBuffer) { GstOMXBuffer *buf = pBuffer->pAppPrivate; + GstOMXPort *port = buf->port; + GstOMXComponent *comp = port->comp; g_assert (buf->omx_buf == pBuffer); /* Output buffer contains output now or * the port was flushed */ - g_mutex_lock (buf->port->port_lock); - GST_DEBUG_OBJECT (buf->port->comp->parent, "Port %u filled buffer %p", - buf->port->index, buf); + g_mutex_lock (port->port_lock); + GST_DEBUG_OBJECT (comp->parent, "Port %u filled buffer %p", port->index, buf); buf->used = FALSE; - g_queue_push_tail (buf->port->pending_buffers, buf); - g_cond_broadcast (buf->port->port_cond); - g_mutex_unlock (buf->port->port_lock); + g_queue_push_tail (port->pending_buffers, buf); + g_cond_broadcast (port->port_cond); + g_mutex_unlock (port->port_lock); + return OMX_ErrorNone; } @@ -435,10 +421,15 @@ gst_omx_component_set_state (GstOMXComponent * comp, OMX_STATETYPE state) old_state = comp->state; GST_DEBUG_OBJECT (comp->parent, "Setting state from %d to %d", old_state, state); - if ((err = comp->last_error) != OMX_ErrorNone) + if ((err = comp->last_error) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component in error state: %d", err); goto done; - if (old_state == state || comp->pending_state == state) + } + + if (old_state == state || comp->pending_state == state) { + GST_DEBUG_OBJECT (comp->parent, "Component already in state %d", state); goto done; + } comp->pending_state = state; err = OMX_SendCommand (comp->handle, OMX_CommandStateSet, state, NULL); @@ -446,9 +437,11 @@ gst_omx_component_set_state (GstOMXComponent * comp, OMX_STATETYPE state) done: g_mutex_unlock (comp->state_lock); - if (err != OMX_ErrorNone) + if (err != OMX_ErrorNone) { GST_ERROR_OBJECT (comp->parent, "Error setting state from %d to %d: %d", old_state, state, err); + gst_omx_component_set_last_error (comp, err); + } return err; } @@ -457,7 +450,7 @@ gst_omx_component_get_state (GstOMXComponent * comp, GstClockTime timeout) { OMX_STATETYPE ret; GTimeVal *timeval, abstimeout; - gboolean signalled; + gboolean signalled = TRUE; g_return_val_if_fail (comp != NULL, OMX_StateInvalid); @@ -469,6 +462,8 @@ gst_omx_component_get_state (GstOMXComponent * comp, GstClockTime timeout) goto done; if (comp->last_error != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component in error state: %d", + comp->last_error); ret = OMX_StateInvalid; goto done; } @@ -499,6 +494,7 @@ gst_omx_component_get_state (GstOMXComponent * comp, GstClockTime timeout) "Got error while waiting for state change: %d", comp->last_error); ret = OMX_StateInvalid; } else if (comp->pending_state == OMX_StateInvalid) { + /* State change finished and everything's fine */ ret = comp->state; } else { ret = OMX_StateInvalid; @@ -506,13 +502,16 @@ gst_omx_component_get_state (GstOMXComponent * comp, GstClockTime timeout) } } else { ret = OMX_StateInvalid; - comp->state = comp->pending_state = OMX_StateInvalid; GST_WARNING_OBJECT (comp->parent, "Timeout while waiting for state change"); } done: g_mutex_unlock (comp->state_lock); + /* If we waited and timed out this component is unusable now */ + if (!signalled) + gst_omx_component_set_last_error (comp, OMX_ErrorTimeout); + GST_DEBUG_OBJECT (comp->parent, "Returning state %d", ret); return ret; @@ -587,6 +586,45 @@ gst_omx_component_get_port (GstOMXComponent * comp, guint32 index) return NULL; } +void +gst_omx_component_set_last_error (GstOMXComponent * comp, OMX_ERRORTYPE err) +{ + gint i, n; + + g_return_if_fail (comp != NULL); + + if (err == OMX_ErrorNone) + return; + + GST_ERROR_OBJECT (comp->parent, "Setting last error: %d", err); + g_mutex_lock (comp->state_lock); + /* We only set the first error ever from which + * we can't recover anymore. + */ + if (comp->last_error == OMX_ErrorNone) + comp->last_error = err; + g_cond_broadcast (comp->state_cond); + g_mutex_unlock (comp->state_lock); + + /* Now notify all ports, no locking needed + * here because the ports are allocated in the + * very beginning and never change again until + * component destruction. + */ + n = comp->ports->len; + for (i = 0; i < n; i++) { + GstOMXPort *tmp = g_ptr_array_index (comp->ports, i); + + /* NOTE: We're not holding port->port_lock here + * to simplify the code of callers, which often + * already hold one of the port mutexes. + * Holding the mutex related to the condition + * variable is not necessary for signalling + */ + g_cond_broadcast (tmp->port_cond); + } +} + OMX_ERRORTYPE gst_omx_component_get_last_error (GstOMXComponent * comp) { @@ -607,15 +645,19 @@ void gst_omx_port_get_port_definition (GstOMXPort * port, OMX_PARAM_PORTDEFINITIONTYPE * port_def) { + GstOMXComponent *comp; + g_return_if_fail (port != NULL); + comp = port->comp; + memset (port_def, 0, sizeof (*port_def)); port_def->nSize = sizeof (*port_def); port_def->nVersion.s.nVersionMajor = 1; port_def->nVersion.s.nVersionMinor = 1; port_def->nPortIndex = port->index; - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, port_def); + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, port_def); } gboolean @@ -623,18 +665,20 @@ gst_omx_port_update_port_definition (GstOMXPort * port, OMX_PARAM_PORTDEFINITIONTYPE * port_def) { OMX_ERRORTYPE err = OMX_ErrorNone; + GstOMXComponent *comp; g_return_val_if_fail (port != NULL, FALSE); + comp = port->comp; + g_mutex_lock (port->port_lock); if (port_def) err = - OMX_SetParameter (port->comp->handle, OMX_IndexParamPortDefinition, - port_def); - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_SetParameter (comp->handle, OMX_IndexParamPortDefinition, port_def); + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); - GST_DEBUG_OBJECT (port->comp->parent, "Updated port %u definition: %d", + GST_DEBUG_OBJECT (comp->parent, "Updated port %u definition: %d", port->index, err); g_mutex_unlock (port->port_lock); @@ -645,22 +689,25 @@ gst_omx_port_update_port_definition (GstOMXPort * port, GstOMXBuffer * gst_omx_port_acquire_buffer (GstOMXPort * port) { + GstOMXComponent *comp; + OMX_ERRORTYPE err; GstOMXBuffer *buf = NULL; - GST_DEBUG_OBJECT (port->comp->parent, "Acquiring buffer from port %u", - port->index); + g_return_val_if_fail (port != NULL, NULL); + + comp = port->comp; + + GST_DEBUG_OBJECT (comp->parent, "Acquiring buffer from port %u", port->index); g_mutex_lock (port->port_lock); if (port->flushing) goto done; /* Check if the component is in an error state */ - g_mutex_lock (port->comp->state_lock); - if (port->comp->last_error != OMX_ErrorNone) { - g_mutex_unlock (port->comp->state_lock); + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); goto done; } - g_mutex_unlock (port->comp->state_lock); /* Wait until there's something in the queue * or something else happened that requires @@ -670,48 +717,60 @@ gst_omx_port_acquire_buffer (GstOMXPort * port) g_cond_wait (port->port_cond, port->port_lock); /* Check if the component is in an error state */ - g_mutex_lock (port->comp->state_lock); - if (port->comp->last_error != OMX_ErrorNone) { - g_mutex_unlock (port->comp->state_lock); + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); goto done; } - g_mutex_unlock (port->comp->state_lock); - if (!g_queue_is_empty (port->pending_buffers)) - buf = g_queue_pop_head (port->pending_buffers); + buf = g_queue_pop_head (port->pending_buffers); done: g_mutex_unlock (port->port_lock); - GST_DEBUG_OBJECT (port->comp->parent, "Acquired buffer %p from port %u", buf, + GST_DEBUG_OBJECT (comp->parent, "Acquired buffer %p from port %u", buf, port->index); return buf; } OMX_ERRORTYPE -gst_omx_port_release_buffer (GstOMXPort * port, GstOMXBuffer * buffer) +gst_omx_port_release_buffer (GstOMXPort * port, GstOMXBuffer * buf) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; - GST_DEBUG_OBJECT (port->comp->parent, "Releasing buffer %p to port %u", - buffer, port->index); + g_return_val_if_fail (port != NULL, OMX_ErrorUndefined); + g_return_val_if_fail (buf != NULL, OMX_ErrorUndefined); + g_return_val_if_fail (buf->port == port, OMX_ErrorUndefined); + + comp = port->comp; + + GST_DEBUG_OBJECT (comp->parent, "Releasing buffer %p to port %u", + buf, port->index); g_mutex_lock (port->port_lock); - if (port->flushing) + if (port->flushing) { + GST_DEBUG_OBJECT (comp->parent, "Port %u is flushing, not releasing buffer", + port->index); goto done; + } - buffer->used = TRUE; + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); + goto done; + } + + buf->used = TRUE; if (port->port_def.eDir == OMX_DirInput) { - err = OMX_EmptyThisBuffer (port->comp->handle, buffer->omx_buf); + err = OMX_EmptyThisBuffer (comp->handle, buf->omx_buf); } else { - err = OMX_FillThisBuffer (port->comp->handle, buffer->omx_buf); + err = OMX_FillThisBuffer (comp->handle, buf->omx_buf); } done: - GST_DEBUG_OBJECT (port->comp->parent, "Released buffer %p to port %u: %d", - buffer, port->index, err); + GST_DEBUG_OBJECT (comp->parent, "Released buffer %p to port %u: %d", + buf, port->index, err); g_mutex_unlock (port->port_lock); return err; @@ -720,39 +779,38 @@ done: OMX_ERRORTYPE gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; g_return_val_if_fail (port != NULL, OMX_ErrorUndefined); - GST_DEBUG_OBJECT (port->comp->parent, "Setting port %d to %sflushing", + comp = port->comp; + GST_DEBUG_OBJECT (comp->parent, "Setting port %d to %sflushing", port->index, (flush ? "" : "not ")); g_mutex_lock (port->port_lock); if (! !flush == ! !port->flushing) { - GST_DEBUG_OBJECT (port->comp->parent, "Port %u was %sflushing already", + GST_DEBUG_OBJECT (comp->parent, "Port %u was %sflushing already", port->index, (flush ? "" : "not ")); goto done; } - g_mutex_lock (port->comp->state_lock); - if ((port->comp->state != OMX_StateIdle - && port->comp->state != OMX_StateExecuting) - || port->comp->last_error != OMX_ErrorNone) { - - if (port->comp->last_error != OMX_ErrorNone) { - err = port->comp->last_error; - GST_ERROR_OBJECT (port->comp->parent, "Component is in error state: %d", - err); - } else { - GST_ERROR_OBJECT (port->comp->parent, "Component is in wrong state: %d", - port->comp->state); - err = OMX_ErrorUndefined; - } - - g_mutex_unlock (port->comp->state_lock); + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err); goto done; } - g_mutex_unlock (port->comp->state_lock); + + g_mutex_lock (comp->state_lock); + if (comp->state != OMX_StateIdle && comp->state != OMX_StateExecuting) { + + GST_ERROR_OBJECT (comp->parent, "Component is in wrong state: %d", + comp->state); + err = OMX_ErrorUndefined; + + g_mutex_unlock (comp->state_lock); + goto done; + } + g_mutex_unlock (comp->state_lock); port->flushing = flush; if (flush) @@ -764,11 +822,9 @@ gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush) OMX_ERRORTYPE last_error; port->flushed = FALSE; - err = - OMX_SendCommand (port->comp->handle, OMX_CommandFlush, port->index, - NULL); + err = OMX_SendCommand (comp->handle, OMX_CommandFlush, port->index, NULL); if (err != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Error sending flush command to port %u: %d", port->index, err); goto done; } @@ -776,7 +832,7 @@ gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush) g_get_current_time (&abstimeout); g_time_val_add (&abstimeout, 5 * 10000000); timeval = &abstimeout; - GST_DEBUG_OBJECT (port->comp->parent, "Waiting for 5s"); + GST_DEBUG_OBJECT (comp->parent, "Waiting for 5s"); /* Retry until timeout or until an error happend or * until all buffers were released by the component and @@ -784,71 +840,66 @@ gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush) do { signalled = g_cond_timed_wait (port->port_cond, port->port_lock, timeval); - g_mutex_lock (port->comp->state_lock); - last_error = port->comp->last_error; - g_mutex_unlock (port->comp->state_lock); + last_error = gst_omx_component_get_last_error (comp); } while (signalled && last_error == OMX_ErrorNone && !port->flushed && port->buffers->len != g_queue_get_length (port->pending_buffers)); port->flushed = FALSE; - GST_DEBUG_OBJECT (port->comp->parent, "Port %d flushed", port->index); + GST_DEBUG_OBJECT (comp->parent, "Port %d flushed", port->index); if (last_error != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Got error while flushing port %u: %d", port->index, last_error); err = last_error; goto done; } else if (!signalled) { - GST_ERROR_OBJECT (port->comp->parent, "Timeout while flushing port %u", + GST_ERROR_OBJECT (comp->parent, "Timeout while flushing port %u", port->index); err = OMX_ErrorTimeout; goto done; } } else { if (port->port_def.eDir == OMX_DirOutput && port->buffers) { - gint i, n; + GstOMXBuffer *buf; /* Enqueue all buffers for the component to fill */ - n = port->buffers->len; - for (i = 0; i < n; i++) { - GstOMXBuffer *buf = g_ptr_array_index (port->buffers, i); - + while ((buf = g_queue_pop_head (port->pending_buffers))) { g_assert (!buf->used); - err = OMX_FillThisBuffer (port->comp->handle, buf->omx_buf); + err = OMX_FillThisBuffer (comp->handle, buf->omx_buf); if (err != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Failed to pass buffer %p to port %u: %d", buf, port->index, err); - g_mutex_lock (port->comp->state_lock); - port->comp->last_error = err; - g_mutex_unlock (port->comp->state_lock); + gst_omx_component_set_last_error (comp, err); goto done; } } - - g_queue_clear (port->pending_buffers); } } done: - GST_DEBUG_OBJECT (port->comp->parent, "Set port %u to %sflushing: %d", + GST_DEBUG_OBJECT (comp->parent, "Set port %u to %sflushing: %d", port->index, (flush ? "" : "not "), err); g_mutex_unlock (port->port_lock); + return err; } gboolean gst_omx_port_is_flushing (GstOMXPort * port) { + GstOMXComponent *comp; gboolean flushing; g_return_val_if_fail (port != NULL, FALSE); + comp = port->comp; + g_mutex_lock (port->port_lock); flushing = port->flushing; g_mutex_unlock (port->port_lock); - GST_DEBUG_OBJECT (port->comp->parent, "Port %u is flushing: %d", port->index, + GST_DEBUG_OBJECT (comp->parent, "Port %u is flushing: %d", port->index, flushing); return flushing; @@ -858,16 +909,24 @@ gst_omx_port_is_flushing (GstOMXPort * port) static OMX_ERRORTYPE gst_omx_port_allocate_buffers_unlocked (GstOMXPort * port) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; gint i, n; g_assert (!port->buffers || port->buffers->len == 0); + comp = port->comp; + + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component in error state: %d", err); + goto done; + } + /* Update the port definition to check if we need more * buffers after the port configuration was done and to * update the buffer size */ - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); /* If the configured, actual number of buffers is less than @@ -876,14 +935,22 @@ gst_omx_port_allocate_buffers_unlocked (GstOMXPort * port) */ if (port->port_def.nBufferCountActual < port->port_def.nBufferCountMin) { port->port_def.nBufferCountActual = port->port_def.nBufferCountMin; - OMX_SetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + err = OMX_SetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); } + if (err != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, + "Failed to configure number of buffers of port %u: %d", port->index, + err); + gst_omx_component_set_last_error (comp, err); + goto done; + } + n = port->port_def.nBufferCountActual; - GST_DEBUG_OBJECT (port->comp->parent, + GST_DEBUG_OBJECT (comp->parent, "Allocating %d buffers of size %u for port %u", n, port->port_def.nBufferSize, port->index); @@ -899,12 +966,12 @@ gst_omx_port_allocate_buffers_unlocked (GstOMXPort * port) g_ptr_array_add (port->buffers, buf); err = - OMX_AllocateBuffer (port->comp->handle, &buf->omx_buf, port->index, buf, + OMX_AllocateBuffer (comp->handle, &buf->omx_buf, port->index, buf, port->port_def.nBufferSize); if (err != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, "Failed to allocate buffer: %d", - err); - port->comp->last_error = err; + GST_ERROR_OBJECT (comp->parent, + "Failed to allocate buffer for port %u: %d", port->index, err); + gst_omx_component_set_last_error (comp, err); break; } @@ -913,7 +980,8 @@ gst_omx_port_allocate_buffers_unlocked (GstOMXPort * port) } g_cond_broadcast (port->port_cond); - GST_DEBUG_OBJECT (port->comp->parent, "Allocated buffers for port %u: %d", +done: + GST_DEBUG_OBJECT (comp->parent, "Allocated buffers for port %u: %d", port->index, err); return err; @@ -937,18 +1005,26 @@ gst_omx_port_allocate_buffers (GstOMXPort * port) static OMX_ERRORTYPE gst_omx_port_deallocate_buffers_unlocked (GstOMXPort * port) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; gint i, n; - GST_DEBUG_OBJECT (port->comp->parent, "Deallocating buffers of port %u", + comp = port->comp; + + GST_DEBUG_OBJECT (comp->parent, "Deallocating buffers of port %u", port->index); if (!port->buffers) { - GST_DEBUG_OBJECT (port->comp->parent, "No buffers allocated for port %u", + GST_DEBUG_OBJECT (comp->parent, "No buffers allocated for port %u", port->index); goto done; } + if ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, "Component in error state: %d", err); + goto done; + } + /* We only allow deallocation of buffers after they * were all released from the port, either by flushing * the port or by disabling it. @@ -956,7 +1032,6 @@ gst_omx_port_deallocate_buffers_unlocked (GstOMXPort * port) g_assert (g_queue_get_length (port->pending_buffers) == port->buffers->len); n = port->buffers->len; - for (i = 0; i < n; i++) { GstOMXBuffer *buf = g_ptr_array_index (port->buffers, i); OMX_ERRORTYPE tmp = OMX_ErrorNone; @@ -970,10 +1045,14 @@ gst_omx_port_deallocate_buffers_unlocked (GstOMXPort * port) * to deallocate as much as possible. */ if (buf->omx_buf) { - tmp = OMX_FreeBuffer (port->comp->handle, port->index, buf->omx_buf); - if (tmp != OMX_ErrorNone && err == OMX_ErrorNone) - err = tmp; - + tmp = OMX_FreeBuffer (comp->handle, port->index, buf->omx_buf); + if (tmp != OMX_ErrorNone) { + GST_ERROR_OBJECT (comp->parent, + "Failed to deallocate buffer %d of port %u: %d", i, port->index, + tmp); + if (err == OMX_ErrorNone) + err = tmp; + } } g_slice_free (GstOMXBuffer, buf); } @@ -981,8 +1060,9 @@ gst_omx_port_deallocate_buffers_unlocked (GstOMXPort * port) g_queue_clear (port->pending_buffers); g_ptr_array_unref (port->buffers); port->buffers = NULL; + done: - GST_DEBUG_OBJECT (port->comp->parent, "Deallocated buffers of port %u: %d", + GST_DEBUG_OBJECT (comp->parent, "Deallocated buffers of port %u: %d", port->index, err); return err; @@ -1006,39 +1086,44 @@ gst_omx_port_deallocate_buffers (GstOMXPort * port) static OMX_ERRORTYPE gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; GTimeVal abstimeout, *timeval; gboolean signalled; OMX_ERRORTYPE last_error; - GST_DEBUG_OBJECT (port->comp->parent, "Setting port %u to %s", port->index, + comp = port->comp; + + GST_DEBUG_OBJECT (comp->parent, "Setting port %u to %s", port->index, (enabled ? "enabled" : "disabled")); /* Check if the port is already enabled/disabled first */ - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); if (! !port->port_def.bEnabled == ! !enabled) goto done; if (enabled) err = - OMX_SendCommand (port->comp->handle, OMX_CommandPortEnable, port->index, + OMX_SendCommand (comp->handle, OMX_CommandPortEnable, port->index, NULL); else err = - OMX_SendCommand (port->comp->handle, OMX_CommandPortDisable, + OMX_SendCommand (comp->handle, OMX_CommandPortDisable, port->index, NULL); + if (err != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Failed to send enable/disable command to port %u: %d", port->index, err); + gst_omx_component_set_last_error (comp, err); goto done; } g_get_current_time (&abstimeout); g_time_val_add (&abstimeout, 5 * 10000000); timeval = &abstimeout; - GST_DEBUG_OBJECT (port->comp->parent, "Waiting for 5s"); + GST_DEBUG_OBJECT (comp->parent, "Waiting for 5s"); /* FIXME XXX: The spec says that bEnabled should be set *immediately* * but bellagio sets bEnabled after all buffers are allocated/deallocated @@ -1051,20 +1136,19 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled) && port->buffers->len != g_queue_get_length (port->pending_buffers))) { signalled = g_cond_timed_wait (port->port_cond, port->port_lock, timeval); - g_mutex_lock (port->comp->state_lock); - last_error = port->comp->last_error; - g_mutex_unlock (port->comp->state_lock); + last_error = gst_omx_component_get_last_error (comp); } if (last_error != OMX_ErrorNone) { err = last_error; - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Got error while waiting for port %u to release all buffers: %d", port->index, err); } else if (!signalled) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Timeout waiting for port %u to release all buffers", port->index); err = OMX_ErrorTimeout; + gst_omx_component_set_last_error (comp, err); } /* Allocate/deallocate all buffers for the port to finish @@ -1072,20 +1156,14 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled) if (enabled) { /* If allocation fails this component can't really be used anymore */ if ((err = gst_omx_port_allocate_buffers_unlocked (port)) != OMX_ErrorNone) { - g_mutex_lock (port->comp->state_lock); - port->comp->last_error = err; - g_cond_broadcast (port->comp->state_cond); - g_mutex_unlock (port->comp->state_lock); + gst_omx_component_set_last_error (comp, err); goto done; } } else { /* If deallocation fails this component can't really be used anymore */ if ((err = gst_omx_port_deallocate_buffers_unlocked (port)) != OMX_ErrorNone) { - g_mutex_lock (port->comp->state_lock); - port->comp->last_error = err; - g_cond_broadcast (port->comp->state_cond); - g_mutex_unlock (port->comp->state_lock); + gst_omx_component_set_last_error (comp, err); goto done; } } @@ -1093,31 +1171,30 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled) /* And now wait until the enable/disable command is finished */ signalled = TRUE; last_error = OMX_ErrorNone; - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); while (signalled && last_error == OMX_ErrorNone && (! !port->port_def.bEnabled != ! !enabled)) { signalled = g_cond_timed_wait (port->port_cond, port->port_lock, timeval); - g_mutex_lock (port->comp->state_lock); - last_error = port->comp->last_error; - g_mutex_unlock (port->comp->state_lock); - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + last_error = gst_omx_component_get_last_error (comp); + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); } if (!signalled) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Timeout waiting for port %u to be enabled/disabled", port->index); err = OMX_ErrorTimeout; + gst_omx_component_set_last_error (comp, err); } else if (last_error != OMX_ErrorNone) { - GST_ERROR_OBJECT (port->comp->parent, + GST_ERROR_OBJECT (comp->parent, "Got error while waiting for port %u to be enabled/disabled: %d", port->index, err); err = last_error; } done: - GST_DEBUG_OBJECT (port->comp->parent, "Port %u is %s%s: %d", port->index, + GST_DEBUG_OBJECT (comp->parent, "Port %u is %s%s: %d", port->index, (err == OMX_ErrorNone ? "" : "not "), (enabled ? "enabled" : "disabled"), err); @@ -1141,17 +1218,20 @@ gst_omx_port_set_enabled (GstOMXPort * port, gboolean enabled) gboolean gst_omx_port_is_enabled (GstOMXPort * port) { + GstOMXComponent *comp; gboolean enabled; g_return_val_if_fail (port != NULL, FALSE); + comp = port->comp; + g_mutex_lock (port->port_lock); - OMX_GetParameter (port->comp->handle, OMX_IndexParamPortDefinition, + OMX_GetParameter (comp->handle, OMX_IndexParamPortDefinition, &port->port_def); enabled = port->port_def.bEnabled; g_mutex_unlock (port->port_lock); - GST_DEBUG_OBJECT (port->comp->parent, "Port %u is enabled: %d", port->index, + GST_DEBUG_OBJECT (comp->parent, "Port %u is enabled: %d", port->index, enabled); return enabled; @@ -1160,15 +1240,18 @@ gst_omx_port_is_enabled (GstOMXPort * port) gboolean gst_omx_port_is_settings_changed (GstOMXPort * port) { + GstOMXComponent *comp; gboolean settings_changed; g_return_val_if_fail (port != NULL, FALSE); + comp = port->comp; + g_mutex_lock (port->port_lock); settings_changed = port->settings_changed; g_mutex_unlock (port->port_lock); - GST_DEBUG_OBJECT (port->comp->parent, "Port %u has settings-changed: %d", + GST_DEBUG_OBJECT (comp->parent, "Port %u has settings-changed: %d", port->index, settings_changed); return settings_changed; @@ -1177,11 +1260,14 @@ gst_omx_port_is_settings_changed (GstOMXPort * port) OMX_ERRORTYPE gst_omx_port_reconfigure (GstOMXPort * port) { + GstOMXComponent *comp; OMX_ERRORTYPE err = OMX_ErrorNone; g_return_val_if_fail (port != NULL, OMX_ErrorUndefined); - GST_DEBUG_OBJECT (port->comp->parent, "Reconfiguring port %u", port->index); + comp = port->comp; + + GST_DEBUG_OBJECT (comp->parent, "Reconfiguring port %u", port->index); g_mutex_lock (port->port_lock); @@ -1202,8 +1288,7 @@ gst_omx_port_reconfigure (GstOMXPort * port) port->settings_changed = FALSE; done: - GST_DEBUG_OBJECT (port->comp->parent, "Reconfigured port %u: %d", port->index, - err); + GST_DEBUG_OBJECT (comp->parent, "Reconfigured port %u: %d", port->index, err); g_mutex_unlock (port->port_lock); return err; diff --git a/omx/gstomx.h b/omx/gstomx.h index a8405f5daa..a8cdfed801 100644 --- a/omx/gstomx.h +++ b/omx/gstomx.h @@ -54,7 +54,7 @@ struct _GstOMXCore { struct _GstOMXPort { GstOMXComponent *comp; - OMX_U32 index; + guint32 index; /* Protects port_def, buffers, pending_buffers, * settings_changed, flushing and flushed. @@ -121,6 +121,7 @@ void gst_omx_component_free (GstOMXComponent * comp); OMX_ERRORTYPE gst_omx_component_set_state (GstOMXComponent * comp, OMX_STATETYPE state); OMX_STATETYPE gst_omx_component_get_state (GstOMXComponent * comp, GstClockTime timeout); +void gst_omx_component_set_last_error (GstOMXComponent * comp, OMX_ERRORTYPE err); OMX_ERRORTYPE gst_omx_component_get_last_error (GstOMXComponent * comp); GstOMXPort * gst_omx_component_add_port (GstOMXComponent * comp, guint32 index); @@ -131,7 +132,7 @@ void gst_omx_port_get_port_definition (GstOMXPort * port, OMX_PARAM gboolean gst_omx_port_update_port_definition (GstOMXPort *port, OMX_PARAM_PORTDEFINITIONTYPE *port_definition); GstOMXBuffer * gst_omx_port_acquire_buffer (GstOMXPort *port); -OMX_ERRORTYPE gst_omx_port_release_buffer (GstOMXPort *port, GstOMXBuffer *buffer); +OMX_ERRORTYPE gst_omx_port_release_buffer (GstOMXPort *port, GstOMXBuffer *buf); OMX_ERRORTYPE gst_omx_port_set_flushing (GstOMXPort *port, gboolean flush); gboolean gst_omx_port_is_flushing (GstOMXPort *port);