[libcamera-devel,v7,8/9] libcamera: camera: Rename name() to id()

9191 diff mbox series

Rename Camera::name() to camera::id() to better describe what it
represents, a unique and stable ID for the camera. While at it improve
the documentation for the camera ID to describe it needs to be stable
for a camera between resets of the system.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/camera.h                    |  6 +--
 src/android/camera_device.cpp                 |  4 +-
 src/cam/main.cpp                              |  8 ++--
 src/gstreamer/gstlibcameraprovider.cpp        |  4 +-
 src/gstreamer/gstlibcamerasrc.cpp             |  6 +--
 src/libcamera/camera.cpp                      | 40 ++++++++++++-------
 src/libcamera/camera_controls.cpp             |  2 +-
 src/libcamera/camera_manager.cpp              | 16 ++++----
 src/libcamera/framebuffer_allocator.cpp       |  2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  5 +--
 .../pipeline/raspberrypi/raspberrypi.cpp      |  4 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 14 +++----
 src/qcam/dng_writer.cpp                       |  4 +-
 src/qcam/main_window.cpp                      | 18 ++++-----
 src/v4l2/v4l2_camera_proxy.cpp                |  2 +-
 test/pipeline/ipu3/ipu3_pipeline_test.cpp     |  2 +-
 test/pipeline/rkisp1/rkisp1_pipeline_test.cpp |  2 +-
 17 files changed, 74 insertions(+), 65 deletions(-)

Hi Niklas,

Thank you for the patch.

On Tue, Aug 04, 2020 at 06:13:57PM +0200, Niklas Söderlund wrote:
> Rename Camera::name() to camera::id() to better describe what it
> represents, a unique and stable ID for the camera. While at it improve
> the documentation for the camera ID to describe it needs to be stable
> for a camera between resets of the system.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/camera.h                    |  6 +--
>  src/android/camera_device.cpp                 |  4 +-
>  src/cam/main.cpp                              |  8 ++--
>  src/gstreamer/gstlibcameraprovider.cpp        |  4 +-
>  src/gstreamer/gstlibcamerasrc.cpp             |  6 +--
>  src/libcamera/camera.cpp                      | 40 ++++++++++++-------
>  src/libcamera/camera_controls.cpp             |  2 +-
>  src/libcamera/camera_manager.cpp              | 16 ++++----
>  src/libcamera/framebuffer_allocator.cpp       |  2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  5 +--
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  4 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 14 +++----
>  src/qcam/dng_writer.cpp                       |  4 +-
>  src/qcam/main_window.cpp                      | 18 ++++-----
>  src/v4l2/v4l2_camera_proxy.cpp                |  2 +-
>  test/pipeline/ipu3/ipu3_pipeline_test.cpp     |  2 +-
>  test/pipeline/rkisp1/rkisp1_pipeline_test.cpp |  2 +-
>  17 files changed, 74 insertions(+), 65 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 7dd23d75864ef2db..48d88d64a0a94050 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -71,13 +71,13 @@ class Camera final : public Object, public std::enable_shared_from_this<Camera>
>  {
>  public:
>  	static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> -					      const std::string &name,
> +					      const std::string &id,
>  					      const std::set<Stream *> &streams);
>  
>  	Camera(const Camera &) = delete;
>  	Camera &operator=(const Camera &) = delete;
>  
> -	const std::string &name() const;
> +	const std::string &id() const;
>  
>  	Signal<Request *, FrameBuffer *> bufferCompleted;
>  	Signal<Request *> requestCompleted;
> @@ -100,7 +100,7 @@ public:
>  	int stop();
>  
>  private:
> -	Camera(PipelineHandler *pipe, const std::string &name,
> +	Camera(PipelineHandler *pipe, const std::string &id,
>  	       const std::set<Stream *> &streams);
>  	~Camera();
>  
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b5206fd623c62b23..c74e8a6e84251838 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1072,7 +1072,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  	int ret = camera_->configure(config_.get());
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to configure camera '"
> -				<< camera_->name() << "'";
> +				<< camera_->id() << "'";
>  		return ret;
>  	}
>  
> @@ -1240,7 +1240,7 @@ void CameraDevice::requestComplete(Request *request)
>  
>  std::string CameraDevice::logPrefix() const
>  {
> -	return "'" + camera_->name() + "'";
> +	return "'" + camera_->id() + "'";
>  }
>  
>  void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp)
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index ec59e9eaf1176a94..6791e57bfdea05b2 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -118,7 +118,7 @@ int CamApp::init(int argc, char **argv)
>  			return -EINVAL;
>  		}
>  
> -		std::cout << "Using camera " << camera_->name() << std::endl;
> +		std::cout << "Using camera " << camera_->id() << std::endl;
>  
>  		ret = prepareConfig();
>  		if (ret)

