CameraHal: Error handling before sending frames

Adding a few error checks before sending frames to make
sure we have the proper conditions to send a frame.

Fixes a bug where FillBufferDone gets preempted by the stop
path and by the time it returns the buffers and subscribers
are freed. Adding checks in send frame context since it is
protected by a Mutex.

Change-Id: I860bee796669606e1ec3e26e19916f386fc3e9aa

4 files changed, 91 insertions, 104 deletions

diff --git a/camera/ANativeWindowDisplayAdapter.cpp b/camera/ANativeWindowDisplayAdapter.cpp
index 0c4df00..220d637 100644
--- a/camera/ANativeWindowDisplayAdapter.cpp
+++ b/camera/ANativeWindowDisplayAdapter.cpp
@@ -826,6 +826,7 @@ int ANativeWindowDisplayAdapter::freeBuffer(void* buf)
     if ( NULL != buf )
     {
         delete [] buffers;
+        mGrallocHandleMap = NULL;
     }
 
     if( mBufferHandleMap != NULL)
@@ -1001,6 +1002,12 @@ status_t ANativeWindowDisplayAdapter::PostFrame(ANativeWindowDisplayAdapter::Dis
     ///@todo Insert logic to drop frames here based on refresh rate of
     ///display or rendering rate whichever is lower
     ///Queue the buffer to overlay
+
+    if (!mGrallocHandleMap || !dispFrame.mBuffer) {
+        CAMHAL_LOGEA("NULL sent to PostFrame");
+        return -EINVAL;
+    }
+
     for ( i = 0; i < mBufferCount; i++ )
         {
         if ( ((int) dispFrame.mBuffer ) == (int)mGrallocHandleMap[i] )
diff --git a/camera/BaseCameraAdapter.cpp b/camera/BaseCameraAdapter.cpp
index 74f80de..45e5027 100644
--- a/camera/BaseCameraAdapter.cpp
+++ b/camera/BaseCameraAdapter.cpp
@@ -1083,10 +1083,6 @@ status_t BaseCameraAdapter::notifyFaceSubscribers(sp<CameraFDResult> &faces)
 status_t BaseCameraAdapter::sendFrameToSubscribers(CameraFrame *frame)
 {
     status_t ret = NO_ERROR;
-    frame_callback callback;
-    uint32_t i = 0;
-    KeyedVector<int, frame_callback> *subscribers = NULL;
-    size_t refCount = 0;
     unsigned int mask;
 
     if ( NULL == frame )
@@ -1104,122 +1100,32 @@ status_t BaseCameraAdapter::sendFrameToSubscribers(CameraFrame *frame)
 #if PPM_INSTRUMENTATION || PPM_INSTRUMENTATION_ABS
             CameraHal::PPM("Shot to Jpeg: ", &mStartCapture);
 #endif
-            subscribers = &mImageSubscribers;
-            frame->mFrameType = CameraFrame::IMAGE_FRAME;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::IMAGE_FRAME);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mImageSubscribers, CameraFrame::IMAGE_FRAME);
           }
           break;
         case CameraFrame::RAW_FRAME:
           {
-            subscribers = &mRawSubscribers;
-            frame->mFrameType = CameraFrame::RAW_FRAME;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::RAW_FRAME);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mRawSubscribers, CameraFrame::RAW_FRAME);
           }
           break;
         case CameraFrame::PREVIEW_FRAME_SYNC:
           {
-            subscribers = &mFrameSubscribers;
-            frame->mFrameType = CameraFrame::PREVIEW_FRAME_SYNC;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::PREVIEW_FRAME_SYNC);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mFrameSubscribers, CameraFrame::PREVIEW_FRAME_SYNC);
           }
           break;
         case CameraFrame::SNAPSHOT_FRAME:
           {
-            subscribers = &mFrameSubscribers;
-            frame->mFrameType = CameraFrame::SNAPSHOT_FRAME;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::SNAPSHOT_FRAME);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mFrameSubscribers, CameraFrame::SNAPSHOT_FRAME);
           }
           break;
         case CameraFrame::VIDEO_FRAME_SYNC:
           {
-            subscribers = &mVideoSubscribers;
-            frame->mFrameType = CameraFrame::VIDEO_FRAME_SYNC;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::VIDEO_FRAME_SYNC);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mVideoSubscribers, CameraFrame::VIDEO_FRAME_SYNC);
           }
           break;
         case CameraFrame::FRAME_DATA_SYNC:
           {
-            subscribers = &mFrameDataSubscribers;
-            frame->mFrameType = CameraFrame::FRAME_DATA_SYNC;
-            if (NULL != subscribers)
-              {
-                refCount = getFrameRefCount(frame->mBuffer, CameraFrame::FRAME_DATA_SYNC);
-                CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
-                             frame->mFrameType,
-                             ( uint32_t ) frame->mBuffer,
-                             refCount);
-                for ( i = 0 ; i < refCount; i++ )
-                  {
-                    frame->mCookie = ( void * ) subscribers->keyAt(i);
-                    callback = (frame_callback) subscribers->valueAt(i);
-                    callback(frame);
-                  }
-              }
+            ret = __sendFrameToSubscribers(frame, &mFrameDataSubscribers, CameraFrame::FRAME_DATA_SYNC);
           }
           break;
         default:
@@ -1227,10 +1133,64 @@ status_t BaseCameraAdapter::sendFrameToSubscribers(CameraFrame *frame)
         break;
         }//SWITCH
         frame->mFrameMask &= ~mask;
