omx: Split enabling/disabling of port into sending the command and waiting for it

This allows to do anything necessary after sending the command to actually let it finish

omx/gstomx.c

@@ -258,7 +258,10 @@ gst_omx_component_handle_messages (GstOMXComponent * comp)
         GST_DEBUG_OBJECT (comp->parent, "Port %u %s", port->index,
             (enable ? "enabled" : "disabled"));
 
-        port->enabled_changed = TRUE;
+        if (enable)
+          port->enabled_pending = FALSE;
+        else
+          port->disabled_pending = FALSE;
         break;
       }
       case GST_OMX_MESSAGE_PORT_SETTINGS_CHANGED:{
@@ -849,7 +852,8 @@ gst_omx_component_add_port (GstOMXComponent * comp, guint32 index)
   port->flushing = TRUE;
   port->flushed = FALSE;
   port->settings_changed = FALSE;
-  port->enabled_changed = FALSE;
+  port->enabled_pending = FALSE;
+  port->disabled_pending = FALSE;
 
   if (port->port_def.eDir == OMX_DirInput)
     comp->n_in_ports++;
@@ -1690,6 +1694,12 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
     goto done;
   }
 
+  if (port->enabled_pending || port->disabled_pending) {
+    GST_ERROR_OBJECT (comp->parent, "Port enabled/disabled pending already");
+    err = OMX_ErrorInvalidState;
+    goto done;
+  }
+
   GST_DEBUG_OBJECT (comp->parent, "Setting port %u to %s", port->index,
       (enabled ? "enabled" : "disabled"));
 
@@ -1699,7 +1709,10 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
   if (! !port->port_def.bEnabled == ! !enabled)
     goto done;
 
-  port->enabled_changed = FALSE;
+  if (enabled)
+    port->enabled_pending = TRUE;
+  else
+    port->disabled_pending = TRUE;
 
   if (!enabled) {
     /* This is also like flushing, i.e. all buffers are returned
@@ -1730,13 +1743,8 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
     goto done;
   }
 
-  if (port->enabled_changed) {
-    GST_ERROR_OBJECT (comp->parent, "Port enabled/disabled in the meantime");
-    goto done;
-  }
-
   wait_until = g_get_monotonic_time () + 5 * G_TIME_SPAN_SECOND;
-  GST_DEBUG_OBJECT (comp->parent, "Waiting for 5s");
+  GST_DEBUG_OBJECT (comp->parent, "Waiting 5s for buffers to be released");
 
   /* First wait until all buffers are released by the port */
   signalled = TRUE;
@@ -1747,12 +1755,13 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
           g_queue_get_length (&port->pending_buffers))) {
     g_mutex_lock (&comp->messages_lock);
     g_mutex_unlock (&comp->lock);
-    if (!g_queue_is_empty (&comp->messages))
+    if (!g_queue_is_empty (&comp->messages)) {
       signalled = TRUE;
-    else
+    } else {
       signalled =
           g_cond_wait_until (&comp->messages_cond, &comp->messages_lock,
           wait_until);
+    }
     g_mutex_unlock (&comp->messages_lock);
     g_mutex_lock (&comp->lock);
     if (signalled)
@@ -1765,7 +1774,7 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
     GST_ERROR_OBJECT (comp->parent,
         "Got error while waiting for port %u to release all buffers: %s (0x%08x)",
         port->index, gst_omx_error_to_string (err), err);
-    goto done;
+    goto error;
   } else if (!signalled) {
     GST_ERROR_OBJECT (comp->parent,
         "Timeout waiting for port %u to release all buffers", port->index);
@@ -1773,19 +1782,89 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
     goto error;
   }
 
