[libcamera-devel,v5,2/4] libcamera: add model() for retriving model name in V4L2Subdevice

14806 diff mbox series

CameraSensor retrives model name from media entity. Move the heuristics
method into V4L2Subdevice, so CameraLens can reuse the function.

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
---
 include/libcamera/internal/v4l2_subdevice.h |  5 +++
 src/libcamera/camera_sensor.cpp             | 13 ++-----
 src/libcamera/v4l2_subdevice.cpp            | 40 +++++++++++++++++++++
 3 files changed, 47 insertions(+), 11 deletions(-)

Hi Han-lin,

Thank you for the patch.

On Fri, Nov 26, 2021 at 07:29:01PM +0800, Han-Lin Chen wrote:
> CameraSensor retrives model name from media entity. Move the heuristics
> method into V4L2Subdevice, so CameraLens can reuse the function.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> ---
>  include/libcamera/internal/v4l2_subdevice.h |  5 +++
>  src/libcamera/camera_sensor.cpp             | 13 ++-----
>  src/libcamera/v4l2_subdevice.cpp            | 40 +++++++++++++++++++++
>  3 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index 97b89fb9..794c80c6 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -61,6 +61,8 @@ public:
>  	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  		      Whence whence = ActiveFormat);
>  
> +	std::string model() { return model_; }
> +
>  	static std::unique_ptr<V4L2Subdevice>
>  	fromEntityName(const MediaDevice *media, const std::string &entity);
>  
> @@ -73,8 +75,11 @@ private:
>  	std::vector<unsigned int> enumPadCodes(unsigned int pad);
>  	std::vector<SizeRange> enumPadSizes(unsigned int pad,
>  					    unsigned int code);
> +	void generateModel();
>  
>  	const MediaEntity *entity_;
> +
> +	std::string model_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..3659ff2d 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -13,7 +13,6 @@
>  #include <iomanip>
>  #include <limits.h>
>  #include <math.h>
> -#include <regex>
>  #include <string.h>
>  
>  #include <libcamera/property_ids.h>
> @@ -366,15 +365,7 @@ int CameraSensor::initProperties()
>  	 * part of the entity name before the first space if the name contains
>  	 * an I2C address, and use the full entity name otherwise.
>  	 */

Shouldn't the comment be removed too ?

> -	std::string entityName = entity_->name();
> -	std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> -	std::smatch match;
> -
> -	if (std::regex_search(entityName, match, i2cRegex))
> -		model_ = entityName.substr(0, entityName.find(' '));
> -	else
> -		model_ = entityName;
> -
> +	model_ = subdev_->model();
>  	properties_.set(properties::Model, utils::toAscii(model_));
>  
>  	/* Generate a unique ID for the sensor. */
> @@ -832,7 +823,7 @@ int CameraSensor::generateId()
>  	/*
>  	 * Virtual sensors not described in firmware
>  	 *
> -	 * Verify it's a platform device and construct ID from the deive path
> +	 * Verify it's a platform device and construct ID from the device path
>  	 * and model of sensor.
>  	 */
>  	if (devPath.find("/sys/devices/platform/", 0) == 0) {
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 023e2328..0e194081 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -9,6 +9,7 @@
>  
>  #include <fcntl.h>
>  #include <iomanip>
> +#include <regex>
>  #include <sstream>
>  #include <string.h>
>  #include <sys/ioctl.h>
> @@ -239,6 +240,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
>  V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
>  	: V4L2Device(entity->deviceNode()), entity_(entity)
>  {
> +	generateModel();

How about doing this the first time model() is called, so that we won't
try to generate a model for every single subdev when only a few of them
will need it ?

>  }
>  
>  V4L2Subdevice::~V4L2Subdevice()
> @@ -442,6 +444,12 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  	return 0;
>  }
>  
> +/**
> + * \fn V4L2Subdevice::model()
> + * \brief Retrieve the model name
> + * \return The model name of the device
> + */
> +
>  /**
>   * \brief Create a new video subdevice instance from \a entity in media device
>   * \a media
> @@ -525,4 +533,36 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
>  	return sizes;
>  }
>  
> +void V4L2Subdevice::generateModel()
> +{
> +	/*
> +	 * Extract model name from the media entity name.
> +	 *
> +	 * There is no standardized naming scheme for sensor entities in the
> +	 * Linux kernel at the moment.
> +	 *
> +	 * - The most common rule, used by I2C sensors, associates the model
> +	 *   name with the I2C bus number and address (e.g. 'imx219 0-0010').
> +	 *
> +	 * - When the sensor exposes multiple subdevs, the model name is
> +	 *   usually followed by a function name, as in the smiapp driver (e.g.
> +	 *   'jt8ew9 pixel_array 0-0010').
> +	 *
> +	 * - The vimc driver names its sensors 'Sensor A' and 'Sensor B'.
> +	 *
> +	 * Other schemes probably exist. As a best effort heuristic, use the
> +	 * part of the entity name before the first space if the name contains
> +	 * an I2C address, and use the full entity name otherwise.
> +	 */
> +	std::string entityName = entity_->name();
> +	std::regex i2cRegex{ " [0-9]+-[0-9a-f]{4}" };
> +	std::smatch match;
> +
> +	std::string model;
> +	if (std::regex_search(entityName, match, i2cRegex))
> +		model_ = entityName.substr(0, entityName.find(' '));
> +	else
> +		model_ = entityName;
> +}
> +
>  } /* namespace libcamera */