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

Commit 186b92a0 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: 6c8d71e9

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

Change-Id: I171ff923ff37ac5af77c70116cdc6d695bc73011
parents f08b244c 6c8d71e9
Loading
Loading
Loading
Loading
+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\"",
+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";
}
+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))) {