From cc0b795ac29688e1675d177e60eb4f37184240c5 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Tue, 7 Jan 2020 13:54:47 -0800 Subject: [PATCH] Camera: Propagate all offline stream ids to clients Camera clients must be aware of any configured streams that can support offline processing mode. A few corner cases that need to be considered: - Composite streams can support offline mode only when all internal streams support it as well. - Streams that use the internal camera buffer manager will not have support for offline mode. - Shared streams are also unsupported in offline mode. Bug: 135142453 Test: Camera CTS Change-Id: Idde826a6fb18a8907850e87cfe593de7cb1c5f4a --- .../hardware/camera2/ICameraDeviceUser.aidl | 3 +- camera/ndk/impl/ACameraDevice.cpp | 3 +- camera/tests/CameraBinderTests.cpp | 7 ++- .../api2/CameraDeviceClient.cpp | 62 ++++++++++++++++--- .../api2/CameraDeviceClient.h | 4 +- .../common/CameraDeviceBase.h | 6 ++ .../device3/Camera3Device.cpp | 26 ++++++++ .../libcameraservice/device3/Camera3Device.h | 2 + .../device3/Camera3SharedOutputStream.h | 6 ++ .../device3/Camera3StreamInterface.h | 1 + .../hidl/HidlCameraDeviceUser.cpp | 3 +- 11 files changed, 108 insertions(+), 15 deletions(-) diff --git a/camera/aidl/android/hardware/camera2/ICameraDeviceUser.aidl b/camera/aidl/android/hardware/camera2/ICameraDeviceUser.aidl index a442a91a40..b183ccc822 100644 --- a/camera/aidl/android/hardware/camera2/ICameraDeviceUser.aidl +++ b/camera/aidl/android/hardware/camera2/ICameraDeviceUser.aidl @@ -83,8 +83,9 @@ interface ICameraDeviceUser * @param operatingMode The kind of session to create; either NORMAL_MODE or * CONSTRAINED_HIGH_SPEED_MODE. Must be a non-negative value. * @param sessionParams Session wide camera parameters + * @return a list of stream ids that can be used in offline mode via "switchToOffline" */ - void endConfigure(int operatingMode, in CameraMetadataNative sessionParams); + int[] endConfigure(int operatingMode, in CameraMetadataNative sessionParams); /** * Check whether a particular session configuration has camera device diff --git a/camera/ndk/impl/ACameraDevice.cpp b/camera/ndk/impl/ACameraDevice.cpp index 46a8dae317..0d7180abb5 100644 --- a/camera/ndk/impl/ACameraDevice.cpp +++ b/camera/ndk/impl/ACameraDevice.cpp @@ -710,7 +710,8 @@ CameraDevice::configureStreamsLocked(const ACaptureSessionOutputContainer* outpu if ((sessionParameters != nullptr) && (sessionParameters->settings != nullptr)) { params.append(sessionParameters->settings->getInternalData()); } - remoteRet = mRemote->endConfigure(/*isConstrainedHighSpeed*/ false, params); + std::vector offlineStreamIds; + remoteRet = mRemote->endConfigure(/*isConstrainedHighSpeed*/ false, params, &offlineStreamIds); if (remoteRet.serviceSpecificErrorCode() == hardware::ICameraService::ERROR_ILLEGAL_ARGUMENT) { ALOGE("Camera device %s cannnot support app output configuration: %s", getId(), remoteRet.toString8().string()); diff --git a/camera/tests/CameraBinderTests.cpp b/camera/tests/CameraBinderTests.cpp index 9a18b10222..cd5bdd1cb9 100644 --- a/camera/tests/CameraBinderTests.cpp +++ b/camera/tests/CameraBinderTests.cpp @@ -498,7 +498,9 @@ TEST_F(CameraClientBinderTest, CheckBinderCameraDeviceUser) { EXPECT_TRUE(res.isOk()) << res; EXPECT_LE(0, streamId); CameraMetadata sessionParams; - res = device->endConfigure(/*isConstrainedHighSpeed*/ false, sessionParams); + std::vector offlineStreamIds; + res = device->endConfigure(/*isConstrainedHighSpeed*/ false, sessionParams, + &offlineStreamIds); EXPECT_TRUE(res.isOk()) << res; EXPECT_FALSE(callbacks->hadError()); @@ -609,7 +611,8 @@ TEST_F(CameraClientBinderTest, CheckBinderCameraDeviceUser) { EXPECT_TRUE(res.isOk()) << res; res = device->deleteStream(streamId); EXPECT_TRUE(res.isOk()) << res; - res = device->endConfigure(/*isConstrainedHighSpeed*/ false, sessionParams); + res = device->endConfigure(/*isConstrainedHighSpeed*/ false, sessionParams, + &offlineStreamIds); EXPECT_TRUE(res.isOk()) << res; sleep(/*second*/1); // allow some time for errors to show up, if any diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp index ba17eb6559..9deb662aeb 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp @@ -470,7 +470,8 @@ binder::Status CameraDeviceClient::beginConfigure() { } binder::Status CameraDeviceClient::endConfigure(int operatingMode, - const hardware::camera2::impl::CameraMetadataNative& sessionParams) { + const hardware::camera2::impl::CameraMetadataNative& sessionParams, + std::vector* offlineStreamIds /*out*/) { ATRACE_CALL(); ALOGV("%s: ending configure (%d input stream, %zu output surfaces)", __FUNCTION__, mInputStream.configured ? 1 : 0, @@ -479,6 +480,12 @@ binder::Status CameraDeviceClient::endConfigure(int operatingMode, binder::Status res; if (!(res = checkPidStatus(__FUNCTION__)).isOk()) return res; + if (offlineStreamIds == nullptr) { + String8 msg = String8::format("Invalid offline stream ids"); + ALOGE("%s: %s", __FUNCTION__, msg.string()); + return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.string()); + } + Mutex::Autolock icl(mBinderSerializationLock); if (!mDevice.get()) { @@ -502,15 +509,41 @@ binder::Status CameraDeviceClient::endConfigure(int operatingMode, ALOGE("%s: %s", __FUNCTION__, msg.string()); res = STATUS_ERROR(CameraService::ERROR_INVALID_OPERATION, msg.string()); } else { + offlineStreamIds->clear(); + mDevice->getOfflineStreamIds(offlineStreamIds); + for (size_t i = 0; i < mCompositeStreamMap.size(); ++i) { err = mCompositeStreamMap.valueAt(i)->configureStream(); - if (err != OK ) { + if (err != OK) { String8 msg = String8::format("Camera %s: Error configuring composite " "streams: %s (%d)", mCameraIdStr.string(), strerror(-err), err); ALOGE("%s: %s", __FUNCTION__, msg.string()); res = STATUS_ERROR(CameraService::ERROR_INVALID_OPERATION, msg.string()); break; } + + // Composite streams can only support offline mode in case all individual internal + // streams are also supported. + std::vector internalStreams; + mCompositeStreamMap.valueAt(i)->insertCompositeStreamIds(&internalStreams); + std::remove_if(offlineStreamIds->begin(), offlineStreamIds->end(), + [&internalStreams] (int streamId) { + auto it = std::find(internalStreams.begin(), internalStreams.end(), + streamId); + if (it != internalStreams.end()) { + internalStreams.erase(it); + return true; + } + + return false; + }); + if (internalStreams.empty()) { + offlineStreamIds->push_back(mCompositeStreamMap.valueAt(i)->getStreamId()); + } + } + + for (const auto& offlineStreamId : *offlineStreamIds) { + mStreamInfoMap[offlineStreamId].supportsOffline = true; } } @@ -1918,7 +1951,7 @@ binder::Status CameraDeviceClient::switchToOffline( } if (offlineOutputIds.empty()) { - String8 msg = String8::format("Offline outputs must not be empty"); + String8 msg = String8::format("Offline surfaces must not be empty"); ALOGE("%s: %s", __FUNCTION__, msg.string()); return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.string()); } @@ -1934,11 +1967,18 @@ binder::Status CameraDeviceClient::switchToOffline( for (const auto& streamId : offlineOutputIds) { ssize_t index = mConfiguredOutputs.indexOfKey(streamId); if (index == NAME_NOT_FOUND) { - String8 msg = String8::format("Offline output is invalid"); + String8 msg = String8::format("Offline surface with id: %d is not registered", + streamId); + ALOGE("%s: %s", __FUNCTION__, msg.string()); + return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.string()); + } + + if (!mStreamInfoMap[streamId].supportsOffline) { + String8 msg = String8::format("Offline surface with id: %d doesn't support " + "offline mode", streamId); ALOGE("%s: %s", __FUNCTION__, msg.string()); return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.string()); } - // TODO: Also check whether the offline output is supported by Hal for offline mode. bool isCompositeStream = false; for (const auto& gbp : mConfiguredOutputs[streamId].getGraphicBufferProducers()) { @@ -1974,10 +2014,14 @@ binder::Status CameraDeviceClient::switchToOffline( mCameraIdStr.string(), strerror(ret), ret); } - sp offlineClient = new CameraOfflineSessionClient(sCameraService, - offlineSession, offlineCompositeStreamMap, cameraCb, mClientPackageName, - mClientFeatureId, mCameraIdStr, mCameraFacing, mClientPid, mClientUid, mServicePid); - ret = sCameraService->addOfflineClient(mCameraIdStr, offlineClient); + sp offlineClient; + if (offlineSession.get() != nullptr) { + offlineClient = new CameraOfflineSessionClient(sCameraService, + offlineSession, offlineCompositeStreamMap, cameraCb, mClientPackageName, + mClientFeatureId, mCameraIdStr, mCameraFacing, mClientPid, mClientUid, mServicePid); + ret = sCameraService->addOfflineClient(mCameraIdStr, offlineClient); + } + if (ret == OK) { // A successful offline session switch must reset the current camera client // and release any resources occupied by previously configured streams. diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h index 295ab29761..a3a3189b88 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.h +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h @@ -92,7 +92,9 @@ public: virtual binder::Status beginConfigure() override; virtual binder::Status endConfigure(int operatingMode, - const hardware::camera2::impl::CameraMetadataNative& sessionParams) override; + const hardware::camera2::impl::CameraMetadataNative& sessionParams, + /*out*/ + std::vector* offlineStreamIds) override; // Verify specific session configuration. virtual binder::Status isSessionConfigurationSupported( diff --git a/services/camera/libcameraservice/common/CameraDeviceBase.h b/services/camera/libcameraservice/common/CameraDeviceBase.h index 9e5fbe8db1..2f011984f1 100644 --- a/services/camera/libcameraservice/common/CameraDeviceBase.h +++ b/services/camera/libcameraservice/common/CameraDeviceBase.h @@ -234,6 +234,12 @@ class CameraDeviceBase : public virtual RefBase { virtual status_t configureStreams(const CameraMetadata& sessionParams, int operatingMode = 0) = 0; + /** + * Retrieve a list of all stream ids that were advertised as capable of + * supporting offline processing mode by Hal after the last stream configuration. + */ + virtual void getOfflineStreamIds(std::vector *offlineStreamIds) = 0; + // get the buffer producer of the input stream virtual status_t getInputBufferProducer( sp *producer) = 0; diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp index 0385a6f63e..23e26ce65a 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.cpp +++ b/services/camera/libcameraservice/device3/Camera3Device.cpp @@ -5863,4 +5863,30 @@ status_t Camera3Device::switchToOffline( // Java side to make sure the CameraCaptureSession is properly closed } +void Camera3Device::getOfflineStreamIds(std::vector *offlineStreamIds) { + ATRACE_CALL(); + + if (offlineStreamIds == nullptr) { + return; + } + + Mutex::Autolock il(mInterfaceLock); + + auto streamIds = mOutputStreams.getStreamIds(); + bool hasInputStream = mInputStream != nullptr; + if (hasInputStream && mInputStream->getOfflineProcessingSupport()) { + offlineStreamIds->push_back(mInputStream->getId()); + } + + for (const auto & streamId : streamIds) { + sp stream = mOutputStreams.get(streamId); + // Streams that use the camera buffer manager are currently not supported in + // offline mode + if (stream->getOfflineProcessingSupport() && + (stream->getStreamSetId() == CAMERA3_STREAM_SET_ID_INVALID)) { + offlineStreamIds->push_back(streamId); + } + } +} + }; // namespace android diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h index 6810fea6c2..7279baf5a5 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.h +++ b/services/camera/libcameraservice/device3/Camera3Device.h @@ -150,6 +150,8 @@ class Camera3Device : status_t getInputBufferProducer( sp *producer) override; + void getOfflineStreamIds(std::vector *offlineStreamIds) override; + status_t createDefaultRequest(int templateId, CameraMetadata *request) override; // Transitions to the idle state on success diff --git a/services/camera/libcameraservice/device3/Camera3SharedOutputStream.h b/services/camera/libcameraservice/device3/Camera3SharedOutputStream.h index b5e37c2ffd..e645e05e38 100644 --- a/services/camera/libcameraservice/device3/Camera3SharedOutputStream.h +++ b/services/camera/libcameraservice/device3/Camera3SharedOutputStream.h @@ -65,6 +65,12 @@ public: const std::vector &removedSurfaceIds, KeyedVector, size_t> *outputMap/*out*/); + virtual bool getOfflineProcessingSupport() const { + // As per Camera spec. shared streams currently do not support + // offline mode. + return false; + } + private: static const size_t kMaxOutputs = 4; diff --git a/services/camera/libcameraservice/device3/Camera3StreamInterface.h b/services/camera/libcameraservice/device3/Camera3StreamInterface.h index 9d3e50fab6..667e3bb36e 100644 --- a/services/camera/libcameraservice/device3/Camera3StreamInterface.h +++ b/services/camera/libcameraservice/device3/Camera3StreamInterface.h @@ -55,6 +55,7 @@ class OutputStreamInfo { android_dataspace dataSpace; uint64_t consumerUsage; bool finalized = false; + bool supportsOffline = false; OutputStreamInfo() : width(-1), height(-1), format(-1), dataSpace(HAL_DATASPACE_UNKNOWN), consumerUsage(0) {} diff --git a/services/camera/libcameraservice/hidl/HidlCameraDeviceUser.cpp b/services/camera/libcameraservice/hidl/HidlCameraDeviceUser.cpp index 675ad24bcf..bf89ca5a7b 100644 --- a/services/camera/libcameraservice/hidl/HidlCameraDeviceUser.cpp +++ b/services/camera/libcameraservice/hidl/HidlCameraDeviceUser.cpp @@ -201,8 +201,9 @@ Return HidlCameraDeviceUser::endConfigure(StreamConfigurationMode opera return HStatus::ILLEGAL_ARGUMENT; } + std::vector offlineStreamIds; binder::Status ret = mDeviceRemote->endConfigure(convertFromHidl(operatingMode), - cameraMetadata); + cameraMetadata, &offlineStreamIds); return B2HStatus(ret); }