Update OVDA to handle error condition.

content/common/gpu/media/omx_video_decode_accelerator.cc

Index: content/common/gpu/media/omx_video_decode_accelerator.cc
===================================================================
--- content/common/gpu/media/omx_video_decode_accelerator.cc	(revision 91463)
+++ content/common/gpu/media/omx_video_decode_accelerator.cc	(working copy)
@@ -146,11 +146,13 @@
   // For now consider only what we care about.
   std::vector<uint32> matched_configs;
   GetConfigs(config, &matched_configs);
-  if (config != matched_configs)
+  if (config != matched_configs) {
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
+  }
   client_state_ = OMX_StateLoaded;
   if (!CreateComponent()) {
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_UNINITIALIZED);

[Ami GONE FROM CHROMIUM, 2011/07/05 15:55:53]

Most of the error codes seem like very poor translations to me (from
OMX_ERRORTYPE to VideoDecodeAccelerator::Error) caused by the mismatch between
OMX_ET and VDA::E)
Do you want to take a crack at rewriting the VDA::E (and
PP_VideoDecoderError_Dev) enum values to better match the plugin-visible APIs? 
Otherwise I can put it on my TODO list.

[vjain, 2011/07/05 16:04:14]

I agree with you that error translation is not as expected in OVDA. I will at
this moment continue with this mapping and let you create a new error mapping.

     return false;
   }
 
@@ -160,13 +162,13 @@
       &OmxVideoDecodeAccelerator::OnStateChangeLoadedToIdle;
   if (!TransitionToState(OMX_StateIdle)) {
     LOG(ERROR) << "TransitionToState(OMX_StateIdle) error";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_UNINITIALIZED);
     return false;
   }
 
   if (!AllocateInputBuffers()) {
     LOG(ERROR) << "OMX_AllocateBuffer() Input buffer error";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_MEMFAILURE);
     return false;
   }
 
@@ -191,7 +193,7 @@
       const_cast<OMX_STRING>(role_name), &num_roles, 0);
   if (result != OMX_ErrorNone || num_roles == 0) {
     LOG(ERROR) << "Unsupported Role: " << role_name << ", " << result;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_UNSUPPORTED);
     return false;
   }
 
@@ -216,7 +218,7 @@
 
   if (result != OMX_ErrorNone || num_roles == 0) {
     LOG(ERROR) << "Unsupported Role: " << component_name.c_str();
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_UNSUPPORTED);
     return false;
   }
 
@@ -227,7 +229,7 @@
                          &omx_accelerator_callbacks);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "Failed to Load the component: " << component;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INSUFFICIENT_RESOURCES);
     return false;
   }
 
@@ -240,7 +242,7 @@
   if ((result != OMX_ErrorNone) || (port_param.nPorts != 2)) {
     LOG(ERROR) << "Failed to get Port Param: "
                << result << ", " << port_param.nPorts;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INSUFFICIENT_RESOURCES);
     return false;
   }
   input_port_ = port_param.nStartPortNumber;
@@ -258,7 +260,7 @@
                             &role_type);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "Failed to Set Role";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
 
@@ -271,12 +273,12 @@
                             &port_format);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "GetParameter(OMX_IndexParamPortDefinition) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
   if (OMX_DirInput != port_format.eDir) {
     LOG(ERROR) << "Expected input port";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
   input_buffer_count_ = port_format.nBufferCountActual;
@@ -290,12 +292,12 @@
                             &port_format);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "GetParameter(OMX_IndexParamPortDefinition) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
   if (OMX_DirOutput != port_format.eDir) {
     LOG(ERROR) << "Expect Output Port";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
 
@@ -312,7 +314,7 @@
                             &port_format);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "SetParameter(OMX_IndexParamPortDefinition) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
 
@@ -372,7 +374,7 @@
   result = OMX_EmptyThisBuffer(component_handle_, omx_buffer);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "OMX_EmptyThisBuffer() failed with result " << result;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
   input_buffers_at_component_++;
@@ -381,7 +383,7 @@
 void OmxVideoDecodeAccelerator::AssignGLESBuffers(
     const std::vector<media::GLESBuffer>& buffers) {
   if (!CanFillBuffer()) {
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_UNINITIALIZED);
     return;
   }
   CHECK_EQ(output_buffers_at_component_, 0);
@@ -400,7 +402,7 @@
 
   if (!AllocateOutputBuffers()) {
     LOG(ERROR) << "OMX_AllocateBuffer() Output buffer error";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_MEMFAILURE);
     return;
   }
 
@@ -434,7 +436,7 @@
       OMX_FillThisBuffer(component_handle_, output_picture.omx_buffer_header);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "OMX_FillThisBuffer() failed with result " << result;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
 }
@@ -462,7 +464,7 @@
   result = OMX_EmptyThisBuffer(component_handle_, omx_buffer);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "OMX_EmptyThisBuffer() failed with result " << result;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
   input_buffers_at_component_++;
@@ -492,7 +494,7 @@
                                          input_port_, 0);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "OMX_SendCommand(OMX_CommandFlush) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
 }
@@ -509,7 +511,7 @@
                       OMX_CommandFlush,
                       output_port_, 0)) {
     LOG(ERROR) << "OMX_SendCommand(OMX_CommandFlush) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
 }
@@ -564,7 +566,7 @@
     OMX_ERRORTYPE result = OMX_FillThisBuffer(component_handle_, buffer);
     if (result != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_FillThisBuffer() failed with: " << result;
-      StopOnError();
+      StopOnError(VIDEODECODERERROR_INVALIDINPUT);
       return;
     }
     ++output_buffers_at_component_;
