Merge "Support non-numerical id (with prefix) for virtual camera" into main
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.cc b/services/camera/virtualcamera/VirtualCameraDevice.cc
index e455378..c3be62b 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 296383f..a33d4cf 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>&
@@ -141,7 +143,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 93f90b4..a9eb413 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;