-  /* Allocate/deallocate all buffers for the port to finish
+done:
-   * the enable/disable command */
+  gst_omx_component_handle_messages (comp);
-  if (enabled) {
+
-    /* If allocation fails this component can't really be used anymore */
+  gst_omx_port_update_port_definition (port, NULL);
-    if ((err = gst_omx_port_allocate_buffers_unlocked (port)) != OMX_ErrorNone) {
+
-      goto error;
+  GST_DEBUG_OBJECT (comp->parent, "Set port %u to %s%s: %s (0x%08x)",
+      port->index, (err == OMX_ErrorNone ? "" : "not "),
+      (enabled ? "enabled" : "disabled"), gst_omx_error_to_string (err), err);
+
+  return err;
+
+error:
+  {
+    g_mutex_unlock (&comp->lock);
+    gst_omx_component_set_last_error (comp, err);
+    g_mutex_lock (&comp->lock);
+    goto done;
+  }
+}
+
+/* NOTE: Uses comp->lock and comp->messages_lock */
+OMX_ERRORTYPE
+gst_omx_port_set_enabled (GstOMXPort * port, gboolean enabled)
+{
+  OMX_ERRORTYPE err;
+
+  g_return_val_if_fail (port != NULL, OMX_ErrorUndefined);
+
+  g_mutex_lock (&port->comp->lock);
+  err = gst_omx_port_set_enabled_unlocked (port, enabled);
+  g_mutex_unlock (&port->comp->lock);
+
+  return err;
+}
+
+/* NOTE: Must be called while holding comp->lock, uses comp->messages_lock */
+static OMX_ERRORTYPE
+gst_omx_port_wait_enabled_unlocked (GstOMXPort * port, GstClockTime timeout)
+{
+  GstOMXComponent *comp;
+  OMX_ERRORTYPE err = OMX_ErrorNone;
+  gint64 wait_until = -1;
+  gboolean signalled;
+  OMX_ERRORTYPE last_error;
+  gboolean enabled;
+
+  comp = port->comp;
+
+  /* Check the current port status */
+  gst_omx_component_get_parameter (comp, OMX_IndexParamPortDefinition,
+      &port->port_def);
+
+  if (port->enabled_pending)
+    enabled = TRUE;
+  else if (port->disabled_pending)
+    enabled = FALSE;
+  else
+    enabled = port->port_def.bEnabled;
+
+  gst_omx_component_handle_messages (comp);
+
+  if ((err = comp->last_error) != OMX_ErrorNone) {
+    GST_ERROR_OBJECT (comp->parent, "Component in error state: %s (0x%08x)",
+        gst_omx_error_to_string (err), err);
+    goto done;
+  }
+
+  GST_DEBUG_OBJECT (comp->parent, "Waiting for port %u to be %s", port->index,
+      (enabled ? "enabled" : "disabled"));
+
+  if (timeout != GST_CLOCK_TIME_NONE) {
+    gint64 add = timeout / (GST_SECOND / G_TIME_SPAN_SECOND);
+
+    if (add == 0) {
+      if (port->enabled_pending || port->disabled_pending)
+        err = OMX_ErrorTimeout;
+      goto done;
     }
+
+    wait_until = g_get_monotonic_time () + add;
+    GST_DEBUG_OBJECT (comp->parent, "Waiting for %" G_GINT64_FORMAT "us", add);
   } else {
-    /* If deallocation fails this component can't really be used anymore */
+    GST_DEBUG_OBJECT (comp->parent, "Waiting for signal");
-    if ((err =
-            gst_omx_port_deallocate_buffers_unlocked (port)) != OMX_ErrorNone) {
-      goto error;
-    }
   }
 
   /* And now wait until the enable/disable command is finished */
@@ -1794,16 +1873,21 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
   gst_omx_component_get_parameter (comp, OMX_IndexParamPortDefinition,
       &port->port_def);
   gst_omx_component_handle_messages (comp);
-  while (signalled && last_error == OMX_ErrorNone
+  while (signalled && last_error == OMX_ErrorNone &&
-      && (! !port->port_def.bEnabled != ! !enabled || !port->enabled_changed)) {
+      (! !port->port_def.bEnabled != ! !enabled || port->enabled_pending
+          || port->disabled_pending)) {
     g_mutex_lock (&comp->messages_lock);
     g_mutex_unlock (&comp->lock);
-    if (!g_queue_is_empty (&comp->messages))
+    if (!g_queue_is_empty (&comp->messages)) {
       signalled = TRUE;
-    else
+    } else if (timeout != GST_CLOCK_TIME_NONE) {
       signalled =
           g_cond_wait_until (&comp->messages_cond, &comp->messages_lock,
           wait_until);
+    } else {
+      signalled = TRUE;
+      g_cond_wait (&comp->messages_cond, &comp->messages_lock);
+    }
     g_mutex_unlock (&comp->messages_lock);
     g_mutex_lock (&comp->lock);
     if (signalled)
@@ -1812,7 +1896,8 @@ gst_omx_port_set_enabled_unlocked (GstOMXPort * port, gboolean enabled)
     gst_omx_component_get_parameter (comp, OMX_IndexParamPortDefinition,
         &port->port_def);
   }
-  port->enabled_changed = FALSE;
+  port->enabled_pending = FALSE;
+  port->disabled_pending = FALSE;
 
   if (!signalled) {
     GST_ERROR_OBJECT (comp->parent,
@@ -1886,14 +1971,14 @@ error:
 
 /* NOTE: Uses comp->lock and comp->messages_lock */
 OMX_ERRORTYPE
-gst_omx_port_set_enabled (GstOMXPort * port, gboolean enabled)
+gst_omx_port_wait_enabled (GstOMXPort * port, GstClockTime timeout)
 {
   OMX_ERRORTYPE err;
 
   g_return_val_if_fail (port != NULL, OMX_ErrorUndefined);
 
   g_mutex_lock (&port->comp->lock);
-  err = gst_omx_port_set_enabled_unlocked (port, enabled);
+  err = gst_omx_port_wait_enabled_unlocked (port, timeout);
   g_mutex_unlock (&port->comp->lock);
 
   return err;
@@ -1948,10 +2033,26 @@ gst_omx_port_reconfigure (GstOMXPort * port)
   if (err != OMX_ErrorNone)
     goto done;
 
+  err = gst_omx_port_deallocate_buffers_unlocked (port);
+  if (err != OMX_ErrorNone)
+    goto done;
+
+  err = gst_omx_port_wait_enabled_unlocked (port, 5 * GST_SECOND);
+  if (err != OMX_ErrorNone)
+    goto done;
+
   err = gst_omx_port_set_enabled_unlocked (port, TRUE);
   if (err != OMX_ErrorNone)
     goto done;
 
+  err = gst_omx_port_allocate_buffers_unlocked (port);
+  if (err != OMX_ErrorNone)
+    goto done;
+
+  err = gst_omx_port_wait_enabled_unlocked (port, 5 * GST_SECOND);
+  if (err != OMX_ErrorNone)
+    goto done;
+
   port->configured_settings_cookie = port->settings_cookie;
 
   /* If this is an output port, notify all input ports

omx/gstomx.h

@@ -192,7 +192,8 @@ struct _GstOMXPort {
   gboolean settings_changed;
   gboolean flushing;
   gboolean flushed; /* TRUE after OMX_CommandFlush was done */
-  gboolean enabled_changed; /* TRUE after OMX_Command{En,Dis}able was done */
+  gboolean enabled_pending;  /* TRUE after OMX_Command{En,Dis}able */
+  gboolean disabled_pending; /* was done until it took effect */
 
   /* Increased whenever the settings of these port change.
    * If settings_cookie != configured_settings_cookie
@@ -304,6 +305,7 @@ OMX_ERRORTYPE     gst_omx_port_deallocate_buffers (GstOMXPort *port);
 OMX_ERRORTYPE     gst_omx_port_reconfigure (GstOMXPort * port);
 
 OMX_ERRORTYPE     gst_omx_port_set_enabled (GstOMXPort * port, gboolean enabled);
+OMX_ERRORTYPE     gst_omx_port_wait_enabled (GstOMXPort * port, GstClockTime timeout);
 gboolean          gst_omx_port_is_enabled (GstOMXPort * port);
 
 OMX_ERRORTYPE     gst_omx_port_manual_reconfigure (GstOMXPort * port, gboolean start);

omx/gstomxaudioenc.c

@@ -576,6 +576,11 @@ gst_omx_audio_enc_set_format (GstAudioEncoder * encoder, GstAudioInfo * info)
       return FALSE;
     if (gst_omx_port_set_enabled (self->in_port, FALSE) != OMX_ErrorNone)
       return FALSE;
+    if (gst_omx_port_deallocate_buffers (self->in_port) != OMX_ErrorNone)
+      return FALSE;
+    if (gst_omx_port_wait_enabled (self->in_port,
+            5 * GST_SECOND) != OMX_ErrorNone)
+      return FALSE;
 
     GST_DEBUG_OBJECT (self, "Encoder drained and disabled");
   }
@@ -663,6 +668,11 @@ gst_omx_audio_enc_set_format (GstAudioEncoder * encoder, GstAudioInfo * info)
   if (needs_disable) {
     if (gst_omx_port_set_enabled (self->in_port, TRUE) != OMX_ErrorNone)
       return FALSE;
+    if (gst_omx_port_allocate_buffers (self->in_port) != OMX_ErrorNone)
+      return FALSE;
+    if (gst_omx_port_wait_enabled (self->in_port,
+            5 * GST_SECOND) != OMX_ErrorNone)
+      return FALSE;
     if (gst_omx_port_manual_reconfigure (self->in_port, FALSE) != OMX_ErrorNone)
       return FALSE;
   } else {

omx/gstomxvideodec.c

@@ -1063,6 +1063,11 @@ gst_omx_video_dec_set_format (GstVideoDecoder * decoder,
 
       if (gst_omx_port_set_enabled (self->in_port, FALSE) != OMX_ErrorNone)
         return FALSE;
+      if (gst_omx_port_deallocate_buffers (self->in_port) != OMX_ErrorNone)
+        return FALSE;
+      if (gst_omx_port_wait_enabled (self->in_port,
+              5 * GST_SECOND) != OMX_ErrorNone)
+        return FALSE;
     }
     if (self->input_state)
       gst_video_codec_state_unref (self->input_state);
@@ -1106,6 +1111,11 @@ gst_omx_video_dec_set_format (GstVideoDecoder * decoder,
   if (needs_disable) {
     if (gst_omx_port_set_enabled (self->in_port, TRUE) != OMX_ErrorNone)
       return FALSE;
+    if (gst_omx_port_allocate_buffers (self->in_port) != OMX_ErrorNone)
+      return FALSE;
+    if (gst_omx_port_wait_enabled (self->in_port,
+            5 * GST_SECOND) != OMX_ErrorNone)
+      return FALSE;
     if (gst_omx_port_manual_reconfigure (self->in_port, FALSE) != OMX_ErrorNone)
       return FALSE;
   } else {

omx/gstomxvideoenc.c

@@ -974,6 +974,11 @@ gst_omx_video_enc_set_format (GstVideoEncoder * encoder,
       return FALSE;
     if (gst_omx_port_set_enabled (self->in_port, FALSE) != OMX_ErrorNone)
       return FALSE;
+    if (gst_omx_port_deallocate_buffers (self->in_port) != OMX_ErrorNone)
+      return FALSE;
+    if (gst_omx_port_wait_enabled (self->in_port,
+            5 * GST_SECOND) != OMX_ErrorNone)
+      return FALSE;
 
     GST_DEBUG_OBJECT (self, "Encoder drained and disabled");
   }
@@ -1119,6 +1124,11 @@ gst_omx_video_enc_set_format (GstVideoEncoder * encoder,
   if (needs_disable) {
     if (gst_omx_port_set_enabled (self->in_port, TRUE) != OMX_ErrorNone)
       return FALSE;
+    if (gst_omx_port_allocate_buffers (self->in_port) != OMX_ErrorNone)
+      return FALSE;
+    if (gst_omx_port_wait_enabled (self->in_port,
+            5 * GST_SECOND) != OMX_ErrorNone)
+      return FALSE;
     if (gst_omx_port_manual_reconfigure (self->in_port, FALSE) != OMX_ErrorNone)
       return FALSE;
   } else {