From 46910bdc57c35ac36bd4adcbb76f4f3a590e3f21 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Thu, 18 Jul 2013 19:15:17 -0700 Subject: [PATCH] Camera2/3: Fix deadlock when starting recording before preview. Move 3A notification synthesis for HAL3 devices from Camera3Device::processCaptureResult to Camera2Client's FrameProcessor. This will ensure that calls to processCaptureResult from HAL can never block on Camera2Client internal mutexes. Bug: 9923891 Change-Id: I5184649bf45c0807babe6b8c0e1239e959cd3480 --- services/camera/libcameraservice/Camera2Device.cpp | 4 + services/camera/libcameraservice/Camera2Device.h | 1 + services/camera/libcameraservice/Camera3Device.cpp | 84 ++--------------- services/camera/libcameraservice/Camera3Device.h | 13 +-- .../camera/libcameraservice/CameraDeviceBase.h | 7 ++ .../libcameraservice/camera2/FrameProcessor.cpp | 101 +++++++++++++++++++++ .../libcameraservice/camera2/FrameProcessor.h | 19 ++++ 7 files changed, 140 insertions(+), 89 deletions(-) diff --git a/services/camera/libcameraservice/Camera2Device.cpp b/services/camera/libcameraservice/Camera2Device.cpp index 77df1524fb..710d0e9f27 100644 --- a/services/camera/libcameraservice/Camera2Device.cpp +++ b/services/camera/libcameraservice/Camera2Device.cpp @@ -445,6 +445,10 @@ status_t Camera2Device::setNotifyCallback(NotificationListener *listener) { return res; } +bool Camera2Device::willNotify3A() { + return true; +} + void Camera2Device::notificationCallback(int32_t msg_type, int32_t ext1, int32_t ext2, diff --git a/services/camera/libcameraservice/Camera2Device.h b/services/camera/libcameraservice/Camera2Device.h index 3034a1d04a..372ce9fdbf 100644 --- a/services/camera/libcameraservice/Camera2Device.h +++ b/services/camera/libcameraservice/Camera2Device.h @@ -59,6 +59,7 @@ class Camera2Device: public CameraDeviceBase { virtual status_t createDefaultRequest(int templateId, CameraMetadata *request); virtual status_t waitUntilDrained(); virtual status_t setNotifyCallback(NotificationListener *listener); + virtual bool willNotify3A(); virtual status_t waitForNextFrame(nsecs_t timeout); virtual status_t getNextFrame(CameraMetadata *frame); virtual status_t triggerAutofocus(uint32_t id); diff --git a/services/camera/libcameraservice/Camera3Device.cpp b/services/camera/libcameraservice/Camera3Device.cpp index cc7802bbf3..353fe742ba 100644 --- a/services/camera/libcameraservice/Camera3Device.cpp +++ b/services/camera/libcameraservice/Camera3Device.cpp @@ -837,6 +837,10 @@ status_t Camera3Device::setNotifyCallback(NotificationListener *listener) { return OK; } +bool Camera3Device::willNotify3A() { + return false; +} + status_t Camera3Device::waitForNextFrame(nsecs_t timeout) { ATRACE_CALL(); status_t res; @@ -1235,13 +1239,6 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) { } - AlgState cur3aState; - AlgState new3aState; - int32_t aeTriggerId = 0; - int32_t afTriggerId = 0; - - NotificationListener *listener = NULL; - // Process the result metadata, if provided if (result->result != NULL) { Mutex::Autolock l(mOutputLock); @@ -1280,59 +1277,6 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) { " metadata for frame %d (%lld vs %lld respectively)", frameNumber, timestamp, entry.data.i64[0]); } - - // Get 3A states from result metadata - - entry = captureResult.find(ANDROID_CONTROL_AE_STATE); - if (entry.count == 0) { - CLOGE("No AE state provided by HAL for frame %d!", - frameNumber); - } else { - new3aState.aeState = - static_cast( - entry.data.u8[0]); - } - - entry = captureResult.find(ANDROID_CONTROL_AF_STATE); - if (entry.count == 0) { - CLOGE("No AF state provided by HAL for frame %d!", - frameNumber); - } else { - new3aState.afState = - static_cast( - entry.data.u8[0]); - } - - entry = captureResult.find(ANDROID_CONTROL_AWB_STATE); - if (entry.count == 0) { - CLOGE("No AWB state provided by HAL for frame %d!", - frameNumber); - } else { - new3aState.awbState = - static_cast( - entry.data.u8[0]); - } - - entry = captureResult.find(ANDROID_CONTROL_AF_TRIGGER_ID); - if (entry.count == 0) { - CLOGE("No AF trigger ID provided by HAL for frame %d!", - frameNumber); - } else { - afTriggerId = entry.data.i32[0]; - } - - entry = captureResult.find(ANDROID_CONTROL_AE_PRECAPTURE_ID); - if (entry.count == 0) { - CLOGE("No AE precapture trigger ID provided by HAL" - " for frame %d!", frameNumber); - } else { - aeTriggerId = entry.data.i32[0]; - } - - listener = mListener; - cur3aState = m3AState; - - m3AState = new3aState; } // scope for mOutputLock // Return completed buffers to their streams with the timestamp @@ -1349,30 +1293,16 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) { } } - // Finally, dispatch any 3A change events to listeners if we got metadata + // Finally, signal any waiters for new frames if (result->result != NULL) { mResultSignal.signal(); } - if (result->result != NULL && listener != NULL) { - if (new3aState.aeState != cur3aState.aeState) { - ALOGVV("%s: AE state changed from 0x%x to 0x%x", - __FUNCTION__, cur3aState.aeState, new3aState.aeState); - listener->notifyAutoExposure(new3aState.aeState, aeTriggerId); - } - if (new3aState.afState != cur3aState.afState) { - ALOGVV("%s: AF state changed from 0x%x to 0x%x", - __FUNCTION__, cur3aState.afState, new3aState.afState); - listener->notifyAutoFocus(new3aState.afState, afTriggerId); - } - if (new3aState.awbState != cur3aState.awbState) { - listener->notifyAutoWhitebalance(new3aState.awbState, aeTriggerId); - } - } - } + + void Camera3Device::notify(const camera3_notify_msg *msg) { NotificationListener *listener; { diff --git a/services/camera/libcameraservice/Camera3Device.h b/services/camera/libcameraservice/Camera3Device.h index faa42b9b65..2328f89462 100644 --- a/services/camera/libcameraservice/Camera3Device.h +++ b/services/camera/libcameraservice/Camera3Device.h @@ -107,6 +107,7 @@ class Camera3Device : virtual status_t waitUntilDrained(); virtual status_t setNotifyCallback(NotificationListener *listener); + virtual bool willNotify3A(); virtual status_t waitForNextFrame(nsecs_t timeout); virtual status_t getNextFrame(CameraMetadata *frame); @@ -389,18 +390,6 @@ class Camera3Device : Condition mResultSignal; NotificationListener *mListener; - struct AlgState { - camera_metadata_enum_android_control_ae_state aeState; - camera_metadata_enum_android_control_af_state afState; - camera_metadata_enum_android_control_awb_state awbState; - - AlgState() : - aeState(ANDROID_CONTROL_AE_STATE_INACTIVE), - afState(ANDROID_CONTROL_AF_STATE_INACTIVE), - awbState(ANDROID_CONTROL_AWB_STATE_INACTIVE) { - } - } m3AState; - /**** End scope for mOutputLock ****/ /** diff --git a/services/camera/libcameraservice/CameraDeviceBase.h b/services/camera/libcameraservice/CameraDeviceBase.h index 8c457d9c70..aa92bec55b 100644 --- a/services/camera/libcameraservice/CameraDeviceBase.h +++ b/services/camera/libcameraservice/CameraDeviceBase.h @@ -156,6 +156,13 @@ class CameraDeviceBase : public virtual RefBase { virtual status_t setNotifyCallback(NotificationListener *listener) = 0; /** + * Whether the device supports calling notifyAutofocus, notifyAutoExposure, + * and notifyAutoWhitebalance; if this returns false, the client must + * synthesize these notifications from received frame metadata. + */ + virtual bool willNotify3A() = 0; + + /** * Wait for a new frame to be produced, with timeout in nanoseconds. * Returns TIMED_OUT when no frame produced within the specified duration */ diff --git a/services/camera/libcameraservice/camera2/FrameProcessor.cpp b/services/camera/libcameraservice/camera2/FrameProcessor.cpp index d13d398537..114a7a8ad4 100644 --- a/services/camera/libcameraservice/camera2/FrameProcessor.cpp +++ b/services/camera/libcameraservice/camera2/FrameProcessor.cpp @@ -33,6 +33,9 @@ FrameProcessor::FrameProcessor(wp device, ProFrameProcessor(device), mClient(client), mLastFrameNumberOfFaces(0) { + + sp d = device.promote(); + mSynthesize3ANotify = !(d->willNotify3A()); } FrameProcessor::~FrameProcessor() { @@ -50,6 +53,11 @@ bool FrameProcessor::processSingleFrame(CameraMetadata &frame, return false; } + if (mSynthesize3ANotify) { + // Ignoring missing fields for now + process3aState(frame, client); + } + if (!ProFrameProcessor::processSingleFrame(frame, device)) { return false; } @@ -185,6 +193,99 @@ status_t FrameProcessor::processFaceDetect(const CameraMetadata &frame, return OK; } +status_t FrameProcessor::process3aState(const CameraMetadata &frame, + const sp &client) { + + ATRACE_CALL(); + camera_metadata_ro_entry_t entry; + int mId = client->getCameraId(); + + entry = frame.find(ANDROID_REQUEST_FRAME_COUNT); + int32_t frameNumber = entry.data.i32[0]; + + // Get 3A states from result metadata + bool gotAllStates = true; + + AlgState new3aState; + + entry = frame.find(ANDROID_CONTROL_AE_STATE); + if (entry.count == 0) { + ALOGE("%s: Camera %d: No AE state provided by HAL for frame %d!", + __FUNCTION__, mId, frameNumber); + gotAllStates = false; + } else { + new3aState.aeState = + static_cast( + entry.data.u8[0]); + } + + entry = frame.find(ANDROID_CONTROL_AF_STATE); + if (entry.count == 0) { + ALOGE("%s: Camera %d: No AF state provided by HAL for frame %d!", + __FUNCTION__, mId, frameNumber); + gotAllStates = false; + } else { + new3aState.afState = + static_cast( + entry.data.u8[0]); + } + + entry = frame.find(ANDROID_CONTROL_AWB_STATE); + if (entry.count == 0) { + ALOGE("%s: Camera %d: No AWB state provided by HAL for frame %d!", + __FUNCTION__, mId, frameNumber); + gotAllStates = false; + } else { + new3aState.awbState = + static_cast( + entry.data.u8[0]); + } + + int32_t afTriggerId = 0; + entry = frame.find(ANDROID_CONTROL_AF_TRIGGER_ID); + if (entry.count == 0) { + ALOGE("%s: Camera %d: No AF trigger ID provided by HAL for frame %d!", + __FUNCTION__, mId, frameNumber); + gotAllStates = false; + } else { + afTriggerId = entry.data.i32[0]; + } + + int32_t aeTriggerId = 0; + entry = frame.find(ANDROID_CONTROL_AE_PRECAPTURE_ID); + if (entry.count == 0) { + ALOGE("%s: Camera %d: No AE precapture trigger ID provided by HAL" + " for frame %d!", + __FUNCTION__, mId, frameNumber); + gotAllStates = false; + } else { + aeTriggerId = entry.data.i32[0]; + } + + if (!gotAllStates) return BAD_VALUE; + + if (new3aState.aeState != m3aState.aeState) { + ALOGV("%s: AE state changed from 0x%x to 0x%x", + __FUNCTION__, m3aState.aeState, new3aState.aeState); + client->notifyAutoExposure(new3aState.aeState, aeTriggerId); + } + if (new3aState.afState != m3aState.afState) { + ALOGV("%s: AF state changed from 0x%x to 0x%x", + __FUNCTION__, m3aState.afState, new3aState.afState); + client->notifyAutoFocus(new3aState.afState, afTriggerId); + } + if (new3aState.awbState != m3aState.awbState) { + ALOGV("%s: AWB state changed from 0x%x to 0x%x", + __FUNCTION__, m3aState.awbState, new3aState.awbState); + client->notifyAutoWhitebalance(new3aState.awbState, aeTriggerId); + } + + m3aState = new3aState; + + return OK; +} + + void FrameProcessor::callbackFaceDetection(sp client, const camera_frame_metadata &metadata) { diff --git a/services/camera/libcameraservice/camera2/FrameProcessor.h b/services/camera/libcameraservice/camera2/FrameProcessor.h index 27ed8f6401..f480c551ed 100644 --- a/services/camera/libcameraservice/camera2/FrameProcessor.h +++ b/services/camera/libcameraservice/camera2/FrameProcessor.h @@ -44,6 +44,9 @@ class FrameProcessor : public ProFrameProcessor { private: wp mClient; + + bool mSynthesize3ANotify; + int mLastFrameNumberOfFaces; void processNewFrames(const sp &client); @@ -54,6 +57,22 @@ class FrameProcessor : public ProFrameProcessor { status_t processFaceDetect(const CameraMetadata &frame, const sp &client); + // Send 3A state change notifications to client based on frame metadata + status_t process3aState(const CameraMetadata &frame, + const sp &client); + + struct AlgState { + camera_metadata_enum_android_control_ae_state aeState; + camera_metadata_enum_android_control_af_state afState; + camera_metadata_enum_android_control_awb_state awbState; + + AlgState() : + aeState(ANDROID_CONTROL_AE_STATE_INACTIVE), + afState(ANDROID_CONTROL_AF_STATE_INACTIVE), + awbState(ANDROID_CONTROL_AWB_STATE_INACTIVE) { + } + } m3aState; + // Emit FaceDetection event to java if faces changed void callbackFaceDetection(sp client, const camera_frame_metadata &metadata); -- 2.11.0