From 573a270f26d063a388eb467d381a06c94bcfe7c4 Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Wed, 17 Apr 2019 10:08:55 -0700 Subject: [PATCH] Camera: clear buffer cache when stream is reconfigured Also fix a issue for finishConfiguration is unintentionally delayed till first capture request. Test: Camera CTS + partner device testing Bug: 126390310 Change-Id: Ibca740a7160cbf41e01884dbcef8ba51eb4c75f7 --- .../device3/Camera3Device.cpp | 59 ++++++++++++------- .../libcameraservice/device3/Camera3Device.h | 2 + .../device3/Camera3Stream.cpp | 11 +++- .../libcameraservice/device3/Camera3Stream.h | 4 +- .../device3/Camera3StreamInterface.h | 4 +- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp index 415b2d80a5..f4abba432d 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.cpp +++ b/services/camera/libcameraservice/device3/Camera3Device.cpp @@ -2522,6 +2522,9 @@ status_t Camera3Device::setConsumerSurfaces(int streamId, CLOGE("Stream %d is unknown", streamId); return BAD_VALUE; } + + // isConsumerConfigurationDeferred will be off after setConsumers + bool isDeferred = stream->isConsumerConfigurationDeferred(); status_t res = stream->setConsumers(consumers); if (res != OK) { CLOGE("Stream %d set consumer failed (error %d %s) ", streamId, res, strerror(-res)); @@ -2537,7 +2540,7 @@ status_t Camera3Device::setConsumerSurfaces(int streamId, surfaceIds->push_back(id); } - if (stream->isConsumerConfigurationDeferred()) { + if (isDeferred) { if (!stream->isConfiguring()) { CLOGE("Stream %d was already fully configured.", streamId); return INVALID_OPERATION; @@ -2612,7 +2615,6 @@ status_t Camera3Device::dropStreamBuffers(bool dropping, int streamId) { sp Camera3Device::createCaptureRequest( const PhysicalCameraSettingsList &request, const SurfaceMap &surfaceMap) { ATRACE_CALL(); - status_t res; sp newRequest = new CaptureRequest; newRequest->mSettingsList = request; @@ -2626,16 +2628,11 @@ sp Camera3Device::createCaptureRequest( inputStreams.data.u8[0]); return NULL; } - // Lazy completion of stream configuration (allocation/registration) - // on first use + if (mInputStream->isConfiguring()) { - res = mInputStream->finishConfiguration(); - if (res != OK) { - SET_ERR_L("Unable to finish configuring input stream %d:" - " %s (%d)", - mInputStream->getId(), strerror(-res), res); - return NULL; - } + SET_ERR_L("%s: input stream %d is not configured!", + __FUNCTION__, mInputStream->getId()); + return NULL; } // Check if stream prepare is blocking requests. if (mInputStream->isBlockedByPrepare()) { @@ -2675,15 +2672,9 @@ sp Camera3Device::createCaptureRequest( newRequest->mOutputSurfaces[streams.data.i32[i]] = surfaces; } - // Lazy completion of stream configuration (allocation/registration) - // on first use if (stream->isConfiguring()) { - res = stream->finishConfiguration(); - if (res != OK) { - SET_ERR_L("Unable to finish configuring stream %d: %s (%d)", - stream->getId(), strerror(-res), res); - return NULL; - } + SET_ERR_L("%s: stream %d is not configured!", __FUNCTION__, stream->getId()); + return NULL; } // Check if stream prepare is blocking requests. if (stream->isBlockedByPrepare()) { @@ -2908,7 +2899,8 @@ status_t Camera3Device::configureStreamsLocked(int operatingMode, // faster if (mInputStream != NULL && mInputStream->isConfiguring()) { - res = mInputStream->finishConfiguration(); + bool streamReConfigured = false; + res = mInputStream->finishConfiguration(&streamReConfigured); if (res != OK) { CLOGE("Can't finish configuring input stream %d: %s (%d)", mInputStream->getId(), strerror(-res), res); @@ -2918,12 +2910,16 @@ status_t Camera3Device::configureStreamsLocked(int operatingMode, } return BAD_VALUE; } + if (streamReConfigured) { + mInterface->onStreamReConfigured(mInputStream->getId()); + } } for (size_t i = 0; i < mOutputStreams.size(); i++) { sp outputStream = mOutputStreams[i]; if (outputStream->isConfiguring() && !outputStream->isConsumerConfigurationDeferred()) { - res = outputStream->finishConfiguration(); + bool streamReConfigured = false; + res = outputStream->finishConfiguration(&streamReConfigured); if (res != OK) { CLOGE("Can't finish configuring output stream %d: %s (%d)", outputStream->getId(), strerror(-res), res); @@ -2933,6 +2929,9 @@ status_t Camera3Device::configureStreamsLocked(int operatingMode, } return BAD_VALUE; } + if (streamReConfigured) { + mInterface->onStreamReConfigured(outputStream->getId()); + } } } @@ -4780,7 +4779,7 @@ void Camera3Device::HalInterface::onBufferFreed( __FUNCTION__, handle, streamId); return; } else { - bufferId = it->second; + bufferId = it->second; bIdMap.erase(it); ALOGV("%s: stream %d now have %zu buffer caches after removing buf %p", __FUNCTION__, streamId, bIdMap.size(), handle); @@ -4788,6 +4787,22 @@ void Camera3Device::HalInterface::onBufferFreed( mFreedBuffers.push_back(std::make_pair(streamId, bufferId)); } +void Camera3Device::HalInterface::onStreamReConfigured(int streamId) { + std::lock_guard lock(mBufferIdMapLock); + auto mapIt = mBufferIdMaps.find(streamId); + if (mapIt == mBufferIdMaps.end()) { + ALOGE("%s: streamId %d not found!", __FUNCTION__, streamId); + return; + } + + BufferIdMap& bIdMap = mapIt->second; + for (const auto& it : bIdMap) { + uint64_t bufferId = it.second; + mFreedBuffers.push_back(std::make_pair(streamId, bufferId)); + } + bIdMap.clear(); +} + /** * RequestThread inner class methods */ diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h index f8245df821..23df3c739d 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.h +++ b/services/camera/libcameraservice/device3/Camera3Device.h @@ -336,6 +336,8 @@ class Camera3Device : // Get a vector of bufferId of currently inflight buffers void getInflightRequestBufferKeys(std::vector* out); + void onStreamReConfigured(int streamId); + static const uint64_t BUFFER_ID_NO_BUFFER = 0; private: // Always valid diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp index 12ff1308da..d73a2f985d 100644 --- a/services/camera/libcameraservice/device3/Camera3Stream.cpp +++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp @@ -287,8 +287,11 @@ bool Camera3Stream::isConfiguring() const { return (mState == STATE_IN_CONFIG) || (mState == STATE_IN_RECONFIG); } -status_t Camera3Stream::finishConfiguration() { +status_t Camera3Stream::finishConfiguration(/*out*/bool* streamReconfigured) { ATRACE_CALL(); + if (streamReconfigured != nullptr) { + *streamReconfigured = false; + } Mutex::Autolock l(mLock); switch (mState) { case STATE_ERROR: @@ -313,7 +316,7 @@ status_t Camera3Stream::finishConfiguration() { // Register for idle tracking sp statusTracker = mStatusTracker.promote(); - if (statusTracker != 0) { + if (statusTracker != 0 && mStatusId == StatusTracker::NO_STATUS_ID) { mStatusId = statusTracker->addComponent(); } @@ -332,6 +335,7 @@ status_t Camera3Stream::finishConfiguration() { mPrepareBlockRequest = true; mStreamUnpreparable = false; + bool reconfiguring = (mState == STATE_IN_RECONFIG); status_t res; res = configureQueueLocked(); // configureQueueLocked could return error in case of abandoned surface. @@ -348,6 +352,9 @@ status_t Camera3Stream::finishConfiguration() { return res; } + if (reconfiguring && streamReconfigured != nullptr) { + *streamReconfigured = true; + } mState = STATE_CONFIGURED; return res; diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h index 3d21029892..c916fe8a01 100644 --- a/services/camera/libcameraservice/device3/Camera3Stream.h +++ b/services/camera/libcameraservice/device3/Camera3Stream.h @@ -197,6 +197,8 @@ class Camera3Stream : * after this call, but can still be read until the destruction of the * stream. * + * streamReconfigured: set to true when a stream is being reconfigured. + * * Returns: * OK on a successful configuration * NO_INIT in case of a serious error from the HAL device @@ -204,7 +206,7 @@ class Camera3Stream : * INVALID_OPERATION in case connecting to the consumer failed or consumer * doesn't exist yet. */ - status_t finishConfiguration(); + status_t finishConfiguration(/*out*/bool* streamReconfigured = nullptr); /** * Cancels the stream configuration process. This returns the stream to the diff --git a/services/camera/libcameraservice/device3/Camera3StreamInterface.h b/services/camera/libcameraservice/device3/Camera3StreamInterface.h index 5cd11b7c70..73f501a57b 100644 --- a/services/camera/libcameraservice/device3/Camera3StreamInterface.h +++ b/services/camera/libcameraservice/device3/Camera3StreamInterface.h @@ -130,13 +130,15 @@ class Camera3StreamInterface : public virtual RefBase { * modified after this call, but can still be read until the destruction of * the stream. * + * streamReconfigured: set to true when a stream is being reconfigured. + * * Returns: * OK on a successful configuration * NO_INIT in case of a serious error from the HAL device * NO_MEMORY in case of an error registering buffers * INVALID_OPERATION in case connecting to the consumer failed */ - virtual status_t finishConfiguration() = 0; + virtual status_t finishConfiguration(/*out*/bool* streamReconfigured = nullptr) = 0; /** * Cancels the stream configuration process. This returns the stream to the