+
+        if (ret != NO_ERROR) {
+            goto EXIT;
+        }
       }//IF
     }//FOR
+
+ EXIT:
+    return ret;
+}
+
+status_t BaseCameraAdapter::__sendFrameToSubscribers(CameraFrame* frame,
+                                                     KeyedVector<int, frame_callback> *subscribers,
+                                                     CameraFrame::FrameType frameType)
+{
+    size_t refCount = 0;
+    status_t ret = NO_ERROR;
+    frame_callback callback = NULL;
+
+    frame->mFrameType = frameType;
+
+    if (NULL != subscribers) {
+        refCount = getFrameRefCount(frame->mBuffer, frameType);
+
+        if (refCount == 0) {
+            CAMHAL_LOGEB("Invalid ref count of 0");
+            return -EINVAL;
+        }
+
+        if (refCount > subscribers->size()) {
+            CAMHAL_LOGEB("Invalid ref count for frame type: 0x%x", frameType);
+            return -EINVAL;
+        }
+
+        CAMHAL_LOGVB("Type of Frame: 0x%x address: 0x%x refCount start %d",
+                     frame->mFrameType,
+                     ( uint32_t ) frame->mBuffer,
+                     refCount);
+
+        for ( unsigned int i = 0 ; i < refCount; i++ ) {
+            frame->mCookie = ( void * ) subscribers->keyAt(i);
+            callback = (frame_callback) subscribers->valueAt(i);
+
+            if (!callback) {
+                CAMHAL_LOGEB("callback not set for frame type: 0x%x", frameType);
+                return -EINVAL;
+            }
+
+            callback(frame);
+        }
+    } else {
+        CAMHAL_LOGEB("Subscribers is null??");
+        return -EINVAL;
+    }
+
     return ret;
 }
+
 int BaseCameraAdapter::setInitFrameRefCount(void* buf, unsigned int mask)
 {
   int ret = NO_ERROR;
diff --git a/camera/OMXCameraAdapter/OMXCameraAdapter.cpp b/camera/OMXCameraAdapter/OMXCameraAdapter.cpp
index 577d0fd..b378a92 100644
--- a/camera/OMXCameraAdapter/OMXCameraAdapter.cpp
+++ b/camera/OMXCameraAdapter/OMXCameraAdapter.cpp
@@ -2405,7 +2405,7 @@ void OMXCameraAdapter::onOrientationEvent(uint32_t orientation, uint32_t tilt)
 {
     LOG_FUNCTION_NAME;
 
-    static const int DEGREES_TILT_IGNORE = 45;
+    static const unsigned int DEGREES_TILT_IGNORE = 45;
     int device_orientation = 0;
     int mount_orientation = 0;
     const char *facing_direction = NULL;
@@ -2530,7 +2530,7 @@ OMX_ERRORTYPE OMXCameraAdapter::OMXCameraAdapterEventHandler(OMX_IN OMX_HANDLETY
                 */
                 if ( !mEventSignalQ.isEmpty() )
                   {
-                    for (int i = 0 ; i < mEventSignalQ.size(); i++ )
+                    for (unsigned int i = 0 ; i < mEventSignalQ.size(); i++ )
                       {
                         CAMHAL_LOGEB("***Removing %d EVENTS***** \n", mEventSignalQ.size());
                         //remove from queue and free msg
@@ -2811,6 +2811,12 @@ OMX_ERRORTYPE OMXCameraAdapter::OMXCameraAdapterFillBufferDone(OMX_IN OMX_HANDLE
 
     res1 = res2 = NO_ERROR;
     pPortParam = &(mCameraAdapterParameters.mCameraPortParams[pBuffHeader->nOutputPortIndex]);
+
+    if ( !pBuffHeader || !pBuffHeader->pBuffer ) {
+        CAMHAL_LOGEA("NULL Buffer from OMX");
+        return OMX_ErrorNone;
+    }
+
     if (pBuffHeader->nOutputPortIndex == OMX_CAMERA_PORT_VIDEO_OUT_PREVIEW)
         {
 
@@ -3064,8 +3070,14 @@ status_t OMXCameraAdapter::sendCallBacks(CameraFrame frame, OMX_IN OMX_BUFFERHEA
     }
 
   frame.mTimestamp = (pBuffHeader->nTimeStamp * 1000) - mTimeSourceDelta;
-  setInitFrameRefCount(frame.mBuffer, mask);
-  ret = sendFrameToSubscribers(&frame);
+
+  ret = setInitFrameRefCount(frame.mBuffer, mask);
+
+  if (ret != NO_ERROR) {
+     CAMHAL_LOGEA("Error in setInitFrameRefCount %d", ret);
+  } else {
+      ret = sendFrameToSubscribers(&frame);
+  }
 
   LOG_FUNCTION_NAME_EXIT;
 
diff --git a/camera/inc/BaseCameraAdapter.h b/camera/inc/BaseCameraAdapter.h
index c982f94..5a014b5 100644
--- a/camera/inc/BaseCameraAdapter.h
+++ b/camera/inc/BaseCameraAdapter.h
@@ -154,6 +154,14 @@ protected:
     int getFrameRefCount(void* frameBuf, CameraFrame::FrameType frameType);
     int setInitFrameRefCount(void* buf, unsigned int mask);
 
+// private member functions
+private:
+    status_t __sendFrameToSubscribers(CameraFrame* frame,
+                                      KeyedVector<int, frame_callback> *subscribers,
+                                      CameraFrame::FrameType frameType);
+
+// protected data types and variables
+protected:
     enum FrameState {
         STOPPED = 0,
         RUNNING