Support non-numerical id (with prefix) for virtual camera
This will ensure that virtual camera id's don't clash
with camera id's provided by any other HAL's. Also, this
should be fine as we currently don't expose virtual camera
id's to apps.
Test: atest CtsVirtualDevicesCameraTestCases
Test: atest virtual_camera_tests
Test: atest CtsVirtualDevicesCameraCtsTestCases
Flag: android.companion.virtual.flags.virtual_camera
Bug: 343404629
Change-Id: I304161eee08b144b82d31e8e9b8a9fcc2b9fc45c
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.cc b/services/camera/virtualcamera/VirtualCameraDevice.cc
index ba4ea6b..7122a11 100644
--- a/services/camera/virtualcamera/VirtualCameraDevice.cc
+++ b/services/camera/virtualcamera/VirtualCameraDevice.cc
@@ -68,7 +68,7 @@
using namespace std::chrono_literals;
-// Prefix of camera name - "device@1.1/virtual/{numerical_id}"
+// Prefix of camera name - "device@1.1/virtual/{camera_id}"
const char* kDevicePathPrefix = "device@1.1/virtual/";
constexpr int32_t kMaxJpegSize = 3 * 1024 * 1024 /*3MiB*/;
@@ -404,8 +404,8 @@
} // namespace
VirtualCameraDevice::VirtualCameraDevice(
- const uint32_t cameraId, const VirtualCameraConfiguration& configuration,
- int32_t deviceId)
+ const std::string& cameraId,
+ const VirtualCameraConfiguration& configuration, int32_t deviceId)
: mCameraId(cameraId),
mVirtualCameraClientCallback(configuration.virtualCameraCallback),
mSupportedInputConfigurations(configuration.supportedStreamConfigs) {
@@ -582,11 +582,11 @@
}
binder_status_t VirtualCameraDevice::dump(int fd, const char**, uint32_t) {
- ALOGD("Dumping virtual camera %d", mCameraId);
+ ALOGD("Dumping virtual camera %s", mCameraId.c_str());
const char* indent = " ";
const char* doubleIndent = " ";
- dprintf(fd, "%svirtual_camera %d belongs to virtual device %d\n", indent,
- mCameraId,
+ dprintf(fd, "%svirtual_camera %s belongs to virtual device %d\n", indent,
+ mCameraId.c_str(),
getDeviceId(mCameraCharacteristics)
.value_or(VirtualCameraService::kDefaultDeviceId));
dprintf(fd, "%sSupportedStreamConfiguration:\n", indent);
@@ -597,7 +597,7 @@
}
std::string VirtualCameraDevice::getCameraName() const {
- return std::string(kDevicePathPrefix) + std::to_string(mCameraId);
+ return std::string(kDevicePathPrefix) + mCameraId;
}
const std::vector<SupportedStreamConfiguration>&
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.h b/services/camera/virtualcamera/VirtualCameraDevice.h
index 53dcc4d..c13dcbd 100644
--- a/services/camera/virtualcamera/VirtualCameraDevice.h
+++ b/services/camera/virtualcamera/VirtualCameraDevice.h
@@ -37,7 +37,7 @@
: public ::aidl::android::hardware::camera::device::BnCameraDevice {
public:
explicit VirtualCameraDevice(
- uint32_t cameraId,
+ const std::string& cameraId,
const aidl::android::companion::virtualcamera::VirtualCameraConfiguration&
configuration,
int32_t deviceId);
@@ -92,10 +92,12 @@
binder_status_t dump(int fd, const char** args, uint32_t numArgs) override;
// Returns unique virtual camera name in form
- // "device@{major}.{minor}/virtual/{numerical_id}"
+ // "device@{major}.{minor}/virtual/{camera_id}"
std::string getCameraName() const;
- uint32_t getCameraId() const { return mCameraId; }
+ const std::string& getCameraId() const {
+ return mCameraId;
+ }
const std::vector<
aidl::android::companion::virtualcamera::SupportedStreamConfiguration>&
@@ -138,7 +140,7 @@
private:
std::shared_ptr<VirtualCameraDevice> sharedFromThis();
- const uint32_t mCameraId;
+ const std::string mCameraId;
const std::shared_ptr<
::aidl::android::companion::virtualcamera::IVirtualCameraCallback>
mVirtualCameraClientCallback;
diff --git a/services/camera/virtualcamera/VirtualCameraProvider.cc b/services/camera/virtualcamera/VirtualCameraProvider.cc
index 67eaec0..b2c10f6 100644
--- a/services/camera/virtualcamera/VirtualCameraProvider.cc
+++ b/services/camera/virtualcamera/VirtualCameraProvider.cc
@@ -150,11 +150,10 @@
}
std::shared_ptr<VirtualCameraDevice> VirtualCameraProvider::createCamera(
- const VirtualCameraConfiguration& configuration, const int cameraId,
- const int32_t deviceId) {
- if (cameraId < 0) {
- ALOGE("%s: Cannot create camera with negative id. cameraId: %d", __func__,
- cameraId);
+ const VirtualCameraConfiguration& configuration,
+ const std::string& cameraId, const int32_t deviceId) {
+ if (cameraId.empty()) {
+ ALOGE("%s: Cannot create camera with empty cameraId", __func__);
return nullptr;
}
diff --git a/services/camera/virtualcamera/VirtualCameraProvider.h b/services/camera/virtualcamera/VirtualCameraProvider.h
index c536547..606b44c 100644
--- a/services/camera/virtualcamera/VirtualCameraProvider.h
+++ b/services/camera/virtualcamera/VirtualCameraProvider.h
@@ -77,7 +77,7 @@
std::shared_ptr<VirtualCameraDevice> createCamera(
const aidl::android::companion::virtualcamera::VirtualCameraConfiguration&
configuration,
- int cameraId, int32_t deviceId);
+ const std::string& cameraId, int32_t deviceId);
std::shared_ptr<VirtualCameraDevice> getCamera(const std::string& name);
diff --git a/services/camera/virtualcamera/VirtualCameraService.cc b/services/camera/virtualcamera/VirtualCameraService.cc
index 724ec62..705af86 100644
--- a/services/camera/virtualcamera/VirtualCameraService.cc
+++ b/services/camera/virtualcamera/VirtualCameraService.cc
@@ -57,12 +57,9 @@
using ::aidl::android::companion::virtualcamera::SupportedStreamConfiguration;
using ::aidl::android::companion::virtualcamera::VirtualCameraConfiguration;
-// TODO(b/301023410) Make camera id range configurable / dynamic
-// based on already registered devices.
-std::atomic_int VirtualCameraService::sNextId{1000};
-
namespace {
+constexpr char kCameraIdPrefix[] = "v";
constexpr int kVgaWidth = 640;
constexpr int kVgaHeight = 480;
constexpr int kMaxFps = 60;
@@ -90,6 +87,9 @@
"GL_EXT_YUV_target",
};
+// Numerical portion for id to assign to next created camera.
+static std::atomic_int sNextIdNumericalPortion{1000};
+
ndk::ScopedAStatus validateConfiguration(
const VirtualCameraConfiguration& configuration) {
if (configuration.supportedStreamConfigs.empty()) {
@@ -192,7 +192,7 @@
}
return cmd;
-};
+}
ndk::ScopedAStatus verifyRequiredEglExtensions() {
EglDisplayContext context;
@@ -211,6 +211,11 @@
return ndk::ScopedAStatus::ok();
}
+std::string createCameraId(const int32_t deviceId) {
+ return kCameraIdPrefix + std::to_string(deviceId) + "_" +
+ std::to_string(sNextIdNumericalPortion++);
+}
+
} // namespace
VirtualCameraService::VirtualCameraService(
@@ -224,13 +229,14 @@
const ::ndk::SpAIBinder& token,
const VirtualCameraConfiguration& configuration, const int32_t deviceId,
bool* _aidl_return) {
- return registerCamera(token, configuration, sNextId++, deviceId, _aidl_return);
+ return registerCamera(token, configuration, createCameraId(deviceId),
+ deviceId, _aidl_return);
}
ndk::ScopedAStatus VirtualCameraService::registerCamera(
const ::ndk::SpAIBinder& token,
- const VirtualCameraConfiguration& configuration, const int cameraId,
- const int32_t deviceId, bool* _aidl_return) {
+ const VirtualCameraConfiguration& configuration,
+ const std::string& cameraId, const int32_t deviceId, bool* _aidl_return) {
if (!mPermissionProxy.checkCallingPermission(kCreateVirtualDevicePermission)) {
ALOGE("%s: caller (pid %d, uid %d) doesn't hold %s permission", __func__,
getpid(), getuid(), kCreateVirtualDevicePermission);
@@ -308,7 +314,7 @@
}
ndk::ScopedAStatus VirtualCameraService::getCameraId(
- const ::ndk::SpAIBinder& token, int32_t* _aidl_return) {
+ const ::ndk::SpAIBinder& token, std::string* _aidl_return) {
if (!mPermissionProxy.checkCallingPermission(kCreateVirtualDevicePermission)) {
ALOGE("%s: caller (pid %d, uid %d) doesn't hold %s permission", __func__,
getpid(), getuid(), kCreateVirtualDevicePermission);
@@ -400,13 +406,12 @@
return STATUS_OK;
}
- std::optional<int> cameraId;
+ std::optional<std::string> cameraId;
auto it = options.find("camera_id");
if (it != options.end()) {
- cameraId = parseInt(it->second);
+ cameraId = it->second;
if (!cameraId.has_value()) {
- dprintf(err, "Invalid camera_id: %s\n, must be number > 0",
- it->second.c_str());
+ dprintf(err, "Invalid camera_id: %s", it->second.c_str());
return STATUS_BAD_VALUE;
}
}
@@ -447,7 +452,8 @@
configuration.virtualCameraCallback =
ndk::SharedRefBase::make<VirtualCameraTestInstance>(
inputFps.value_or(kTestCameraDefaultInputFps));
- registerCamera(mTestCameraToken, configuration, cameraId.value_or(sNextId++),
+ registerCamera(mTestCameraToken, configuration,
+ cameraId.value_or(std::to_string(sNextIdNumericalPortion++)),
kDefaultDeviceId, &ret);
if (ret) {
dprintf(out, "Successfully registered test camera %s\n",
diff --git a/services/camera/virtualcamera/VirtualCameraService.h b/services/camera/virtualcamera/VirtualCameraService.h
index f04acb5..4ef01c7 100644
--- a/services/camera/virtualcamera/VirtualCameraService.h
+++ b/services/camera/virtualcamera/VirtualCameraService.h
@@ -50,15 +50,17 @@
const ::ndk::SpAIBinder& token,
const ::aidl::android::companion::virtualcamera::VirtualCameraConfiguration&
configuration,
- int cameraId, int32_t deviceId, bool* _aidl_return) EXCLUDES(mLock);
+ const std::string& cameraId, int32_t deviceId, bool* _aidl_return)
+ EXCLUDES(mLock);
// Unregisters camera corresponding to the binder token.
ndk::ScopedAStatus unregisterCamera(const ::ndk::SpAIBinder& token) override
EXCLUDES(mLock);
// Returns the camera id corresponding to the binder token.
- ndk::ScopedAStatus getCameraId(
- const ::ndk::SpAIBinder& token, int32_t* _aidl_return) override EXCLUDES(mLock);
+ ndk::ScopedAStatus getCameraId(const ::ndk::SpAIBinder& token,
+ std::string* _aidl_return) override
+ EXCLUDES(mLock);
// Returns VirtualCameraDevice corresponding to binder token or nullptr if
// there's no camera asociated with the token.
@@ -101,9 +103,6 @@
// Local binder token for test camera instance, or nullptr if there's none.
::ndk::SpAIBinder mTestCameraToken;
-
- // Numerical id to assign to next created camera.
- static std::atomic_int sNextId;
};
} // namespace virtualcamera
diff --git a/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl b/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
index 1bd99be..2f1e2a9 100644
--- a/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
+++ b/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
@@ -41,5 +41,5 @@
* Returns the camera id for a given binder token. Note that this id corresponds to the id of
* the camera device in the camera framework.
*/
- int getCameraId(in IBinder token);
+ @utf8InCpp String getCameraId(in IBinder token);
}
diff --git a/services/camera/virtualcamera/tests/VirtualCameraDeviceTest.cc b/services/camera/virtualcamera/tests/VirtualCameraDeviceTest.cc
index 3fe7c11..32cd23f 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraDeviceTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraDeviceTest.cc
@@ -54,11 +54,9 @@
using metadata_stream_t =
camera_metadata_enum_android_scaler_available_stream_configurations_t;
-constexpr int kCameraId = 42;
+constexpr char kCameraId[] = "42";
constexpr int kQvgaWidth = 320;
constexpr int kQvgaHeight = 240;
-constexpr int k360pWidth = 640;
-constexpr int k360pHeight = 360;
constexpr int kVgaWidth = 640;
constexpr int kVgaHeight = 480;
constexpr int kHdWidth = 1280;
diff --git a/services/camera/virtualcamera/tests/VirtualCameraProviderTest.cc b/services/camera/virtualcamera/tests/VirtualCameraProviderTest.cc
index f1b2a92..d4bc6de 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraProviderTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraProviderTest.cc
@@ -50,13 +50,12 @@
using ::testing::Not;
using ::testing::Return;
+constexpr char kCameraId[] = "42";
constexpr int kVgaWidth = 640;
constexpr int kVgaHeight = 480;
constexpr int kMaxFps = 30;
-constexpr int kCameraId = 9999;
constexpr int kDefaultDeviceId = 0;
-constexpr char kVirtualCameraNameRegex[] =
- "device@[0-9]+\\.[0-9]+/virtual/[0-9]+";
+constexpr char kVirtualCameraNameRegex[] = "device@[0-9]+\\.[0-9]+/virtual/.+";
class MockCameraProviderCallback : public BnCameraProviderCallback {
public:
diff --git a/services/camera/virtualcamera/tests/VirtualCameraServiceTest.cc b/services/camera/virtualcamera/tests/VirtualCameraServiceTest.cc
index 50977d8..752d390 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraServiceTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraServiceTest.cc
@@ -390,7 +390,7 @@
}
TEST_F(VirtualCameraServiceTest, GetIdWithoutPermissionFails) {
- int32_t aidlRet;
+ std::string aidlRet;
EXPECT_CALL(mMockPermissionsProxy,
checkCallingPermission(kCreateVirtualDevicePermissions))
.WillOnce(Return(false));
@@ -445,11 +445,13 @@
}
TEST_F(VirtualCameraServiceTest, TestCameraShellCmdWithId) {
- EXPECT_THAT(execute_shell_command("enable_test_camera --camera_id=12345"),
- Eq(NO_ERROR));
+ EXPECT_THAT(
+ execute_shell_command("enable_test_camera --camera_id=hello12345"),
+ Eq(NO_ERROR));
std::vector<std::string> cameraIdsAfterEnable = getCameraIds();
- EXPECT_THAT(cameraIdsAfterEnable, ElementsAre("device@1.1/virtual/12345"));
+ EXPECT_THAT(cameraIdsAfterEnable,
+ ElementsAre("device@1.1/virtual/hello12345"));
EXPECT_THAT(execute_shell_command("disable_test_camera"), Eq(NO_ERROR));
@@ -458,9 +460,8 @@
}
TEST_F(VirtualCameraServiceTest, TestCameraShellCmdWithInvalidId) {
- EXPECT_THAT(
- execute_shell_command("enable_test_camera --camera_id=NotNumericalId"),
- Eq(STATUS_BAD_VALUE));
+ EXPECT_THAT(execute_shell_command("enable_test_camera --camera_id="),
+ Eq(STATUS_BAD_VALUE));
}
TEST_F(VirtualCameraServiceTest, TestCameraShellCmdWithUnknownCommand) {
diff --git a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
index 671e031..9b7c560 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
@@ -37,6 +37,7 @@
namespace virtualcamera {
namespace {
+constexpr char kCameraId[] = "42";
constexpr int kQvgaWidth = 320;
constexpr int kQvgaHeight = 240;
constexpr int kVgaWidth = 640;
@@ -46,7 +47,6 @@
constexpr int kMaxFps = 30;
constexpr int kStreamId = 0;
constexpr int kSecondStreamId = 1;
-constexpr int kCameraId = 42;
constexpr int kDefaultDeviceId = 0;
using ::aidl::android::companion::virtualcamera::BnVirtualCameraCallback;