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

Commit caa62892 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Automerger Merge Worker
Browse files

Merge "Camera: update eviction logic for same owner case" into rvc-dev am:... 

Merge "Camera: update eviction logic for same owner case" into rvc-dev am: 6c8d71e9 am: 8f4e32f9 am: 062a24e0 am: 01f39050

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/av/+/11686192

Change-Id: Ib71cab3baa15283d77536e050cf80242b5cf0ac6
parents 80265fb5 01f39050

services/camera/libcameraservice/CameraService.cpp

+10 −1
Original line number Diff line number Diff line
@@ -1377,7 +1377,12 @@ status_t CameraService::handleEvictionsLocked(const String8& cameraId, int clien 
            Mutex::Autolock l(mLogLock);

            mEventLog.add(msg);



            return -EBUSY;

            auto current = mActiveClientManager.get(cameraId);

            if (current != nullptr) {

                return -EBUSY; // CAMERA_IN_USE

            } else {

                return -EUSERS; // MAX_CAMERAS_IN_USE

            }

        }



        for (auto& i : evicted) {
@@ -1669,6 +1674,10 @@ Status CameraService::connectHelper(const sp<CALLBACK>& cameraCb, const String8& 
                    return STATUS_ERROR_FMT(ERROR_CAMERA_IN_USE,

                            "Higher-priority client using camera, ID \"%s\" currently unavailable",

                            cameraId.string());

                case -EUSERS:

                    return STATUS_ERROR_FMT(ERROR_MAX_CAMERAS_IN_USE,

                            "Too many cameras already open, cannot open camera \"%s\"",

                            cameraId.string());

                default:

                    return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION,

                            "Unexpected error %s (%d) opening camera \"%s\"",

services/camera/libcameraservice/tests/ClientManagerTest.cpp

0 → 100644
+108 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2020 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#define LOG_NDEBUG 0

#define LOG_TAG "ClientManagerTest"



#include "../utils/ClientManager.h"

#include <gtest/gtest.h>



using namespace android::resource_policy;



struct TestClient {

    TestClient(int id, int32_t cost, const std::set<int>& conflictingKeys, int32_t ownerId,

            int32_t score, int32_t state, bool isVendorClient) :

            mId(id), mCost(cost), mConflictingKeys(conflictingKeys),

            mOwnerId(ownerId), mScore(score), mState(state), mIsVendorClient(isVendorClient) {};

    int mId;

    int32_t mCost;    // Int 0..100

    std::set<int> mConflictingKeys;

    int32_t mOwnerId; // PID

    int32_t mScore;   // Priority

    int32_t mState;   // Foreground/background etc

    bool mIsVendorClient;

};



using TestClientDescriptor = ClientDescriptor<int, TestClient>;

using TestDescriptorPtr = std::shared_ptr<TestClientDescriptor>;



TestDescriptorPtr makeDescFromTestClient(const TestClient& tc) {

    return std::make_shared<TestClientDescriptor>(/*ID*/tc.mId, tc, tc.mCost, tc.mConflictingKeys,

            tc.mScore, tc.mOwnerId, tc.mState, tc.mIsVendorClient);

}



class TestClientManager : public ClientManager<int, TestClient> {

public:

    TestClientManager() {}

    virtual ~TestClientManager() {}

};





// Test ClientMager behavior when there is only one single owner

// The expected behavior is that if one owner (application or vendor) is trying

// to open second camera, it may succeed or not, but the first opened camera

// should never be evicted.

TEST(ClientManagerTest, SingleOwnerMultipleCamera) {



    TestClientManager cm;

    TestClient cam0Client(/*ID*/0, /*cost*/100, /*conflicts*/{1},

            /*ownerId*/ 1000, /*score*/50, /*state*/ 1, /*isVendorClient*/ false);

    auto cam0Desc = makeDescFromTestClient(cam0Client);

    auto evicted = cm.addAndEvict(cam0Desc);

    ASSERT_EQ(evicted.size(), 0u) << "Evicted list must be empty";



    TestClient cam1Client(/*ID*/1, /*cost*/100, /*conflicts*/{0},

            /*ownerId*/ 1000, /*score*/50, /*state*/ 1, /*isVendorClient*/ false);

    auto cam1Desc = makeDescFromTestClient(cam1Client);



    // 1. Check with conflicting devices, new client would be evicted

    auto wouldBeEvicted = cm.wouldEvict(cam1Desc);

    ASSERT_EQ(wouldBeEvicted.size(), 1u) << "Evicted list length must be 1";

    ASSERT_EQ(wouldBeEvicted[0]->getKey(), cam1Desc->getKey()) << "cam1 must be evicted";



    cm.removeAll();



    TestClient cam2Client(/*ID*/2, /*cost*/100, /*conflicts*/{},

            /*ownerId*/ 1000, /*score*/50, /*state*/ 1, /*isVendorClient*/ false);

    auto cam2Desc = makeDescFromTestClient(cam2Client);

    evicted = cm.addAndEvict(cam2Desc);

    ASSERT_EQ(evicted.size(), 0u) << "Evicted list must be empty";



    TestClient cam3Client(/*ID*/3, /*cost*/100, /*conflicts*/{},

            /*ownerId*/ 1000, /*score*/50, /*state*/ 1, /*isVendorClient*/ false);

    auto cam3Desc = makeDescFromTestClient(cam3Client);



    // 2. Check without conflicting devices, the pre-existing client won't be evicted

    // In this case, the new client would be granted, but could later be rejected by HAL due to

    // resource cost.

    wouldBeEvicted = cm.wouldEvict(cam3Desc);

    ASSERT_EQ(wouldBeEvicted.size(), 0u) << "Evicted list must be empty";



    cm.removeAll();



    evicted = cm.addAndEvict(cam0Desc);

    ASSERT_EQ(evicted.size(), 0u) << "Evicted list must be empty";



    TestClient cam0ClientNew(/*ID*/0, /*cost*/100, /*conflicts*/{1},

            /*ownerId*/ 1000, /*score*/50, /*state*/ 1, /*isVendorClient*/ false);

    auto cam0DescNew = makeDescFromTestClient(cam0ClientNew);

    wouldBeEvicted = cm.wouldEvict(cam0DescNew);



    // 3. Check opening the same camera twice will evict the older client

    ASSERT_EQ(wouldBeEvicted.size(), 1u) << "Evicted list length must be 1";

    ASSERT_EQ(wouldBeEvicted[0], cam0Desc) << "cam0 (old) must be evicted";

}

services/camera/libcameraservice/utils/ClientManager.h

+14 −0
Original line number Diff line number Diff line
@@ -496,6 +496,20 @@ ClientManager<KEY, VALUE, LISTENER>::wouldEvictLocked( 
                evictList.clear();

                evictList.push_back(client);

                return evictList;

            } else if (conflicting && owner == curOwner) {

                // Pre-existing conflicting client with the same client owner exists

                // Open the same device twice -> most recent open wins

                // Otherwise let the existing client wins to avoid behaviors difference

                // due to how HAL advertising conflicting devices (which is hidden from

                // application)

                if (curKey == key) {

                    evictList.push_back(i);

                    totalCost -= curCost;

                } else {

                    evictList.clear();

                    evictList.push_back(client);

                    return evictList;

                }

            } else if (conflicting || ((totalCost > mMaxCost && curCost > 0) &&

                    (curPriority >= priority) &&

                    !(highestPriorityOwner == owner && owner == curOwner))) {