Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 682788a9 authored by Avichal Rakesh's avatar Avichal Rakesh
Browse files

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
parent 07d3bfcf
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -83,3 +83,10 @@ flag {
     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"
}
+28 −6
Original line number Diff line number Diff line
@@ -139,7 +139,7 @@ status_t CameraProviderManager::tryToInitAndAddHidlProvidersLocked(
}

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 @@ CameraProviderManager::AidlServiceInteractionProxyImpl::getAidlService(
    }

    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 @@ status_t CameraProviderManager::tryToInitializeAidlProviderLocked(
        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__,
+12 −3
Original line number Diff line number Diff line
@@ -178,9 +178,15 @@ public:
    // 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 @@ public:
    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;
    };

    /**
+3 −2
Original line number Diff line number Diff line
@@ -297,9 +297,10 @@ const std::shared_ptr<ICameraProvider> AidlProviderInfo::startProviderInterface(
    }

    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;
    }

+60 −24
Original line number Diff line number Diff line
@@ -372,28 +372,22 @@ class TestAidlICameraProvider : public aidl::android::hardware::camera::provider
};

/**
 * 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 @@ struct TestInteractionProxy : public CameraProviderManager::HidlServiceInteracti
        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>
            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>
    getAidlService(const std::string&) {
    tryGetService(const std::string&) override {
        return mTestAidlCameraProvider;
    }
};
@@ -462,7 +494,7 @@ TEST(CameraProviderManagerTest, InitializeDynamicDepthTest) {
    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 @@ TEST(CameraProviderManagerTest, InitializeTest) {
    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 @@ TEST(CameraProviderManagerTest, MultipleVendorTagTest) {

    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 @@ TEST(CameraProviderManagerTest, NotifyStateChangeTest) {
    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 @@ TEST(CameraProviderManagerTest, BadHalStartupTest) {

    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 @@ TEST(CameraProviderManagerTest, BinderDeathRegistrationRaceTest) {

    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 @@ TEST(CameraProviderManagerTest, PhysicalCameraAvailabilityCallbackRaceTest) {

    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 @@ TEST_WITH_FLAGS(CameraProviderManagerTest, AidlVirtualCameraProviderDiscovered,
                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 @@ TEST_WITH_FLAGS(CameraProviderManagerTest, AidlVirtualCameraProviderDiscovered,
            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 @@ TEST_WITH_FLAGS(CameraProviderManagerTest, AidlVirtualCameraProviderDiscoveredOn
                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;