Merge changes I7362942f,I946fb95d

* changes:
  Camera: rework API1 shim takePicture retry logic
  Camera: bug fixes for HAL buffer manager

services/camera/libcameraservice/api1/Camera2Client.cpp

@@ -1773,6 +1773,8 @@ void Camera2Client::notifyError(int32_t errorCode,
             break;
     }
 
+    mCaptureSequencer->notifyError(errorCode, resultExtras);
+
     ALOGE("%s: Error condition %d reported by HAL, requestId %" PRId32, __FUNCTION__, errorCode,
               resultExtras.requestId);
 
@@ -1927,9 +1929,6 @@ void Camera2Client::notifyAutoExposure(uint8_t newState, int triggerId) {
 
 void Camera2Client::notifyShutter(const CaptureResultExtras& resultExtras,
                                   nsecs_t timestamp) {
-    (void)resultExtras;
-    (void)timestamp;
-
     ALOGV("%s: Shutter notification for request id %" PRId32 " at time %" PRId64,
             __FUNCTION__, resultExtras.requestId, timestamp);
     mCaptureSequencer->notifyShutter(resultExtras, timestamp);

services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp

@@ -117,6 +117,31 @@ void CaptureSequencer::notifyShutter(const CaptureResultExtras& resultExtras,
     }
 }
 
+void CaptureSequencer::notifyError(int32_t errorCode, const CaptureResultExtras& resultExtras) {
+    ATRACE_CALL();
+    bool jpegBufferLost = false;
+    if (errorCode == hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_BUFFER) {
+        sp<Camera2Client> client = mClient.promote();
+        if (client == nullptr) {
+            return;
+        }
+        int captureStreamId = client->getCaptureStreamId();
+        if (captureStreamId == resultExtras.errorStreamId) {
+            jpegBufferLost = true;
+        }
+    } else if (errorCode ==
+            hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_REQUEST) {
+        if (resultExtras.requestId == mShutterCaptureId) {
+            jpegBufferLost = true;
+        }
+    }
+
+    if (jpegBufferLost) {
+        sp<MemoryBase> emptyBuffer;
+        onCaptureAvailable(/*timestamp*/0, emptyBuffer, /*captureError*/true);
+    }
+}
+
 void CaptureSequencer::onResultAvailable(const CaptureResult &result) {
     ATRACE_CALL();
     ALOGV("%s: New result available.", __FUNCTION__);

services/camera/libcameraservice/api1/client2/CaptureSequencer.h

@@ -65,6 +65,9 @@ class CaptureSequencer:
     void notifyShutter(const CaptureResultExtras& resultExtras,
                        nsecs_t timestamp);
 
+    // Notifications about shutter (capture start)
+    void notifyError(int32_t errorCode, const CaptureResultExtras& resultExtras);
+
     // Notification from the frame processor
     virtual void onResultAvailable(const CaptureResult &result);
 

services/camera/libcameraservice/api1/client2/JpegProcessor.cpp

@@ -62,31 +62,6 @@ void JpegProcessor::onFrameAvailable(const BufferItem& /*item*/) {
     }
 }
 
-void JpegProcessor::onBufferRequestForFrameNumber(uint64_t /*frameNumber*/,
-        int /*streamId*/, const CameraMetadata& /*settings*/) {
-    // Intentionally left empty
-}
-
-void JpegProcessor::onBufferAcquired(const BufferInfo& /*bufferInfo*/) {
-    // Intentionally left empty
-}
-
-void JpegProcessor::onBufferReleased(const BufferInfo& bufferInfo) {
-    ALOGV("%s", __FUNCTION__);
-    if (bufferInfo.mError) {
-        // Only lock in case of error, since we get one of these for each
-        // onFrameAvailable as well, and scheduling may delay this call late
-        // enough to run into later preview restart operations, for non-error
-        // cases.
-        // b/29524651
-        ALOGV("%s: JPEG buffer lost", __FUNCTION__);
-        Mutex::Autolock l(mInputMutex);
-        mCaptureDone = true;
-        mCaptureSuccess = false;
-        mCaptureDoneSignal.signal();
-    }
-}
-
 status_t JpegProcessor::updateStream(const Parameters &params) {
     ATRACE_CALL();
     ALOGV("%s", __FUNCTION__);
@@ -181,13 +156,6 @@ status_t JpegProcessor::updateStream(const Parameters &params) {
                     strerror(-res), res);
             return res;
         }
-
-        res = device->addBufferListenerForStream(mCaptureStreamId, this);
-        if (res != OK) {
-              ALOGE("%s: Camera %d: Can't add buffer listeneri: %s (%d)",
-                    __FUNCTION__, mId, strerror(-res), res);
-              return res;
-        }
     }
     return OK;
 }

