From b662e330c84fbd49ab41d31bbf857a6dd97723ee Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Wed, 22 May 2013 15:54:57 -0700 Subject: camera3: Fix zsl buffers released-while-in-use race condition Bug: 9007356 Change-Id: I0ced31020410978c549d408b2815f925e9c9ffcf --- .../libcameraservice/camera2/ZslProcessor3.cpp | 2 ++ .../libcameraservice/gui/RingBufferConsumer.cpp | 30 +++++++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/services/camera/libcameraservice/camera2/ZslProcessor3.cpp b/services/camera/libcameraservice/camera2/ZslProcessor3.cpp index e2c120c..56525a3 100644 --- a/services/camera/libcameraservice/camera2/ZslProcessor3.cpp +++ b/services/camera/libcameraservice/camera2/ZslProcessor3.cpp @@ -402,6 +402,8 @@ nsecs_t ZslProcessor3::getCandidateTimestampLocked(size_t* metadataIdx) const { minTimestamp = frameTimestamp; idx = j; } + + ALOGVV("%s: Saw timestamp %lld", __FUNCTION__, frameTimestamp); } } diff --git a/services/camera/libcameraservice/gui/RingBufferConsumer.cpp b/services/camera/libcameraservice/gui/RingBufferConsumer.cpp index 2fa78a4..cd39bad 100644 --- a/services/camera/libcameraservice/gui/RingBufferConsumer.cpp +++ b/services/camera/libcameraservice/gui/RingBufferConsumer.cpp @@ -101,12 +101,6 @@ sp RingBufferConsumer::pinSelectedBuffer( } // end scope of mMutex autolock - if (pinnedBuffer != 0) { - BI_LOGV("Pinned buffer frame %lld, timestamp %lld", - pinnedBuffer->getBufferItem().mFrameNumber, - pinnedBuffer->getBufferItem().mTimestamp); - } - if (waitForFence) { status_t err = pinnedBuffer->getBufferItem().mFence->waitForever( "RingBufferConsumer::pinSelectedBuffer"); @@ -172,6 +166,9 @@ void RingBufferConsumer::pinBufferLocked(const BufferItem& item) { if (it == end) { BI_LOGE("Failed to pin buffer (timestamp %lld, framenumber %lld)", item.mTimestamp, item.mFrameNumber); + } else { + BI_LOGV("Pinned buffer (frame %lld, timestamp %lld)", + item.mFrameNumber, item.mTimestamp); } } @@ -182,7 +179,7 @@ status_t RingBufferConsumer::releaseOldestBufferLocked(size_t* pinnedFrames) { it = mBufferItemList.begin(); end = mBufferItemList.end(); - accIt = it; + accIt = end; if (it == end) { /** @@ -197,12 +194,17 @@ status_t RingBufferConsumer::releaseOldestBufferLocked(size_t* pinnedFrames) { for (; it != end; ++it) { RingBufferItem& find = *it; - if (find.mTimestamp < accIt->mTimestamp && find.mPinCount <= 0) { - accIt = it; + + if (find.mPinCount > 0) { + if (pinnedFrames != NULL) { + ++(*pinnedFrames); + } + // Filter out pinned frame when searching for buffer to release + continue; } - if (find.mPinCount > 0 && pinnedFrames != NULL) { - ++(*pinnedFrames); + if (find.mTimestamp < accIt->mTimestamp || accIt == end) { + accIt = it; } } @@ -323,7 +325,11 @@ void RingBufferConsumer::unpinBuffer(const BufferItem& item) { } if (it == end) { - BI_LOGE("Failed to unpin buffer (timestamp %lld, framenumber %lld", + // This should never happen. If it happens, we have a bug. + BI_LOGE("Failed to unpin buffer (timestamp %lld, framenumber %lld)", + item.mTimestamp, item.mFrameNumber); + } else { + BI_LOGV("Unpinned buffer (timestamp %lld, framenumber %lld)", item.mTimestamp, item.mFrameNumber); } } -- cgit v1.1 From f8f370b6ecb0227aa6f2b1c41d045eeaf613f726 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Thu, 23 May 2013 16:51:44 -0700 Subject: camera3: Disable ZSL for limited mode camera HALs Bug: 9111852 Change-Id: Idad7e0d2f912341bd643d0ad1a0861fb2043aa90 --- .../camera/libcameraservice/camera2/Parameters.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/services/camera/libcameraservice/camera2/Parameters.cpp b/services/camera/libcameraservice/camera2/Parameters.cpp index 49fc3d8..a248b76 100644 --- a/services/camera/libcameraservice/camera2/Parameters.cpp +++ b/services/camera/libcameraservice/camera2/Parameters.cpp @@ -795,13 +795,21 @@ status_t Parameters::initialize(const CameraMetadata *info) { previewCallbackFlags = 0; previewCallbackOneShot = false; - char value[PROPERTY_VALUE_MAX]; - property_get("camera.disable_zsl_mode", value, "0"); - if (!strcmp(value,"1")) { - ALOGI("Camera %d: Disabling ZSL mode", cameraId); + camera_metadata_ro_entry_t supportedHardwareLevel = + staticInfo(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL); + if (!supportedHardwareLevel.count || (supportedHardwareLevel.data.u8[0] == + ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED)) { + ALOGI("Camera %d: ZSL mode disabled for limited mode HALs", cameraId); zslMode = false; } else { - zslMode = true; + char value[PROPERTY_VALUE_MAX]; + property_get("camera.disable_zsl_mode", value, "0"); + if (!strcmp(value,"1")) { + ALOGI("Camera %d: Disabling ZSL mode", cameraId); + zslMode = false; + } else { + zslMode = true; + } } lightFx = LIGHTFX_NONE; -- cgit v1.1 From b4a0680452c4049b6342ebbee5f33db232f9370a Mon Sep 17 00:00:00 2001 From: Zhijun He Date: Wed, 22 May 2013 14:01:30 -0700 Subject: Camera3: Fix the deadlock during recording pinch zooming When zooming during recording, hal callback thread and request update thread run into deadlock due to lock circular dependency. This change release lock during queuebuffer in callback thread to break the dependency. Bug: 9091576 Change-Id: Ia7b3f0ec17573cb32a5696dcde419ca28f42cfb8 --- .../libcameraservice/camera3/Camera3OutputStream.cpp | 15 ++++++++++++--- .../camera/libcameraservice/camera3/Camera3OutputStream.h | 3 +++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/services/camera/libcameraservice/camera3/Camera3OutputStream.cpp b/services/camera/libcameraservice/camera3/Camera3OutputStream.cpp index a2c97d4..2efeede 100644 --- a/services/camera/libcameraservice/camera3/Camera3OutputStream.cpp +++ b/services/camera/libcameraservice/camera3/Camera3OutputStream.cpp @@ -166,11 +166,20 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( int anwReleaseFence = releaseFence->dup(); /** + * Release the lock briefly to avoid deadlock with + * StreamingProcessor::startStream -> Camera3Stream::isConfiguring (this + * thread will go into StreamingProcessor::onFrameAvailable) during + * queueBuffer + */ + sp currentConsumer = mConsumer; + mLock.unlock(); + + /** * Return buffer back to ANativeWindow */ if (buffer.status == CAMERA3_BUFFER_STATUS_ERROR) { // Cancel buffer - res = mConsumer->cancelBuffer(mConsumer.get(), + res = currentConsumer->cancelBuffer(currentConsumer.get(), container_of(buffer.buffer, ANativeWindowBuffer, handle), anwReleaseFence); if (res != OK) { @@ -178,7 +187,7 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( " %s (%d)", __FUNCTION__, mId, strerror(-res), res); } } else { - res = mConsumer->queueBuffer(mConsumer.get(), + res = currentConsumer->queueBuffer(currentConsumer.get(), container_of(buffer.buffer, ANativeWindowBuffer, handle), anwReleaseFence); if (res != OK) { @@ -186,7 +195,7 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( "%s (%d)", __FUNCTION__, mId, strerror(-res), res); } } - + mLock.lock(); if (res != OK) { close(anwReleaseFence); return res; diff --git a/services/camera/libcameraservice/camera3/Camera3OutputStream.h b/services/camera/libcameraservice/camera3/Camera3OutputStream.h index ce317f9..774fbdd 100644 --- a/services/camera/libcameraservice/camera3/Camera3OutputStream.h +++ b/services/camera/libcameraservice/camera3/Camera3OutputStream.h @@ -66,6 +66,9 @@ class Camera3OutputStream : Camera3OutputStream(int id, camera3_stream_type_t type, uint32_t width, uint32_t height, int format); + /** + * Note that we release the lock briefly in this function + */ virtual status_t returnBufferCheckedLocked( const camera3_stream_buffer &buffer, nsecs_t timestamp, -- cgit v1.1