Camera: rework API1 shim takePicture retry logic

onBufferReleased is no longer reliable indicator of capture
error due to HAL buffer manager feature. Switch to listen
to error callback from HAL directly.

Test: API1 CTS + Pixel 3
Bug: 123952355
Change-Id: I7362942f19356583ec66f259b01e963a1af3a205

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 camera3::Camera3StreamBufferListener {
+            public Thread, public CpuConsumer::FrameAvailableListener {
   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,