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

14888 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 |  4 ++
 src/libcamera/camera_sensor.cpp             | 32 +---------------
 src/libcamera/v4l2_subdevice.cpp            | 42 +++++++++++++++++++++
 3 files changed, 48 insertions(+), 30 deletions(-)

Quoting Han-Lin Chen (2021-11-30 10:51:55)
> CameraSensor retrives model name from media entity. Move the heuristics
> method into V4L2Subdevice, so CameraLens can reuse the function.
> 

(The following line should be added while applying or for a next
version if needed).

"While here, fix a small typo for 'device'."

> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> ---
>  include/libcamera/internal/v4l2_subdevice.h |  4 ++
>  src/libcamera/camera_sensor.cpp             | 32 +---------------
>  src/libcamera/v4l2_subdevice.cpp            | 42 +++++++++++++++++++++
>  3 files changed, 48 insertions(+), 30 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index 97b89fb9..763c4dec 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();
> +
>         static std::unique_ptr<V4L2Subdevice>
>         fromEntityName(const MediaDevice *media, const std::string &entity);
>  
> @@ -75,6 +77,8 @@ private:
>                                             unsigned int code);
>  
>         const MediaEntity *entity_;
> +
> +       std::string model_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..6151b32e 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>
> @@ -347,34 +346,7 @@ void CameraSensor::initTestPatternModes(
>  
>  int CameraSensor::initProperties()
>  {
> -       /*
> -        * Extract the camera sensor 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;
> -
> -       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 +804,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..17ec9fdf 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>
> @@ -442,6 +443,47 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>         return 0;
>  }
>  
> +/**
> + * \brief Retrieve the model name
> + * \return The model name of the device
> + */
> +std::string V4L2Subdevice::model()
> +{
> +       if (!model_.empty())
> +               return model_;
> +

We don't normally use lazy initialisation, but I think this is OK in
this case.

Maybe we might have a subdevice which doesn't ever read the model, so
there wouldn't be any point parsing the information...

And it can't be accessed uninitialised, so it seems fine to me.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> +       /*
> +        * 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;
> +
> +       return model_;
> +}
> +
>  /**
>   * \brief Create a new video subdevice instance from \a entity in media device
>   * \a media
> -- 
> 2.34.0.rc2.393.gf8c9666880-goog
>