Merge "Camera: update eviction logic for same owner case" into rvc-dev (6c8d71e9) · Commits · e / os / android_frameworks_av

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))) {