[libcamera-devel,v2,2/6] libcamera: camera: Generate camera name from a CameraSensor

Message ID 20200728003058.2871461-3-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Generate unique and stable camera names
Related show

Commit Message

Niklas Söderlund July 28, 2020, 12:30 a.m. UTC
Add a create() that generates a camera name from information in a
CameraSensor. The intention is to keep the style of camera names more
standardised and to ease adding more parameters to the name as they
become available.

The goal is to generate a unique name that describes where the camera is
located in the system without using bus information, as enumerated bus
information is not guaranteed to be static between resets of the system.
More parameters should be added to the name as CameraSensor is extended.

In this first change the name is made up of the mandatory CameraSensor
controls model, location and rotation if it is not 0. As many platforms
hardware description (devicetree or ACPI) do not yet describe these
properties the default location front is used for some cameras which is
not true.

All pipelines that implements a CameraSensor (all but UVC) are updated
to make use of this new function. All names of cameras created by the
updated pipelines are modified.

Before this change example of camera names:

ov5695 7-0036 (PipelineHandlerRkISP1)
ov2685 7-003c (PipelineHandlerRkISP1)

After this change the same cameras are:

ov5695 location front (PipelineHandlerRkISP1)
ov2685 location front (PipelineHandlerRkISP1)

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/camera.h                    |  5 +++
 src/libcamera/camera.cpp                      | 38 +++++++++++++++++++
 src/libcamera/pipeline/ipu3/ipu3.cpp          | 12 +++---
 .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-
 src/libcamera/pipeline/simple/simple.cpp      |  2 +-
 src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-
 test/camera/buffer_import.cpp                 |  2 +-
 test/camera/capture.cpp                       |  2 +-
 test/camera/configuration_default.cpp         |  2 +-
 test/camera/configuration_set.cpp             |  2 +-
 test/camera/statemachine.cpp                  |  2 +-
 test/controls/control_info_map.cpp            |  2 +-
 test/controls/control_list.cpp                |  2 +-
 test/serialization/serialization_test.h       |  2 +-
 15 files changed, 62 insertions(+), 20 deletions(-)

Patch

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index 4d1a4a9f52ec0fac..784510c9a79d44b9 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -19,6 +19,7 @@ 
 
 namespace libcamera {
 
+class CameraSensor;
 class FrameBuffer;
 class FrameBufferAllocator;
 class PipelineHandler;
@@ -73,6 +74,10 @@  public:
 					      const std::string &name,
 					      const std::set<Stream *> &streams);
 
+	static std::shared_ptr<Camera> create(PipelineHandler *pipe,
+					      const CameraSensor *sensor,
+					      const std::set<Stream *> &streams);
+
 	Camera(const Camera &) = delete;
 	Camera &operator=(const Camera &) = delete;
 
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 81d5816ef6a653b3..c5413341fefa1124 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -11,9 +11,11 @@ 
 #include <iomanip>
 
 #include <libcamera/framebuffer_allocator.h>
+#include <libcamera/property_ids.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/log.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/utils.h"
@@ -475,6 +477,42 @@  std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
 	return std::shared_ptr<Camera>(camera, Deleter());
 }
 
+/**
+ * \brief Create a camera instance
+ * \param[in] pipe The pipeline handler responsible for the camera device
+ * \param[in] sensor The sensor of the camera device
+ * \param[in] streams Array of streams the camera provides
+ *
+ * Create a camera with name generated from \a sensor.
+ *
+ * \return A shared pointer to the newly created camera object
+ */
+std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
+				       const CameraSensor *sensor,
+				       const std::set<Stream *> &streams)
+{
+	std::string name = sensor->model();
+
+	name += " location ";
+	switch (sensor->properties().get(properties::Location)) {
+	case properties::CameraLocationFront:
+		name += "front";
+		break;
+	case properties::CameraLocationBack:
+		name += "back";
+		break;
+	case properties::CameraLocationExternal:
+		name += "external";
+		break;
+	}
+
+	unsigned int rotation = sensor->properties().get(properties::Location);
+	if (rotation)
+		name += " rotated " + std::to_string(rotation) + " degrees";
+
+	return Camera::create(pipe, name, streams);
+}
+
 /**
  * \brief Retrieve the name of the camera
  * \context This function is \threadsafe.
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index eb00eecfd10a89e4..72da6ed62a7f0de5 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -813,18 +813,16 @@  int PipelineHandlerIPU3::registerCameras()
 					&IPU3CameraData::imguOutputBufferReady);
 
 		/* Create and register the Camera instance. */
