From 81d754306ecd4a587459015da5168270c2a5c167 Mon Sep 17 00:00:00 2001 From: Zhijun He Date: Tue, 26 Nov 2013 15:11:05 -0800 Subject: DO NOT MERGE: camera2/3: Add protection for still capture path Jpeg stream in JpegProcessor could be deleted while process new capture is ongoing, which unsafe to access a dead consumer endpoint. Bug: 9316454 Change-Id: I2950f31ea28d0ba01f08502e2e3ba452bf8bb818 --- .../api1/client2/JpegProcessor.cpp | 79 ++++++++++++---------- 1 file changed, 44 insertions(+), 35 deletions(-) (limited to 'services') diff --git a/services/camera/libcameraservice/api1/client2/JpegProcessor.cpp b/services/camera/libcameraservice/api1/client2/JpegProcessor.cpp index 77d5c8a..ec81456 100644 --- a/services/camera/libcameraservice/api1/client2/JpegProcessor.cpp +++ b/services/camera/libcameraservice/api1/client2/JpegProcessor.cpp @@ -200,50 +200,59 @@ status_t JpegProcessor::processNewCapture() { ATRACE_CALL(); status_t res; sp captureHeap; + sp captureBuffer; CpuConsumer::LockedBuffer imgBuffer; - res = mCaptureConsumer->lockNextBuffer(&imgBuffer); - if (res != OK) { - if (res != BAD_VALUE) { - ALOGE("%s: Camera %d: Error receiving still image buffer: " - "%s (%d)", __FUNCTION__, - mId, strerror(-res), res); + { + Mutex::Autolock l(mInputMutex); + if (mCaptureStreamId == NO_STREAM) { + ALOGW("%s: Camera %d: No stream is available", __FUNCTION__, mId); + return INVALID_OPERATION; } - return res; - } - ALOGV("%s: Camera %d: Still capture available", __FUNCTION__, - mId); + res = mCaptureConsumer->lockNextBuffer(&imgBuffer); + if (res != OK) { + if (res != BAD_VALUE) { + ALOGE("%s: Camera %d: Error receiving still image buffer: " + "%s (%d)", __FUNCTION__, + mId, strerror(-res), res); + } + return res; + } - if (imgBuffer.format != HAL_PIXEL_FORMAT_BLOB) { - ALOGE("%s: Camera %d: Unexpected format for still image: " - "%x, expected %x", __FUNCTION__, mId, - imgBuffer.format, - HAL_PIXEL_FORMAT_BLOB); - mCaptureConsumer->unlockBuffer(imgBuffer); - return OK; - } + ALOGV("%s: Camera %d: Still capture available", __FUNCTION__, + mId); - // Find size of JPEG image - size_t jpegSize = findJpegSize(imgBuffer.data, imgBuffer.width); - if (jpegSize == 0) { // failed to find size, default to whole buffer - jpegSize = imgBuffer.width; - } - size_t heapSize = mCaptureHeap->getSize(); - if (jpegSize > heapSize) { - ALOGW("%s: JPEG image is larger than expected, truncating " - "(got %d, expected at most %d bytes)", - __FUNCTION__, jpegSize, heapSize); - jpegSize = heapSize; - } + if (imgBuffer.format != HAL_PIXEL_FORMAT_BLOB) { + ALOGE("%s: Camera %d: Unexpected format for still image: " + "%x, expected %x", __FUNCTION__, mId, + imgBuffer.format, + HAL_PIXEL_FORMAT_BLOB); + mCaptureConsumer->unlockBuffer(imgBuffer); + return OK; + } - // TODO: Optimize this to avoid memcopy - sp captureBuffer = new MemoryBase(mCaptureHeap, 0, jpegSize); - void* captureMemory = mCaptureHeap->getBase(); - memcpy(captureMemory, imgBuffer.data, jpegSize); + // Find size of JPEG image + size_t jpegSize = findJpegSize(imgBuffer.data, imgBuffer.width); + if (jpegSize == 0) { // failed to find size, default to whole buffer + jpegSize = imgBuffer.width; + } + size_t heapSize = mCaptureHeap->getSize(); + if (jpegSize > heapSize) { + ALOGW("%s: JPEG image is larger than expected, truncating " + "(got %d, expected at most %d bytes)", + __FUNCTION__, jpegSize, heapSize); + jpegSize = heapSize; + } + + // TODO: Optimize this to avoid memcopy + captureBuffer = new MemoryBase(mCaptureHeap, 0, jpegSize); + void* captureMemory = mCaptureHeap->getBase(); + memcpy(captureMemory, imgBuffer.data, jpegSize); - mCaptureConsumer->unlockBuffer(imgBuffer); + mCaptureConsumer->unlockBuffer(imgBuffer); + } sp sequencer = mSequencer.promote(); if (sequencer != 0) { -- cgit v1.1 From 6706009fa8294c8cdab1cdab4585f00d42df483e Mon Sep 17 00:00:00 2001 From: Ruben Brunk Date: Thu, 5 Dec 2013 11:00:37 -0800 Subject: DO NOT MERGE: camera2: Fix race with stream deletion during disconnect. Bug: 11856804 - Shutdown order in Camera2Client allows a stream to be deleted before the corresponding processing thread has quit. This can result in updates being called on the processor thread without a valid stream. Change-Id: Ie4e649771f4321498659211f2a37ed89a6d956c4 --- services/camera/libcameraservice/api1/Camera2Client.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'services') diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp index 0b6ca5c..abcbd06 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.cpp +++ b/services/camera/libcameraservice/api1/Camera2Client.cpp @@ -406,12 +406,6 @@ void Camera2Client::disconnect() { l.mParameters.state = Parameters::DISCONNECTED; } - mStreamingProcessor->deletePreviewStream(); - mStreamingProcessor->deleteRecordingStream(); - mJpegProcessor->deleteStream(); - mCallbackProcessor->deleteStream(); - mZslProcessor->deleteStream(); - mStreamingProcessor->requestExit(); mFrameProcessor->requestExit(); mCaptureSequencer->requestExit(); @@ -428,6 +422,14 @@ void Camera2Client::disconnect() { mZslProcessorThread->join(); mCallbackProcessor->join(); + ALOGV("Camera %d: Deleting streams", mCameraId); + + mStreamingProcessor->deletePreviewStream(); + mStreamingProcessor->deleteRecordingStream(); + mJpegProcessor->deleteStream(); + mCallbackProcessor->deleteStream(); + mZslProcessor->deleteStream(); + ALOGV("Camera %d: Disconnecting device", mCameraId); mDevice->disconnect(); -- cgit v1.1 From a413dd621966044753a8fa1f57c76d847b6f4bec Mon Sep 17 00:00:00 2001 From: Zhijun He Date: Wed, 29 Jan 2014 08:52:01 -0800 Subject: DO NOT MERGE: Camera: fix focusArea wrong indexing issue Bug: 12304559 Change-Id: Id28b35fdd9697c1ec3365f617996801965de8bd0 --- .../libcameraservice/api1/client2/Parameters.cpp | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'services') diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp index 08af566..5196e09 100644 --- a/services/camera/libcameraservice/api1/client2/Parameters.cpp +++ b/services/camera/libcameraservice/api1/client2/Parameters.cpp @@ -1858,23 +1858,23 @@ status_t Parameters::updateRequest(CameraMetadata *request) const { size_t reqFocusingAreasSize = focusingAreas.size() * 5; int32_t *reqFocusingAreas = new int32_t[reqFocusingAreasSize]; - for (size_t i = 0; i < reqFocusingAreasSize; i += 5) { - if (focusingAreas[i].weight != 0) { + for (size_t i = 0, j = 0; i < reqFocusingAreasSize; i += 5, j++) { + if (focusingAreas[j].weight != 0) { reqFocusingAreas[i + 0] = - normalizedXToArray(focusingAreas[i].left); + normalizedXToArray(focusingAreas[j].left); reqFocusingAreas[i + 1] = - normalizedYToArray(focusingAreas[i].top); + normalizedYToArray(focusingAreas[j].top); reqFocusingAreas[i + 2] = - normalizedXToArray(focusingAreas[i].right); + normalizedXToArray(focusingAreas[j].right); reqFocusingAreas[i + 3] = - normalizedYToArray(focusingAreas[i].bottom); + normalizedYToArray(focusingAreas[j].bottom); } else { reqFocusingAreas[i + 0] = 0; reqFocusingAreas[i + 1] = 0; reqFocusingAreas[i + 2] = 0; reqFocusingAreas[i + 3] = 0; } - reqFocusingAreas[i + 4] = focusingAreas[i].weight; + reqFocusingAreas[i + 4] = focusingAreas[j].weight; } res = request->update(ANDROID_CONTROL_AF_REGIONS, reqFocusingAreas, reqFocusingAreasSize); @@ -1887,23 +1887,23 @@ status_t Parameters::updateRequest(CameraMetadata *request) const { size_t reqMeteringAreasSize = meteringAreas.size() * 5; int32_t *reqMeteringAreas = new int32_t[reqMeteringAreasSize]; - for (size_t i = 0; i < reqMeteringAreasSize; i += 5) { - if (meteringAreas[i].weight != 0) { + for (size_t i = 0, j = 0; i < reqMeteringAreasSize; i += 5, j++) { + if (meteringAreas[j].weight != 0) { reqMeteringAreas[i + 0] = - normalizedXToArray(meteringAreas[i].left); + normalizedXToArray(meteringAreas[j].left); reqMeteringAreas[i + 1] = - normalizedYToArray(meteringAreas[i].top); + normalizedYToArray(meteringAreas[j].top); reqMeteringAreas[i + 2] = - normalizedXToArray(meteringAreas[i].right); + normalizedXToArray(meteringAreas[j].right); reqMeteringAreas[i + 3] = - normalizedYToArray(meteringAreas[i].bottom); + normalizedYToArray(meteringAreas[j].bottom); } else { reqMeteringAreas[i + 0] = 0; reqMeteringAreas[i + 1] = 0; reqMeteringAreas[i + 2] = 0; reqMeteringAreas[i + 3] = 0; } - reqMeteringAreas[i + 4] = meteringAreas[i].weight; + reqMeteringAreas[i + 4] = meteringAreas[j].weight; } res = request->update(ANDROID_CONTROL_AE_REGIONS, reqMeteringAreas, reqMeteringAreasSize); -- cgit v1.1