@@ -582,9 +584,10 @@
       component_handle_, OMX_CommandStateSet, new_state, 0);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "SendCommand(OMX_CommandStateSet) failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
+  client_state_ = new_state;
   return true;
 }
 
@@ -631,24 +634,33 @@
   client_->NotifyAbortDone();
 }
 
-void OmxVideoDecodeAccelerator::StopOnError() {
-  OMX_STATETYPE il_state;
-  OMX_GetState(component_handle_, &il_state);
+void OmxVideoDecodeAccelerator::StopOnError(
+                                media::VideoDecodeAccelerator::Error error) {
+  if (client_)
+      client_->NotifyError(error);
+  if (client_state_ == OMX_StateInvalid)
+      return;
+
   client_state_ = OMX_StateInvalid;
-  switch (il_state) {
-    case OMX_StateExecuting:
-      ShutDownOMXFromExecuting();
-      return;
-    case OMX_StateIdle:
-      OnStateChangeExecutingToIdle(OMX_StateIdle);
-      return;
-    case OMX_StateLoaded:
-      OnStateChangeIdleToLoaded(OMX_StateLoaded);
-      return;
-    default:
-      // LOG unexpected state or just ignore?
-      return;
+  if (component_handle_) {

[Ami GONE FROM CHROMIUM, 2011/07/05 15:55:53]

If you reverse this test you can have the return as the then clause, and
de-indent the rest of the function.

[vjain, 2011/07/05 16:04:14]

Will Fix this.

+      OMX_STATETYPE il_state;
+      OMX_GetState(component_handle_, &il_state);
+      switch (il_state) {
+        case OMX_StateExecuting:
+          ShutDownOMXFromExecuting();
+          break;

[Ami GONE FROM CHROMIUM, 2011/07/05 15:55:53]

Here and below I think return is clearer than break, and makes it less likely
that code is accidentally added to the bottom of this function and executed as
fall-through unintentionally.

[vjain, 2011/07/05 16:04:14]

will fix this.

+        case OMX_StateIdle:
+          OnStateChangeExecutingToIdle(OMX_StateIdle);
+          break;
+        case OMX_StateLoaded:
+          OnStateChangeIdleToLoaded(OMX_StateLoaded);
+          break;
+        default:
+          LOG(ERROR) << "Invalid state:"

[Ami GONE FROM CHROMIUM, 2011/07/05 15:55:53]

s/:/: /

[vjain, 2011/07/05 16:04:14]

Done

+                     << il_state << " received in StopOnError()";
+      }
   }
+  return;

[Ami GONE FROM CHROMIUM, 2011/07/05 15:55:53]

If you take my suggestion above this goes away.
Otherwise this is still redundant.

[vjain, 2011/07/05 16:04:14]

Done

 }
 
 bool OmxVideoDecodeAccelerator::AllocateInputBuffers() {
@@ -713,7 +725,7 @@
     result = OMX_FreeBuffer(component_handle_, input_port_, omx_buffer);
     if (result != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_FreeBuffer failed with: " << result;
-      StopOnError();
+      StopOnError(VIDEODECODERERROR_INVALIDINPUT);
       return;
     }
   }
@@ -731,7 +743,7 @@
     result = OMX_FreeBuffer(component_handle_, output_port_, omx_buffer);
     if (result != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_FreeBuffer failed with: " << result;
-      StopOnError();
+      StopOnError(VIDEODECODERERROR_INVALIDINPUT);
       return;
     }
     client_->DismissPictureBuffer(it->first);
@@ -781,7 +793,7 @@
 
   if (!CanFillBuffer()) {
     LOG(ERROR) << "Can't FillBuffer on port-enabled";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INSUFFICIENT_RESOURCES);
     return;
   }
 
@@ -797,7 +809,7 @@
     OMX_ERRORTYPE result = OMX_FillThisBuffer(component_handle_, omx_buffer);
     if (result != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_FillThisBuffer() failed with result " << result;
-      StopOnError();
+      StopOnError(VIDEODECODERERROR_INSUFFICIENT_BUFFERS);
       return;
     }
   }
@@ -815,7 +827,7 @@
         OMX_FreeBuffer(component_handle_, output_port_, buffer);
     if (result != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_FreeBuffer failed with: " << result;
-      StopOnError();
+      StopOnError(VIDEODECODERERROR_INVALIDINPUT);
       return;
     }
     return;
@@ -908,8 +920,7 @@
       break;
     }
     case OMX_EventError:
-      if (static_cast<OMX_ERRORTYPE>(data1) == OMX_ErrorInvalidState)
-        StopOnError();
+        StopOnError(VIDEODECODERERROR_HARDWARE);
       break;
     case OMX_EventPortSettingsChanged:
       if (data2 == OMX_IndexParamPortDefinition) {
@@ -1006,7 +1017,7 @@
   result = OMX_GetState(component_handle_, &il_state);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "OMX_GetState failed";
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return false;
   }
   return (il_state == OMX_StateExecuting);
@@ -1019,7 +1030,7 @@
                                          cmd, port_index, 0);
   if (result != OMX_ErrorNone) {
     LOG(ERROR) << "SendCommand() failed" << cmd << ":" << result;
-    StopOnError();
+    StopOnError(VIDEODECODERERROR_INVALIDINPUT);
     return;
   }
 }