-		std::string cameraName = cio2->sensor()->entity()->name();
-		std::shared_ptr<Camera> camera = Camera::create(this,
-								cameraName,
-								streams);
-
-		registerCamera(std::move(camera), std::move(data));
+		std::shared_ptr<Camera> camera =
+			Camera::create(this, cio2->sensor(), streams);
 
 		LOG(IPU3, Info)
 			<< "Registered Camera[" << numCameras << "] \""
-			<< cameraName << "\""
+			<< camera->name() << "\""
 			<< " connected to CSI-2 receiver " << id;
 
+		registerCamera(std::move(camera), std::move(data));
+
 		numCameras++;
 	}
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 82a0a4dfd6824fce..a62dd24b1ab76b87 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -972,7 +972,8 @@  bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
 	streams.insert(&data->isp_[Isp::Stats]);
 
 	/* Create and register the camera. */
-	std::shared_ptr<Camera> camera = Camera::create(this, data->sensor_->model(), streams);
+	std::shared_ptr<Camera> camera =
+		Camera::create(this, data->sensor_, streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return true;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 52a0d862417cc4ec..663e45b109aae9eb 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -971,7 +971,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 
 	std::set<Stream *> streams{ &data->stream_ };
 	std::shared_ptr<Camera> camera =
-		Camera::create(this, sensor->name(), streams);
+		Camera::create(this, data->sensor_, streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return 0;
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 5221ff3384923f56..fcbf0723b69f87d4 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -800,7 +800,7 @@  bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 			continue;
 
 		std::shared_ptr<Camera> camera =
-			Camera::create(this, data->sensor_->entity()->name(),
+			Camera::create(this, data->sensor_.get(),
 				       data->streams());
 		registerCamera(std::move(camera), std::move(data));
 	}
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 4f461b928514022d..74243de13346d4a8 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -432,9 +432,9 @@  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 		return false;
 
 	/* Create and register the camera. */
-	std::string name{ "VIMC " + data->sensor_->model() };
 	std::set<Stream *> streams{ &data->stream_ };
-	std::shared_ptr<Camera> camera = Camera::create(this, name, streams);
+	std::shared_ptr<Camera> camera =
+		Camera::create(this, data->sensor_, streams);
 	registerCamera(std::move(camera), std::move(data));
 
 	return true;
diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
index ab406c35fe9e07e7..838b3e867b220adc 100644
--- a/test/camera/buffer_import.cpp
+++ b/test/camera/buffer_import.cpp
@@ -28,7 +28,7 @@  class BufferImportTest : public CameraTest, public Test
 {
 public:
 	BufferImportTest()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index 1d57201ef83de1aa..a0d7c5cba8b16c56 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -18,7 +18,7 @@  class Capture : public CameraTest, public Test
 {
 public:
 	Capture()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
index a1159c18d78c8493..7e87c2f84add463d 100644
--- a/test/camera/configuration_default.cpp
+++ b/test/camera/configuration_default.cpp
@@ -18,7 +18,7 @@  class ConfigurationDefault : public CameraTest, public Test
 {
 public:
 	ConfigurationDefault()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
index 63331c0a2c7df799..80a78f63d59b8d7c 100644
--- a/test/camera/configuration_set.cpp
+++ b/test/camera/configuration_set.cpp
@@ -18,7 +18,7 @@  class ConfigurationSet : public CameraTest, public Test
 {
 public:
 	ConfigurationSet()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index 77d021823774a674..1fa63a243405613c 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -18,7 +18,7 @@  class Statemachine : public CameraTest, public Test
 {
 public:
 	Statemachine()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/controls/control_info_map.cpp b/test/controls/control_info_map.cpp
index 1d601a827e10e995..c2bf583eaeeb3d04 100644
--- a/test/controls/control_info_map.cpp
+++ b/test/controls/control_info_map.cpp
@@ -24,7 +24,7 @@  class ControlInfoMapTest : public CameraTest, public Test
 {
 public:
 	ControlInfoMapTest()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index 5dad297501d009a5..b33e44f5014bbb98 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -24,7 +24,7 @@  class ControlListTest : public CameraTest, public Test
 {
 public:
 	ControlListTest()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B location front (PipelineHandlerVimc)")
 	{
 	}
 
diff --git a/test/serialization/serialization_test.h b/test/serialization/serialization_test.h
index 621c6d6279525300..3f326e8f9c90ff34 100644
--- a/test/serialization/serialization_test.h
+++ b/test/serialization/serialization_test.h
@@ -20,7 +20,7 @@  class SerializationTest : public CameraTest, public Test
 {
 public:
 	SerializationTest()
-		: CameraTest("VIMC Sensor B (PipelineHandlerVimc)")
+		: CameraTest("Sensor B facing forward (PipelineHandlerVimc)")
 	{
 	}