camera: delay initialization of lazy camera providers
Camera HALs can be declared as "lazy" which lets cameraservice
only bring them up as needed. However, due to a series of independent
changes, cameraservice was unconditionally bringing up all AIDL HALs
including lazy HALs at startup and keeping them alive throughout its
lifetime.
This CL defers the lazy HAL initialization to when needed, and caches
the remote object in a weak_ptr to allow it to be garbage collected
when not in use. This is the same logic that was used before, and what
HIDL uses to handle lazy HALs.
Bug: 319735068
Test: atest cameraservice_test passes
Test: atest virtual_camera_test passes
Test: Manually that virtual_camera which is registered as a lazy
Provider is not created until camera service is forced to
run some operation on it.
Change-Id: I8f6bc7486843065984c4c572aaf15d33205c4a3a
diff --git a/camera/camera_platform.aconfig b/camera/camera_platform.aconfig
index 076394d..63a514d 100644
--- a/camera/camera_platform.aconfig
+++ b/camera/camera_platform.aconfig
@@ -83,3 +83,10 @@
description: "Enable using ro.board.api_level instead of ro.vndk.version to get VNDK version"
bug: "312315580"
}
+
+flag {
+ namespace: "camera_platform"
+ name: "delay_lazy_hal_instantiation"
+ description: "Only trigger lazy HAL instantiation when the HAL is needed for an operation."
+ bug: "319735068"
+}
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index eaf6faa..70110bd 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -139,7 +139,7 @@
}
std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
-CameraProviderManager::AidlServiceInteractionProxyImpl::getAidlService(
+CameraProviderManager::AidlServiceInteractionProxyImpl::getService(
const std::string& serviceName) {
using aidl::android::hardware::camera::provider::ICameraProvider;
@@ -151,15 +151,31 @@
}
if (binder == nullptr) {
- ALOGD("%s: AIDL Camera provider HAL '%s' is not actually available", __FUNCTION__,
- serviceName.c_str());
+ ALOGE("%s: AIDL Camera provider HAL '%s' is not actually available, despite waiting "
+ "indefinitely?", __FUNCTION__, serviceName.c_str());
return nullptr;
}
std::shared_ptr<ICameraProvider> interface =
ICameraProvider::fromBinder(ndk::SpAIBinder(binder));
return interface;
-};
+}
+
+std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
+CameraProviderManager::AidlServiceInteractionProxyImpl::tryGetService(
+ const std::string& serviceName) {
+ using aidl::android::hardware::camera::provider::ICameraProvider;
+
+ std::shared_ptr<ICameraProvider> interface = ICameraProvider::fromBinder(
+ ndk::SpAIBinder(AServiceManager_checkService(serviceName.c_str())));
+ if (interface == nullptr) {
+ ALOGD("%s: AIDL Camera provider HAL '%s' is not actually available", __FUNCTION__,
+ serviceName.c_str());
+ return nullptr;
+ }
+
+ return interface;
+}
static std::string getFullAidlProviderName(const std::string instance) {
std::string aidlHalServiceDescriptor =
@@ -2101,8 +2117,14 @@
const std::string& providerName, const sp<ProviderInfo>& providerInfo) {
using aidl::android::hardware::camera::provider::ICameraProvider;
- std::shared_ptr<ICameraProvider> interface =
- mAidlServiceProxy->getAidlService(providerName.c_str());
+ std::shared_ptr<ICameraProvider> interface;
+ if (flags::delay_lazy_hal_instantiation()) {
+ // Only get remote instance if already running. Lazy Providers will be
+ // woken up later.
+ interface = mAidlServiceProxy->tryGetService(providerName);
+ } else {
+ interface = mAidlServiceProxy->getService(providerName);
+ }
if (interface == nullptr) {
ALOGW("%s: AIDL Camera provider HAL '%s' is not actually available", __FUNCTION__,
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index 53a2102..fc0172c 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -178,9 +178,15 @@
// Proxy to inject fake services in test.
class AidlServiceInteractionProxy {
public:
- // Returns the Aidl service with the given serviceName
+ // Returns the Aidl service with the given serviceName. Will wait indefinitely
+ // for the service to come up if not running.
virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
- getAidlService(const std::string& serviceName) = 0;
+ getService(const std::string& serviceName) = 0;
+
+ // Attempts to get an already running AIDL service of the given serviceName.
+ // Returns nullptr immediately if service is not running.
+ virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
+ tryGetService(const std::string& serviceName) = 0;
virtual ~AidlServiceInteractionProxy() = default;
};
@@ -190,7 +196,10 @@
class AidlServiceInteractionProxyImpl : public AidlServiceInteractionProxy {
public:
virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
- getAidlService(const std::string& serviceName) override;
+ getService(const std::string& serviceName) override;
+
+ virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
+ tryGetService(const std::string& serviceName) override;
};
/**
diff --git a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
index 921ee43..a721d28 100644
--- a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
+++ b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
@@ -297,9 +297,10 @@
}
ALOGV("Camera provider actually needs restart, calling getService(%s)", mProviderName.c_str());
- interface = mManager->mAidlServiceProxy->getAidlService(mProviderName.c_str());
+ interface = mManager->mAidlServiceProxy->getService(mProviderName);
+
if (interface == nullptr) {
- ALOGD("%s: %s service not started", __FUNCTION__, mProviderName.c_str());
+ ALOGE("%s: %s service not started", __FUNCTION__, mProviderName.c_str());
return nullptr;
}
diff --git a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
index a53d26d..939126c 100644
--- a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
+++ b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp
@@ -372,28 +372,22 @@
};
/**
- * Simple test version of the interaction proxy, to use to inject onRegistered calls to the
+ * Simple test version of HidlServiceInteractionProxy, to use to inject onRegistered calls to the
* CameraProviderManager
*/
-struct TestInteractionProxy : public CameraProviderManager::HidlServiceInteractionProxy,
- public CameraProviderManager::AidlServiceInteractionProxy {
+struct TestHidlInteractionProxy : public CameraProviderManager::HidlServiceInteractionProxy {
sp<hidl::manager::V1_0::IServiceNotification> mManagerNotificationInterface;
sp<TestICameraProvider> mTestCameraProvider;
- std::shared_ptr<TestAidlICameraProvider> mTestAidlCameraProvider;
- TestInteractionProxy() {}
+ TestHidlInteractionProxy() {}
void setProvider(sp<TestICameraProvider> provider) {
mTestCameraProvider = provider;
}
- void setAidlProvider(std::shared_ptr<TestAidlICameraProvider> provider) {
- mTestAidlCameraProvider = provider;
- }
-
std::vector<std::string> mLastRequestedServiceNames;
- virtual ~TestInteractionProxy() {}
+ virtual ~TestHidlInteractionProxy() {}
virtual bool registerForNotifications(
[[maybe_unused]] const std::string &serviceName,
@@ -430,9 +424,47 @@
hardware::hidl_vec<hardware::hidl_string> ret = {"test/0"};
return ret;
}
+};
+
+/**
+ * Simple test version of AidlServiceInteractionProxy, to use to inject onRegistered calls to the
+ * CameraProviderManager
+ */
+struct TestAidlInteractionProxy : public CameraProviderManager::AidlServiceInteractionProxy {
+ std::shared_ptr<TestAidlICameraProvider> mTestAidlCameraProvider;
+
+ TestAidlInteractionProxy() {}
+
+ void setProvider(std::shared_ptr<TestAidlICameraProvider> provider) {
+ mTestAidlCameraProvider = provider;
+ }
+
+ std::vector<std::string> mLastRequestedServiceNames;
+
+ virtual ~TestAidlInteractionProxy() {}
virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
- getAidlService(const std::string&) {
+ getService(const std::string& serviceName) override {
+ if (!flags::delay_lazy_hal_instantiation()) {
+ return mTestAidlCameraProvider;
+ }
+
+ // If no provider has been given, fail; in reality, getService would
+ // block for HALs that don't start correctly, so we should never use
+ // getService when we don't have a valid HAL running
+ if (mTestAidlCameraProvider == nullptr) {
+ ADD_FAILURE() << __FUNCTION__ << "called with no valid provider;"
+ << " would block indefinitely";
+ // Real getService would block, but that's bad in unit tests. So
+ // just record an error and return nullptr
+ return nullptr;
+ }
+ mLastRequestedServiceNames.push_back(serviceName);
+ return mTestAidlCameraProvider;
+ }
+
+ virtual std::shared_ptr<aidl::android::hardware::camera::provider::ICameraProvider>
+ tryGetService(const std::string&) override {
return mTestAidlCameraProvider;
}
};
@@ -462,7 +494,7 @@
status_t res;
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
android::hardware::hidl_vec<uint8_t> chars;
CameraMetadata meta;
@@ -510,7 +542,7 @@
status_t res;
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
vendorSection);
serviceProxy.setProvider(provider);
@@ -560,7 +592,7 @@
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
vendorSection);
@@ -696,7 +728,7 @@
status_t res;
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
vendorSection);
serviceProxy.setProvider(provider);
@@ -730,7 +762,7 @@
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
vendorSection);
@@ -779,7 +811,7 @@
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
vendorSection);
@@ -821,7 +853,7 @@
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestHidlInteractionProxy serviceProxy;
android::hardware::hidl_vec<uint8_t> chars;
CameraMetadata meta;
@@ -857,9 +889,11 @@
REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(vd_flags, virtual_camera_service_discovery))) {
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestAidlInteractionProxy aidlServiceProxy;
+ TestHidlInteractionProxy hidlServiceProxy;
- status_t res = providerManager->initialize(statusListener, &serviceProxy, &serviceProxy);
+ status_t res = providerManager->initialize(statusListener,
+ &hidlServiceProxy, &aidlServiceProxy);
ASSERT_EQ(res, OK) << "Unable to initialize provider manager";
std::vector<std::string> cameraList = {"device@1.1/virtual/123"};
@@ -868,7 +902,7 @@
ndk::SharedRefBase::make<TestAidlICameraProvider>(cameraList);
ndk::SpAIBinder spBinder = aidlProvider->asBinder();
AIBinder* aiBinder = spBinder.get();
- serviceProxy.setAidlProvider(aidlProvider);
+ aidlServiceProxy.setProvider(aidlProvider);
providerManager->onServiceRegistration(
String16("android.hardware.camera.provider.ICameraProvider/virtual/0"),
AIBinder_toPlatformBinder(aiBinder));
@@ -883,15 +917,17 @@
REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(vd_flags, virtual_camera_service_discovery))) {
sp<CameraProviderManager> providerManager = new CameraProviderManager();
sp<TestStatusListener> statusListener = new TestStatusListener();
- TestInteractionProxy serviceProxy;
+ TestAidlInteractionProxy aidlServiceProxy;
+ TestHidlInteractionProxy hidlServiceProxy;
std::vector<std::string> cameraList = {"device@1.1/virtual/123"};
std::shared_ptr<TestAidlICameraProvider> aidlProvider =
ndk::SharedRefBase::make<TestAidlICameraProvider>(cameraList);
- serviceProxy.setAidlProvider(aidlProvider);
+ aidlServiceProxy.setProvider(aidlProvider);
- status_t res = providerManager->initialize(statusListener, &serviceProxy, &serviceProxy);
+ status_t res = providerManager->initialize(statusListener,
+ &hidlServiceProxy, &aidlServiceProxy);
ASSERT_EQ(res, OK) << "Unable to initialize provider manager";
std::unordered_map<std::string, std::set<std::string>> unavailableDeviceIds;