services/camera/libcameraservice/api1/client2/JpegProcessor.h

@@ -42,8 +42,7 @@ struct Parameters;
  * Still image capture output image processing
  */
 class JpegProcessor:
-            public Thread, public CpuConsumer::FrameAvailableListener,
+            public Thread, public CpuConsumer::FrameAvailableListener {
-            public camera3::Camera3StreamBufferListener {
   public:
     JpegProcessor(sp<Camera2Client> client, wp<CaptureSequencer> sequencer);
     ~JpegProcessor();
@@ -51,12 +50,6 @@ class JpegProcessor:
     // CpuConsumer listener implementation
     void onFrameAvailable(const BufferItem& item);
 
-    // Camera3StreamBufferListener implementation
-    void onBufferAcquired(const BufferInfo& bufferInfo) override;
-    void onBufferReleased(const BufferInfo& bufferInfo) override;
-    void onBufferRequestForFrameNumber(uint64_t frameNumber, int streamId,
-            const CameraMetadata& settings) override;
-
     status_t updateStream(const Parameters &params);
     status_t deleteStream();
     int getStreamId() const;

services/camera/libcameraservice/device3/Camera3Device.cpp

@@ -885,14 +885,14 @@ status_t Camera3Device::convertMetadataListToRequestListLocked(
     return OK;
 }
 
-status_t Camera3Device::capture(CameraMetadata &request, int64_t* /*lastFrameNumber*/) {
+status_t Camera3Device::capture(CameraMetadata &request, int64_t* lastFrameNumber) {
     ATRACE_CALL();
 
     List<const PhysicalCameraSettingsList> requestsList;
     std::list<const SurfaceMap> surfaceMaps;
     convertToRequestList(requestsList, surfaceMaps, request);
 
-    return captureList(requestsList, surfaceMaps, /*lastFrameNumber*/NULL);
+    return captureList(requestsList, surfaceMaps, lastFrameNumber);
 }
 
 void Camera3Device::convertToRequestList(List<const PhysicalCameraSettingsList>& requestsList,
@@ -1027,11 +1027,22 @@ hardware::Return<void> Camera3Device::requestStreamBuffers(
             return hardware::Void();
         }
 
+        if (outputStream->isAbandoned()) {
+            bufRet.val.error(StreamBufferRequestError::STREAM_DISCONNECTED);
+            allReqsSucceeds = false;
+            continue;
+        }
+
         bufRet.streamId = streamId;
+        size_t handOutBufferCount = outputStream->getOutstandingBuffersCount();
         uint32_t numBuffersRequested = bufReq.numBuffersRequested;
-        size_t totalHandout = outputStream->getOutstandingBuffersCount() + numBuffersRequested;
+        size_t totalHandout = handOutBufferCount + numBuffersRequested;
-        if (totalHandout > outputStream->asHalStream()->max_buffers) {
+        uint32_t maxBuffers = outputStream->asHalStream()->max_buffers;
+        if (totalHandout > maxBuffers) {
             // Not able to allocate enough buffer. Exit early for this stream
+            ALOGE("%s: request too much buffers for stream %d: at HAL: %zu + requesting: %d"
+                    " > max: %d", __FUNCTION__, streamId, handOutBufferCount,
+                    numBuffersRequested, maxBuffers);
             bufRet.val.error(StreamBufferRequestError::MAX_BUFFER_EXCEEDED);
             allReqsSucceeds = false;
             continue;
@@ -2186,12 +2197,11 @@ status_t Camera3Device::waitUntilStateThenRelock(bool active, nsecs_t timeout) {
 
     mStatusWaiters++;
 
-    // Notify HAL to start draining. We need to notify the HalInterface layer
-    // even when the device is already IDLE, so HalInterface can reject incoming
-    // requestStreamBuffers call.
     if (!active && mUseHalBufManager) {
         auto streamIds = mOutputStreams.getStreamIds();
-        mRequestThread->signalPipelineDrain(streamIds);
+        if (mStatus == STATUS_ACTIVE) {
+            mRequestThread->signalPipelineDrain(streamIds);
+        }
         mRequestBufferSM.onWaitUntilIdle();
     }
 
@@ -5308,6 +5318,11 @@ bool Camera3Device::RequestThread::threadLoop() {
     ALOGVV("%s: %d: submitting %zu requests in a batch.", __FUNCTION__, __LINE__,
             mNextRequests.size());
 
+    sp<Camera3Device> parent = mParent.promote();
+    if (parent != nullptr) {
+        parent->mRequestBufferSM.onSubmittingRequest();
+    }
+
     bool submitRequestSuccess = false;
     nsecs_t tRequestStart = systemTime(SYSTEM_TIME_MONOTONIC);
     if (mInterface->supportBatchRequest()) {
@@ -5318,13 +5333,6 @@ bool Camera3Device::RequestThread::threadLoop() {
     nsecs_t tRequestEnd = systemTime(SYSTEM_TIME_MONOTONIC);
     mRequestLatency.add(tRequestStart, tRequestEnd);
 
-    if (submitRequestSuccess) {
-        sp<Camera3Device> parent = mParent.promote();
-        if (parent != nullptr) {
-            parent->mRequestBufferSM.onRequestSubmitted();
-        }
-    }
-
     if (useFlushLock) {
         mFlushLock.unlock();
     }
@@ -6486,9 +6494,11 @@ void Camera3Device::RequestBufferStateMachine::onStreamsConfigured() {
     return;
 }
 
-void Camera3Device::RequestBufferStateMachine::onRequestSubmitted() {
+void Camera3Device::RequestBufferStateMachine::onSubmittingRequest() {
     std::lock_guard<std::mutex> lock(mLock);
     mRequestThreadPaused = false;
+    // inflight map register actually happens in prepareHalRequest now, but it is close enough
+    // approximation.
     mInflightMapEmpty = false;
     if (mStatus == RB_STATUS_STOPPED) {
         mStatus = RB_STATUS_READY;

services/camera/libcameraservice/device3/Camera3Device.h

@@ -1320,7 +1320,7 @@ class Camera3Device :
         void onInflightMapEmpty();
 
         // Events triggered by RequestThread
-        void onRequestSubmitted();
+        void onSubmittingRequest();
         void onRequestThreadPaused();
 
       private:

services/camera/libcameraservice/device3/Camera3Stream.cpp

@@ -588,7 +588,11 @@ status_t Camera3Stream::getBuffer(camera3_stream_buffer *buffer,
     if (mState != STATE_CONFIGURED) {
         ALOGE("%s: Stream %d: Can't get buffers if stream is not in CONFIGURED state %d",
                 __FUNCTION__, mId, mState);
-        return INVALID_OPERATION;
+        if (mState == STATE_ABANDONED) {
+            return DEAD_OBJECT;
+        } else {
+            return INVALID_OPERATION;
+        }
     }
 
     // Wait for new buffer returned back if we are running into the limit.