omx: Improve error handling and reporting
authorSebastian Dröge <sebastian.droege@collabora.co.uk>
Wed, 6 Jul 2011 08:29:54 +0000 (10:29 +0200)
committerSebastian Dröge <sebastian.droege@collabora.co.uk>
Sat, 9 Jul 2011 09:06:05 +0000 (11:06 +0200)
omx/gstomx.c
omx/gstomx.h

index fbf55ab..c1aaa58 100644 (file)
@@ -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);
-
-      /* 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);
+      /* Error events are always fatal */
+      gst_omx_component_set_last_error (comp, err);
 
-        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 ((err = gst_omx_component_get_last_error (comp)) != OMX_ErrorNone) {
+    GST_ERROR_OBJECT (comp->parent, "Component is in error state: %d", err);
+    goto done;
+  }
 
-    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_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 (port->comp->state_lock);
+    g_mutex_unlock (comp->state_lock);
     goto done;
   }
-  g_mutex_unlock (port->comp->state_lock);
+  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;
index a8405f5..a8cdfed 100644 (file)
@@ -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);