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

Commit 0c956126 authored by Shuzhen Wang's avatar Shuzhen Wang Committed by Automerger Merge Worker
Browse files

Camera: Fix missing physical camera availability callback am: 3d316f37

parents 36799206 3d316f37
Loading
Loading
Loading
Loading
+9 −2
Original line number Diff line number Diff line
@@ -203,6 +203,7 @@ status_t CameraService::enumerateProviders() {
    status_t res;

    std::vector<std::string> deviceIds;
    std::unordered_map<std::string, std::set<std::string>> unavailPhysicalIds;
    {
        Mutex::Autolock l(mServiceLock);

@@ -233,7 +234,7 @@ status_t CameraService::enumerateProviders() {
            ALOGE("Failed to enumerate flash units: %s (%d)", strerror(-res), res);
        }

        deviceIds = mCameraProviderManager->getCameraDeviceIds();
        deviceIds = mCameraProviderManager->getCameraDeviceIds(&unavailPhysicalIds);
    }


@@ -242,6 +243,12 @@ status_t CameraService::enumerateProviders() {
        if (getCameraState(id8) == nullptr) {
            onDeviceStatusChanged(id8, CameraDeviceStatus::PRESENT);
        }
        if (unavailPhysicalIds.count(cameraId) > 0) {
            for (const auto& physicalId : unavailPhysicalIds[cameraId]) {
                String8 physicalId8 = String8(physicalId.c_str());
                onDeviceStatusChanged(id8, physicalId8, CameraDeviceStatus::NOT_PRESENT);
            }
        }
    }

    // Derive primary rear/front cameras, and filter their charactierstics.
@@ -495,7 +502,7 @@ void CameraService::onDeviceStatusChanged(const String8& id,

    if (state == nullptr) {
        ALOGE("%s: Physical camera id %s status change on a non-present ID %s",
                __FUNCTION__, id.string(), physicalId.string());
                __FUNCTION__, physicalId.string(), id.string());
        return;
    }

+18 −39
Original line number Diff line number Diff line
@@ -197,12 +197,17 @@ std::pair<int, int> CameraProviderManager::getCameraCount() const {
    return std::make_pair(systemCameraCount, publicCameraCount);
}

std::vector<std::string> CameraProviderManager::getCameraDeviceIds() const {
std::vector<std::string> CameraProviderManager::getCameraDeviceIds(std::unordered_map<
            std::string, std::set<std::string>>* unavailablePhysicalIds) const {
    std::lock_guard<std::mutex> lock(mInterfaceMutex);
    std::vector<std::string> deviceIds;
    for (auto& provider : mProviders) {
        for (auto& id : provider->mUniqueCameraIds) {
            deviceIds.push_back(id);
            if (unavailablePhysicalIds != nullptr &&
                    provider->mUnavailablePhysicalCameras.count(id) > 0) {
                (*unavailablePhysicalIds)[id] = provider->mUnavailablePhysicalCameras.at(id);
            }
        }
    }
    return deviceIds;
@@ -843,9 +848,6 @@ status_t CameraProviderManager::dump(int fd, const Vector<String16>& args) {

void CameraProviderManager::ProviderInfo::initializeProviderInfoCommon(
        const std::vector<std::string> &devices) {

    sp<StatusListener> listener = mManager->getStatusListener();

    for (auto& device : devices) {
        std::string id;
        status_t res = addDevice(device, CameraDeviceStatus::PRESENT, &id);
@@ -860,38 +862,22 @@ void CameraProviderManager::ProviderInfo::initializeProviderInfoCommon(
            mProviderName.c_str(), mDevices.size());

    // Process cached status callbacks
    std::unique_ptr<std::vector<CameraStatusInfoT>> cachedStatus =
            std::make_unique<std::vector<CameraStatusInfoT>>();
    {
        std::lock_guard<std::mutex> lock(mInitLock);

        for (auto& statusInfo : mCachedStatus) {
            std::string id, physicalId;
            status_t res = OK;
            if (statusInfo.isPhysicalCameraStatus) {
                res = physicalCameraDeviceStatusChangeLocked(&id, &physicalId,
                physicalCameraDeviceStatusChangeLocked(&id, &physicalId,
                    statusInfo.cameraId, statusInfo.physicalCameraId, statusInfo.status);
            } else {
                res = cameraDeviceStatusChangeLocked(&id, statusInfo.cameraId, statusInfo.status);
            }
            if (res == OK) {
                cachedStatus->emplace_back(statusInfo.isPhysicalCameraStatus,
                        id.c_str(), physicalId.c_str(), statusInfo.status);
                cameraDeviceStatusChangeLocked(&id, statusInfo.cameraId, statusInfo.status);
            }
        }
        mCachedStatus.clear();

        mInitialized = true;
    }

    // The cached status change callbacks cannot be fired directly from this
    // function, due to same-thread deadlock trying to acquire mInterfaceMutex
    // twice.
    if (listener != nullptr) {
        mInitialStatusCallbackFuture = std::async(std::launch::async,
                &CameraProviderManager::ProviderInfo::notifyInitialStatusChange, this,
                listener, std::move(cachedStatus));
    }
}

CameraProviderManager::ProviderInfo::DeviceInfo* CameraProviderManager::findDeviceInfoLocked(
@@ -1961,6 +1947,7 @@ void CameraProviderManager::ProviderInfo::removeDevice(std::string id) {
    for (auto it = mDevices.begin(); it != mDevices.end(); it++) {
        if ((*it)->mId == id) {
            mUniqueCameraIds.erase(id);
            mUnavailablePhysicalCameras.erase(id);
            if ((*it)->isAPI1Compatible()) {
                mUniqueAPI1CompatibleCameraIds.erase(std::remove(
                    mUniqueAPI1CompatibleCameraIds.begin(),
@@ -2228,6 +2215,15 @@ status_t CameraProviderManager::ProviderInfo::physicalCameraDeviceStatusChangeLo
        return BAD_VALUE;
    }

    if (mUnavailablePhysicalCameras.count(cameraId) == 0) {
        mUnavailablePhysicalCameras.emplace(cameraId, std::set<std::string>{});
    }
    if (newStatus != CameraDeviceStatus::PRESENT) {
        mUnavailablePhysicalCameras[cameraId].insert(physicalCameraDeviceName);
    } else {
        mUnavailablePhysicalCameras[cameraId].erase(physicalCameraDeviceName);
    }

    *id = cameraId;
    *physicalId = physicalCameraDeviceName.c_str();
    return OK;
@@ -2286,20 +2282,6 @@ void CameraProviderManager::ProviderInfo::notifyDeviceInfoStateChangeLocked(
    }
}

void CameraProviderManager::ProviderInfo::notifyInitialStatusChange(
        sp<StatusListener> listener,
        std::unique_ptr<std::vector<CameraStatusInfoT>> cachedStatus) {
    for (auto& statusInfo : *cachedStatus) {
        if (statusInfo.isPhysicalCameraStatus) {
            listener->onDeviceStatusChanged(String8(statusInfo.cameraId.c_str()),
                    String8(statusInfo.physicalCameraId.c_str()), statusInfo.status);
        } else {
            listener->onDeviceStatusChanged(
                    String8(statusInfo.cameraId.c_str()), statusInfo.status);
        }
    }
}

CameraProviderManager::ProviderInfo::DeviceInfo3::DeviceInfo3(const std::string& name,
        const metadata_vendor_id_t tagId, const std::string &id,
        uint16_t minorVersion,
@@ -2649,9 +2631,6 @@ status_t CameraProviderManager::ProviderInfo::parseDeviceName(const std::string&
}

CameraProviderManager::ProviderInfo::~ProviderInfo() {
    if (mInitialStatusCallbackFuture.valid()) {
        mInitialStatusCallbackFuture.wait();
    }
    // Destruction of ProviderInfo is only supposed to happen when the respective
    // CameraProvider interface dies, so do not unregister callbacks.
}
+9 −7
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@
#include <set>
#include <string>
#include <mutex>
#include <future>

#include <camera/camera2/ConcurrentCamera.h>
#include <camera/CameraParameters2.h>
@@ -220,7 +219,14 @@ public:
     */
    std::pair<int, int> getCameraCount() const;

    std::vector<std::string> getCameraDeviceIds() const;
    /**
     * Upon the function return, if unavailablePhysicalIds is not nullptr, it
     * will contain all of the unavailable physical camera Ids represented in
     * the form of:
     * {[logicalCamera, {physicalCamera1, physicalCamera2, ...}], ...}.
     */
    std::vector<std::string> getCameraDeviceIds(std::unordered_map<
            std::string, std::set<std::string>>* unavailablePhysicalIds = nullptr) const;

    /**
     * Retrieve the number of API1 compatible cameras; these are internal and
@@ -607,6 +613,7 @@ private:
        };
        std::vector<std::unique_ptr<DeviceInfo>> mDevices;
        std::unordered_set<std::string> mUniqueCameraIds;
        std::unordered_map<std::string, std::set<std::string>> mUnavailablePhysicalCameras;
        int mUniqueDeviceCount;
        std::vector<std::string> mUniqueAPI1CompatibleCameraIds;
        // The initial public camera IDs published by the camera provider.
@@ -715,8 +722,6 @@ private:
        std::vector<CameraStatusInfoT> mCachedStatus;
        // End of scope for mInitLock

        std::future<void> mInitialStatusCallbackFuture;

        std::unique_ptr<ProviderInfo::DeviceInfo>
        virtual initializeDeviceInfo(
                const std::string &name, const metadata_vendor_id_t tagId,
@@ -724,9 +729,6 @@ private:

        virtual status_t reCacheConcurrentStreamingCameraIdsLocked() = 0;

        void notifyInitialStatusChange(sp<StatusListener> listener,
                std::unique_ptr<std::vector<CameraStatusInfoT>> cachedStatus);

        std::vector<std::unordered_set<std::string>> mConcurrentCameraIdCombinations;

        // Parse provider instance name for type and id
+84 −3
Original line number Diff line number Diff line
@@ -102,23 +102,57 @@ struct TestICameraProvider : virtual public provider::V2_5::ICameraProvider {
    sp<device::V3_2::ICameraDevice> mDeviceInterface;
    hardware::hidl_vec<common::V1_0::VendorTagSection> mVendorTagSections;

    // Whether to call a physical camera unavailable callback upon setCallback
    bool mHasPhysicalCameraUnavailableCallback;
    hardware::hidl_string mLogicalCameraId;
    hardware::hidl_string mUnavailablePhysicalCameraId;

    TestICameraProvider(const std::vector<hardware::hidl_string> &devices,
            const hardware::hidl_vec<common::V1_0::VendorTagSection> &vendorSection) :
        mDeviceNames(devices),
        mDeviceInterface(new TestDeviceInterface(devices)),
        mVendorTagSections (vendorSection) {}
        mVendorTagSections (vendorSection),
        mHasPhysicalCameraUnavailableCallback(false) {}

    TestICameraProvider(const std::vector<hardware::hidl_string> &devices,
            const hardware::hidl_vec<common::V1_0::VendorTagSection> &vendorSection,
            android::hardware::hidl_vec<uint8_t> chars) :
        mDeviceNames(devices),
        mDeviceInterface(new TestDeviceInterface(devices, chars)),
        mVendorTagSections (vendorSection) {}
        mVendorTagSections (vendorSection),
        mHasPhysicalCameraUnavailableCallback(false) {}

    TestICameraProvider(const std::vector<hardware::hidl_string> &devices,
            const hardware::hidl_vec<common::V1_0::VendorTagSection> &vendorSection,
            android::hardware::hidl_vec<uint8_t> chars,
            const hardware::hidl_string& logicalCameraId,
            const hardware::hidl_string& unavailablePhysicalCameraId) :
        mDeviceNames(devices),
        mDeviceInterface(new TestDeviceInterface(devices, chars)),
        mVendorTagSections (vendorSection),
        mHasPhysicalCameraUnavailableCallback(true),
        mLogicalCameraId(logicalCameraId),
        mUnavailablePhysicalCameraId(unavailablePhysicalCameraId) {}

    virtual hardware::Return<Status> setCallback(
            const sp<provider::V2_4::ICameraProviderCallback>& callbacks) override {
        mCalledCounter[SET_CALLBACK]++;
        mCallbacks = callbacks;
        if (mHasPhysicalCameraUnavailableCallback) {
            auto cast26 = provider::V2_6::ICameraProviderCallback::castFrom(callbacks);
            if (!cast26.isOk()) {
                ADD_FAILURE() << "Failed to cast ICameraProviderCallback to V2_6";
            } else {
                sp<provider::V2_6::ICameraProviderCallback> callback26 = cast26;
                if (callback26 == nullptr) {
                    ADD_FAILURE() << "V2_6::ICameraProviderCallback is null after conversion";
                } else {
                    callback26->physicalCameraDeviceStatusChange(mLogicalCameraId,
                            mUnavailablePhysicalCameraId,
                            android::hardware::camera::common::V1_0::CameraDeviceStatus::NOT_PRESENT);
                }
            }
        }
        return hardware::Return<Status>(Status::OK);
    }

@@ -266,12 +300,16 @@ struct TestInteractionProxy : public CameraProviderManager::HidlServiceInteracti
};

struct TestStatusListener : public CameraProviderManager::StatusListener {
    int mPhysicalCameraStatusChangeCount = 0;

    ~TestStatusListener() {}

    void onDeviceStatusChanged(const String8 &,
            CameraDeviceStatus) override {}
    void onDeviceStatusChanged(const String8 &, const String8 &,
            CameraDeviceStatus) override {}
            CameraDeviceStatus) override {
        mPhysicalCameraStatusChangeCount++;
    }
    void onTorchStatusChanged(const String8 &,
            TorchModeStatus) override {}
    void onTorchStatusChanged(const String8 &,
@@ -634,3 +672,46 @@ TEST(CameraProviderManagerTest, BinderDeathRegistrationRaceTest) {
    ASSERT_EQ(deviceCount, deviceNames.size()) <<
            "Unexpected amount of camera devices";
}

// Test that CameraProviderManager does not trigger
// onDeviceStatusChanged(NOT_PRESENT) for physical camera before initialize()
// returns.
TEST(CameraProviderManagerTest, PhysicalCameraAvailabilityCallbackRaceTest) {
    std::vector<hardware::hidl_string> deviceNames;
    deviceNames.push_back("device@3.2/test/0");
    hardware::hidl_vec<common::V1_0::VendorTagSection> vendorSection;

    sp<CameraProviderManager> providerManager = new CameraProviderManager();
    sp<TestStatusListener> statusListener = new TestStatusListener();
    TestInteractionProxy serviceProxy;

    android::hardware::hidl_vec<uint8_t> chars;
    CameraMetadata meta;
    int32_t charKeys[] = { ANDROID_REQUEST_AVAILABLE_CAPABILITIES };
    meta.update(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, charKeys,
            sizeof(charKeys) / sizeof(charKeys[0]));
    uint8_t capabilities[] = { ANDROID_REQUEST_AVAILABLE_CAPABILITIES_LOGICAL_MULTI_CAMERA };
    meta.update(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capabilities,
            sizeof(capabilities)/sizeof(capabilities[0]));
    uint8_t physicalCameraIds[] = { '2', '\0', '3', '\0' };
    meta.update(ANDROID_LOGICAL_MULTI_CAMERA_PHYSICAL_IDS, physicalCameraIds,
            sizeof(physicalCameraIds)/sizeof(physicalCameraIds[0]));
    camera_metadata_t* metaBuffer = const_cast<camera_metadata_t*>(meta.getAndLock());
    chars.setToExternal(reinterpret_cast<uint8_t*>(metaBuffer),
            get_camera_metadata_size(metaBuffer));

    sp<TestICameraProvider> provider = new TestICameraProvider(deviceNames,
            vendorSection, chars, "device@3.2/test/0", "2");
    serviceProxy.setProvider(provider);

    status_t res = providerManager->initialize(statusListener, &serviceProxy);
    ASSERT_EQ(res, OK) << "Unable to initialize provider manager";

    ASSERT_EQ(statusListener->mPhysicalCameraStatusChangeCount, 0)
            << "Unexpected physical camera status change callback upon provider init.";

    std::unordered_map<std::string, std::set<std::string>> unavailablePhysicalIds;
    auto cameraIds = providerManager->getCameraDeviceIds(&unavailablePhysicalIds);
    ASSERT_TRUE(unavailablePhysicalIds.count("0") > 0 && unavailablePhysicalIds["0"].count("2") > 0)
        << "Unavailable physical camera Ids not set properly.";
}