CameraHal: Avoid deadlocks when re-using a tapout

- Locking inside 'takePicture()' is sometimes possible
  when re-using the same SurfaceTexture. This is due
  to the blocking nature of the dequeue call inside
  'BufferSourceAdapter::handleFrameReturn()' which
  does acquire 'mLock' upon entering. The solution
  is to handle this particular case by reusing the
  buffers that were previously allocated and avoiding
  any calls to the Adapter which might try to lock
  'mLock'.

Change-Id: I6c4e49fd84df2659c0466b6f89c88916f93a6b8d
Signed-off-by: Emilian Peev <epeev@mm-sol.com>
Signed-off-by: Vladimir Petrov <vppetrov@mm-sol.com>

2 files changed, 30 insertions, 16 deletions

diff --git a/camera/CameraHal.cpp b/camera/CameraHal.cpp
index 73bca33..15966e3 100644
--- a/camera/CameraHal.cpp
+++ b/camera/CameraHal.cpp
@@ -1465,10 +1465,12 @@ status_t CameraHal::allocImageBufs(unsigned int width, unsigned int height, size
         mImageFd = mMemoryManager->getFd();
         mImageLength = bytes;
         mImageOffsets = mMemoryManager->getOffsets();
+        mImageCount = bufferCount;
     } else {
         mImageFd = -1;
         mImageLength = 0;
         mImageOffsets = NULL;
+        mImageCount = 0;
     }
 
     LOG_FUNCTION_NAME_EXIT;
@@ -3174,6 +3176,7 @@ status_t CameraHal::__takePicture(const char *params)
     unsigned int rawBufferCount = 1;
     bool isCPCamMode = false;
     android::sp<DisplayAdapter> outAdapter = 0;
+    bool reuseTapout = false;
 
 #if PPM_INSTRUMENTATION || PPM_INSTRUMENTATION_ABS
 
@@ -3272,6 +3275,9 @@ status_t CameraHal::__takePicture(const char *params)
             }
             CAMHAL_LOGD("Found matching out adapter at %d", index);
             outAdapter = mOutAdapters.itemAt(index);
+            if ( outAdapter == mBufferSourceAdapter_Out ) {
+                reuseTapout = true;
+            }
         }
 
         mCameraAdapter->setParameters(mParameters);
@@ -3312,9 +3318,13 @@ status_t CameraHal::__takePicture(const char *params)
                                CameraHal::NO_BUFFERS_IMAGE_CAPTURE : burst;
 
              if (outAdapter.get()) {
-                bufferCount = outAdapter->getBufferCount();
-                if (bufferCount < 1) {
-                    bufferCount = NO_BUFFERS_IMAGE_CAPTURE_SYSTEM_HEAP;
+                if ( reuseTapout ) {
+                    bufferCount = mImageCount;
+                } else {
+                    bufferCount = outAdapter->getBufferCount();
+                    if (bufferCount < 1) {
+                        bufferCount = NO_BUFFERS_IMAGE_CAPTURE_SYSTEM_HEAP;
+                    }
                 }
              }
 
@@ -3373,20 +3383,22 @@ status_t CameraHal::__takePicture(const char *params)
             }
 
         if (outAdapter.get()) {
-            bool reset;
-            // Need to reset buffers if we are switching adapters since we don't know
-            // the state of the new buffer list
-            reset = (outAdapter.get() != mBufferSourceAdapter_Out.get());
-            ret = outAdapter->maxQueueableBuffers(max_queueable);
-            if (NO_ERROR != ret) {
-                CAMHAL_LOGE("Couldn't get max queuable");
-                return ret;
+            // Avoid locking the tapout again when reusing it
+            if (!reuseTapout) {
+                // Need to reset buffers if we are switching adapters since we don't know
+                // the state of the new buffer list
+                ret = outAdapter->maxQueueableBuffers(max_queueable);
+                if (NO_ERROR != ret) {
+                    CAMHAL_LOGE("Couldn't get max queuable");
+                    return ret;
+                }
+                mImageBuffers = outAdapter->getBuffers(true);
+                mImageOffsets = outAdapter->getOffsets();
+                mImageFd = outAdapter->getFd();
+                mImageLength = outAdapter->getSize();
+                mImageCount = bufferCount;
+                mBufferSourceAdapter_Out = outAdapter;
             }
-            mImageBuffers = outAdapter->getBuffers(reset);
-            mImageOffsets = outAdapter->getOffsets();
-            mImageFd = outAdapter->getFd();
-            mImageLength = outAdapter->getSize();
-            mBufferSourceAdapter_Out = outAdapter;
         } else {
             mBufferSourceAdapter_Out.clear();
             // allocImageBufs will only allocate new buffers if mImageBuffers is NULL
@@ -3858,6 +3870,7 @@ CameraHal::CameraHal(int cameraId)
     mImageOffsets = NULL;
     mImageLength = 0;
     mImageFd = 0;
+    mImageCount = 0;
     mVideoOffsets = NULL;
     mVideoFd = 0;
     mVideoLength = 0;
diff --git a/camera/inc/CameraHal.h b/camera/inc/CameraHal.h
index d750270..1aa7b7f 100644
--- a/camera/inc/CameraHal.h
+++ b/camera/inc/CameraHal.h
@@ -1460,6 +1460,7 @@ private:
     uint32_t *mImageOffsets;
     int mImageFd;
     int mImageLength;
+    unsigned int mImageCount;
     CameraBuffer *mPreviewBuffers;
     uint32_t *mPreviewOffsets;
     int mPreviewLength;