You're missing

diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index ec59e9eaf117..d16268d8148a 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -173,7 +173,7 @@ int CamApp::parseOptions(int argc, char *argv[])

 	OptionsParser parser;
 	parser.addOption(OptCamera, OptionString,
-			 "Specify which camera to operate on, by name or by index", "camera",
+			 "Specify which camera to operate on, by id or by index", "camera",
 			 ArgumentRequired, "camera");
 	parser.addOption(OptCapture, OptionInteger,
 			 "Capture until interrupted by user or until <count> frames captured",

> @@ -322,12 +322,12 @@ int CamApp::infoConfiguration()
>  
>  void CamApp::cameraAdded(std::shared_ptr<Camera> cam)
>  {
> -	std::cout << "Camera Added: " << cam->name() << std::endl;
> +	std::cout << "Camera Added: " << cam->id() << std::endl;
>  }
>  
>  void CamApp::cameraRemoved(std::shared_ptr<Camera> cam)
>  {
> -	std::cout << "Camera Removed: " << cam->name() << std::endl;
> +	std::cout << "Camera Removed: " << cam->id() << std::endl;
>  }
>  
>  int CamApp::run()
> @@ -339,7 +339,7 @@ int CamApp::run()
>  
>  		unsigned int index = 1;
>  		for (const std::shared_ptr<Camera> &cam : cm_->cameras()) {
> -			std::cout << index << ": " << cam->name() << std::endl;
> +			std::cout << index << ": " << cam->id() << std::endl;
>  			index++;
>  		}
>  	}
> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> index 914ed4fb1612b8c9..840e87a3d8daeae4 100644
> --- a/src/gstreamer/gstlibcameraprovider.cpp
> +++ b/src/gstreamer/gstlibcameraprovider.cpp
> @@ -127,7 +127,7 @@ static GstDevice *
>  gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
>  {
>  	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> -	const gchar *name = camera->name().c_str();
> +	const gchar *name = camera->id().c_str();
>  	StreamRoles roles;
>  
>  	roles.push_back(StreamRole::VideoRecording);
> @@ -189,7 +189,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)
>  	}
>  
>  	for (const std::shared_ptr<Camera> &camera : cm->cameras()) {
> -		GST_INFO_OBJECT(self, "Found camera '%s'", camera->name().c_str());
> +		GST_INFO_OBJECT(self, "Found camera '%s'", camera->id().c_str());
>  		devices = g_list_append(devices,
>  					g_object_ref_sink(gst_libcamera_device_new(camera)));
>  	}
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 9755922ad59b22f6..0c28ae3f28529df2 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -241,12 +241,12 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
>  		cam = cm->cameras()[0];
>  	}
>  
> -	GST_INFO_OBJECT(self, "Using camera named '%s'", cam->name().c_str());
> +	GST_INFO_OBJECT(self, "Using camera '%s'", cam->id().c_str());
>  
>  	ret = cam->acquire();
>  	if (ret) {
>  		GST_ELEMENT_ERROR(self, RESOURCE, BUSY,
> -				  ("Camera name '%s' is already in use.", cam->name().c_str()),
> +				  ("Camera '%s' is already in use.", cam->id().c_str()),
>  				  ("libcamera::Camera::acquire() failed: %s", g_strerror(ret)));
>  		return false;
>  	}
> @@ -495,7 +495,7 @@ gst_libcamera_src_close(GstLibcameraSrc *self)
>  	ret = state->cam_->release();
>  	if (ret) {
>  		GST_ELEMENT_WARNING(self, RESOURCE, BUSY,
> -				    ("Camera name '%s' is still in use.", state->cam_->name().c_str()),
> +				    ("Camera '%s' is still in use.", state->cam_->id().c_str()),
>  				    ("libcamera::Camera.release() failed: %s", g_strerror(-ret)));
>  	}
>  
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 99cb5048ee06e3d3..a547542d1b67cc2b 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -265,7 +265,7 @@ public:
>  		CameraRunning,
>  	};
>  
> -	Private(PipelineHandler *pipe, const std::string &name,
> +	Private(PipelineHandler *pipe, const std::string &id,
>  		const std::set<Stream *> &streams);
>  	~Private();
>  
> @@ -277,7 +277,7 @@ public:
>  	void setState(State state);
>  
>  	std::shared_ptr<PipelineHandler> pipe_;
> -	std::string name_;
> +	std::string id_;
>  	std::set<Stream *> streams_;
>  	std::set<Stream *> activeStreams_;
>  
> @@ -286,9 +286,9 @@ private:
>  	std::atomic<State> state_;
>  };
>  
> -Camera::Private::Private(PipelineHandler *pipe, const std::string &name,
> +Camera::Private::Private(PipelineHandler *pipe, const std::string &id,
>  			 const std::set<Stream *> &streams)
> -	: pipe_(pipe->shared_from_this()), name_(name), streams_(streams),
> +	: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),
>  	  disconnected_(false), state_(CameraAvailable)
>  {
>  }
> @@ -450,15 +450,25 @@ void Camera::Private::setState(State state)
>  /**
>   * \brief Create a camera instance
>   * \param[in] pipe The pipeline handler responsible for the camera device
> - * \param[in] name The name of the camera device
> + * \param[in] id The ID of the camera device
>   * \param[in] streams Array of streams the camera provides
>   *
> - * The caller is responsible for guaranteeing unicity of the camera name.
> + * The caller is responsible for guaranteeing a stable and unique camera ID. For

s/unique camera ID/unique camera ID matching the constraints described by Camera::id()/

> + * an ID to be stable it must be the same each time the pipeline handler creates
> + * it, even between system resets.

You could drop this sentence as Camera::id() mentions it, or keep it and
s/resets/reboots/ (or s/reboot/reset/ in Camera::id()).

> Parameters that are enumerated at system

Maybe s/enumerated/allocated dynamically/ ?

> + * startup such as bus parameters which might be enumerated differently are

s/startup/startup,/
s/bus parameters which might/bus numbers that may/
s/differently/differently,/

> + * therefor not possible to use in the ID. For an ID to be unique no pipeline

s/therefor/therefore/
s/possible/suitable/

> + * should generate an ID that could conflict, either with IDs from the same

s/should/shall/

> + * pipeline handle or any other.

I'm not sure the last sentence adds much.

> + *
> + * Pipeline handlers that use a CameraSensor may use the CameraSensor::id() to
> + * generate and ID which satisfies the criteria of a stable and unique camera

s/and/an/
s/which/that/

> + * ID.
>   *
>   * \return A shared pointer to the newly created camera object
>   */
>  std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> -				       const std::string &name,
> +				       const std::string &id,
>  				       const std::set<Stream *> &streams)
>  {
>  	struct Deleter : std::default_delete<Camera> {
> @@ -468,19 +478,19 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
>  		}
>  	};
>  
> -	Camera *camera = new Camera(pipe, name, streams);
> +	Camera *camera = new Camera(pipe, id, streams);
>  
>  	return std::shared_ptr<Camera>(camera, Deleter());
>  }
>  
>  /**
> - * \brief Retrieve the name of the camera
> + * \brief Retrieve the ID of the camera

I think this should detail what the ID is, as the only other source of
information is Camera::create(), which isn't part of the application
API.

 * The camera ID is a free-form string that identifies a camera in the system.
 * IDs are guaranteed to be unique and stable: the same camera, when connected
 * to the system in the same way (e.g. in the same USB port), will have the same
 * ID across both unplug/replug and system reboots.
 *
 * Applications may store the camera ID and use it later to acquire the same
 * camera. They shall treat the ID as an opaque identifier, without interpreting
 * its value.
 *
 * Camera IDs may change when the system hardware or firmware is modified, for
 * instance when replacing a PCI USB controller or moving it to another PCI
 * slot, or updating the ACPI tables or Device Tree.

Feel free to add any information that you think it missing.

>   * \context This function is \threadsafe.
> - * \return Name of the camera device
> + * \return ID of the camera device
>   */
> -const std::string &Camera::name() const
> +const std::string &Camera::id() const
>  {
> -	return p_->name_;
> +	return p_->id_;
>  }
>  
>  /**
> @@ -506,9 +516,9 @@ const std::string &Camera::name() const
>   * application API calls by returning errors immediately.
>   */
>  
> -Camera::Camera(PipelineHandler *pipe, const std::string &name,
> +Camera::Camera(PipelineHandler *pipe, const std::string &id,
>  	       const std::set<Stream *> &streams)
> -	: p_(new Private(pipe, name, streams))
> +	: p_(new Private(pipe, id, streams))
>  {
>  }
>  
> @@ -530,7 +540,7 @@ Camera::~Camera()
>   */
>  void Camera::disconnect()
>  {
> -	LOG(Camera, Debug) << "Disconnecting camera " << name();
> +	LOG(Camera, Debug) << "Disconnecting camera " << id();
>  
>  	p_->disconnect();
>  	disconnected.emit(this);
> diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp
> index 371f6d079e2d877c..cabdcf75c223fc3e 100644
> --- a/src/libcamera/camera_controls.cpp
> +++ b/src/libcamera/camera_controls.cpp
> @@ -36,7 +36,7 @@ CameraControlValidator::CameraControlValidator(Camera *camera)
>  
>  const std::string &CameraControlValidator::name() const
>  {
> -	return camera_->name();
> +	return camera_->id();
>  }
>  
>  /**
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index c45bf33fdc1ef01b..76543b6aac8df9c4 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -181,10 +181,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,
>  	MutexLocker locker(mutex_);
>  
>  	for (std::shared_ptr<Camera> c : cameras_) {
> -		if (c->name() == camera->name()) {
> +		if (c->id() == camera->id()) {
>  			LOG(Camera, Warning)
> -				<< "Registering camera with duplicate name '"
> -				<< camera->name() << "'";
> +				<< "Registering camera with duplicate ID '"
> +				<< camera->id() << "'";
>  			break;
>  		}
>  	}
> @@ -208,7 +208,7 @@ void CameraManager::Private::removeCamera(Camera *camera)
>  		return;
>  
>  	LOG(Camera, Debug)
> -		<< "Unregistering camera '" << camera->name() << "'";
> +		<< "Unregistering camera '" << camera->id() << "'";
>  
>  	auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
>  				   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
> @@ -329,8 +329,8 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
>  }
>  
>  /**
> - * \brief Get a camera based on name
> - * \param[in] name Name of camera to get
> + * \brief Get a camera based on ID
> + * \param[in] id ID of camera to get
>   *
>   * Before calling this function the caller is responsible for ensuring that
>   * the camera manager is running.
> @@ -339,12 +339,12 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
>   *
>   * \return Shared pointer to Camera object or nullptr if camera not found
>   */
> -std::shared_ptr<Camera> CameraManager::get(const std::string &name)
> +std::shared_ptr<Camera> CameraManager::get(const std::string &id)
>  {
>  	MutexLocker locker(p_->mutex_);
>  
>  	for (std::shared_ptr<Camera> camera : p_->cameras_) {
> -		if (camera->name() == name)
> +		if (camera->id() == id)
>  			return camera;
>  	}
>  
> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> index 252191ba234c5450..2fbba37a1b0b7254 100644
> --- a/src/libcamera/framebuffer_allocator.cpp
> +++ b/src/libcamera/framebuffer_allocator.cpp
> @@ -95,7 +95,7 @@ int FrameBufferAllocator::allocate(Stream *stream)
>  	int ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);
>  	if (ret == -EINVAL)
>  		LOG(Allocator, Error)
> -			<< "Stream is not part of " << camera_->name()
> +			<< "Stream is not part of " << camera_->id()
>  			<< " active configuration";
>  	return ret;
>  }
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index c300f570d727e03e..25f890947b660ecc 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -629,7 +629,7 @@ int PipelineHandlerIPU3::start(Camera *camera)
>  
>  error:
>  	freeBuffers(camera);
> -	LOG(IPU3, Error) << "Failed to start camera " << camera->name();
> +	LOG(IPU3, Error) << "Failed to start camera " << camera->id();
>  
>  	return ret;
>  }
> @@ -642,8 +642,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  	ret |= data->imgu_->stop();
>  	ret |= data->cio2_.stop();
>  	if (ret)
> -		LOG(IPU3, Warning) << "Failed to stop camera "
> -				   << camera->name();
> +		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>  
>  	freeBuffers(camera);
>  }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c55d7325cd44d6cb..eeaf335cbcd2f93f 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -643,7 +643,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  	if (ret)
>  		return ret;
>  
> -	LOG(RPI, Info) << "Sensor: " << camera->name()
> +	LOG(RPI, Info) << "Sensor: " << camera->id()
>  		       << " - Selected mode: " << sensorFormat.toString();
>  
>  	/*
> @@ -793,7 +793,7 @@ int PipelineHandlerRPi::start(Camera *camera)
>  	ret = data->ipa_->start();
>  	if (ret) {
>  		LOG(RPI, Error)
> -			<< "Failed to start IPA for " << camera->name();
> +			<< "Failed to start IPA for " << camera->id();
>  		stop(camera);
>  		return ret;
>  	}
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 94561062c2b9d4fc..b7609cbc8f363135 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -798,7 +798,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  	if (ret) {
>  		freeBuffers(camera);
>  		LOG(RkISP1, Error)
> -			<< "Failed to start IPA " << camera->name();
> +			<< "Failed to start IPA " << camera->id();
>  		return ret;
>  	}
>  
> @@ -809,7 +809,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		data->ipa_->stop();
>  		freeBuffers(camera);
>  		LOG(RkISP1, Error)
> -			<< "Failed to start parameters " << camera->name();
> +			<< "Failed to start parameters " << camera->id();
>  		return ret;
>  	}
>  
> @@ -819,7 +819,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		data->ipa_->stop();
>  		freeBuffers(camera);
>  		LOG(RkISP1, Error)
> -			<< "Failed to start statistics " << camera->name();
> +			<< "Failed to start statistics " << camera->id();
>  		return ret;
>  	}
>  
> @@ -831,7 +831,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		freeBuffers(camera);
>  
>  		LOG(RkISP1, Error)
> -			<< "Failed to start camera " << camera->name();
> +			<< "Failed to start camera " << camera->id();
>  	}
>  
>  	activeCamera_ = camera;
> @@ -870,17 +870,17 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
>  	ret = video_->streamOff();
>  	if (ret)
>  		LOG(RkISP1, Warning)
> -			<< "Failed to stop camera " << camera->name();
> +			<< "Failed to stop camera " << camera->id();
>  
>  	ret = stat_->streamOff();
>  	if (ret)
>  		LOG(RkISP1, Warning)
> -			<< "Failed to stop statistics " << camera->name();
> +			<< "Failed to stop statistics " << camera->id();
>  
>  	ret = param_->streamOff();
>  	if (ret)
>  		LOG(RkISP1, Warning)
> -			<< "Failed to stop parameters " << camera->name();
> +			<< "Failed to stop parameters " << camera->id();
>  
>  	data->ipa_->stop();
>  
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 8fb9681c2b4fb5e8..65fbd409cfe72bf4 100644
> --- a/src/qcam/dng_writer.cpp
> +++ b/src/qcam/dng_writer.cpp
> @@ -386,8 +386,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>  	TIFFSetField(tif, TIFFTAG_DNGBACKWARDVERSION, version);
>  	TIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);
>  	TIFFSetField(tif, TIFFTAG_MAKE, "libcamera");
> -	TIFFSetField(tif, TIFFTAG_MODEL, camera->name().c_str());
> -	TIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, camera->name().c_str());
> +	TIFFSetField(tif, TIFFTAG_MODEL, camera->id().c_str());
> +	TIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, camera->id().c_str());

Can you add a \todo comment here so we don't forget to update the code
with a real model later ?

>  	TIFFSetField(tif, TIFFTAG_SOFTWARE, "qcam");
>  	TIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);
>  
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 13a4fefe89ae7525..526e50f48fd36577 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -169,7 +169,7 @@ int MainWindow::createToolbars()
>  		this, &MainWindow::switchCamera);
>  
>  	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> -		cameraCombo_->addItem(QString::fromStdString(cam->name()));
> +		cameraCombo_->addItem(QString::fromStdString(cam->id()));
>  
>  	toolbar_->addWidget(cameraCombo_);
>  
> @@ -241,11 +241,11 @@ void MainWindow::switchCamera(int index)
>  	const std::shared_ptr<Camera> &cam = cameras[index];
>  
>  	if (cam->acquire()) {
> -		qInfo() << "Failed to acquire camera" << cam->name().c_str();
> +		qInfo() << "Failed to acquire camera" << cam->id().c_str();
>  		return;
>  	}
>  
> -	qInfo() << "Switching to camera" << cam->name().c_str();
> +	qInfo() << "Switching to camera" << cam->id().c_str();
>  
>  	/*
>  	 * Stop the capture session, release the current camera, replace it with
> @@ -266,11 +266,11 @@ std::string MainWindow::chooseCamera()
>  
>  	/* If only one camera is available, use it automatically. */
>  	if (cm_->cameras().size() == 1)
> -		return cm_->cameras()[0]->name();
> +		return cm_->cameras()[0]->id();
>  
>  	/* Present a dialog box to pick a camera. */
>  	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> -		cameras.append(QString::fromStdString(cam->name()));
> +		cameras.append(QString::fromStdString(cam->id()));
>  
>  	QString name = QInputDialog::getItem(this, "Select Camera",
>  					     "Camera:", cameras, 0,

And here,

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 13a4fefe89ae..b67c399d0abe 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -272,13 +272,13 @@ std::string MainWindow::chooseCamera()
 	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
 		cameras.append(QString::fromStdString(cam->name()));

-	QString name = QInputDialog::getItem(this, "Select Camera",
-					     "Camera:", cameras, 0,
-					     false, &result);
+	QString id = QInputDialog::getItem(this, "Select Camera",
+					   "Camera:", cameras, 0,
+					   false, &result);
 	if (!result)
 		return std::string();

-	return name.toStdString();
+	return id.toStdString();
 }

 int MainWindow::openCamera()

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> @@ -582,7 +582,7 @@ void MainWindow::processHotplug(HotplugEvent *e)
>  	HotplugEvent::PlugEvent event = e->hotplugEvent();
>  
>  	if (event == HotplugEvent::HotPlug) {
> -		cameraCombo_->addItem(QString::fromStdString(camera->name()));
> +		cameraCombo_->addItem(QString::fromStdString(camera->id()));
>  	} else if (event == HotplugEvent::HotUnplug) {
>  		/* Check if the currently-streaming camera is removed. */
>  		if (camera == camera_.get()) {
> @@ -592,14 +592,14 @@ void MainWindow::processHotplug(HotplugEvent *e)
>  			cameraCombo_->setCurrentIndex(0);
>  		}
>  
> -		int camIndex = cameraCombo_->findText(QString::fromStdString(camera->name()));
> +		int camIndex = cameraCombo_->findText(QString::fromStdString(camera->id()));
>  		cameraCombo_->removeItem(camIndex);
>  	}
>  }
>  
>  void MainWindow::addCamera(std::shared_ptr<Camera> camera)
>  {
> -	qInfo() << "Adding new camera:" << camera->name().c_str();
> +	qInfo() << "Adding new camera:" << camera->id().c_str();
>  	QCoreApplication::postEvent(this,
>  				    new HotplugEvent(std::move(camera),
>  						     HotplugEvent::HotPlug));
> @@ -607,7 +607,7 @@ void MainWindow::addCamera(std::shared_ptr<Camera> camera)
>  
>  void MainWindow::removeCamera(std::shared_ptr<Camera> camera)
>  {
> -	qInfo() << "Removing camera:" << camera->name().c_str();
> +	qInfo() << "Removing camera:" << camera->id().c_str();
>  	QCoreApplication::postEvent(this,
>  				    new HotplugEvent(std::move(camera),
>  						     HotplugEvent::HotUnplug));
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 50ab7e66cdc02fb1..63affaeec1b3af11 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -189,7 +189,7 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
>  
>  	utils::strlcpy(reinterpret_cast<char *>(capabilities_.driver), driver.c_str(),
>  		       sizeof(capabilities_.driver));
> -	utils::strlcpy(reinterpret_cast<char *>(capabilities_.card), camera->name().c_str(),
> +	utils::strlcpy(reinterpret_cast<char *>(capabilities_.card), camera->id().c_str(),
>  		       sizeof(capabilities_.card));
>  	utils::strlcpy(reinterpret_cast<char *>(capabilities_.bus_info), bus_info.c_str(),
>  		       sizeof(capabilities_.bus_info));
> diff --git a/test/pipeline/ipu3/ipu3_pipeline_test.cpp b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> index 34998f8f6d04473f..9e647af5fdf8ee7f 100644
> --- a/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> +++ b/test/pipeline/ipu3/ipu3_pipeline_test.cpp
> @@ -106,7 +106,7 @@ int IPU3PipelineTest::run()
>  {
>  	auto cameras = cameraManager_->cameras();
>  	for (const std::shared_ptr<Camera> &cam : cameras)
> -		cout << "Found camera '" << cam->name() << "'" << endl;
> +		cout << "Found camera '" << cam->id() << "'" << endl;
>  
>  	if (cameras.size() != sensors_) {
>  		cerr << cameras.size() << " cameras registered, but " << sensors_
> diff --git a/test/pipeline/rkisp1/rkisp1_pipeline_test.cpp b/test/pipeline/rkisp1/rkisp1_pipeline_test.cpp
> index b6678ce76e9af193..acaf3c33b529d31d 100644
> --- a/test/pipeline/rkisp1/rkisp1_pipeline_test.cpp
> +++ b/test/pipeline/rkisp1/rkisp1_pipeline_test.cpp
> @@ -95,7 +95,7 @@ int RKISP1PipelineTest::run()
>  {
>  	auto cameras = cameraManager_->cameras();
>  	for (const std::shared_ptr<Camera> &cam : cameras)
> -		cout << "Found camera '" << cam->name() << "'" << endl;
> +		cout << "Found camera '" << cam->id() << "'" << endl;
>  
>  	if (cameras.size() != sensors_) {
>  		cerr << cameras.size() << " cameras registered, but " << sensors_