[libcamera-devel] libcamera: camera_sensor: Enable to set a test pattern mode

14042 diff mbox series

This adds a function to set a camera sensor driver a test pattern
mode.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 include/libcamera/internal/camera_sensor.h |  5 +++
 src/libcamera/camera_sensor.cpp            | 46 +++++++++++++++++++---
 2 files changed, 45 insertions(+), 6 deletions(-)

Hi Hiro,

Quoting Hirokazu Honda (2021-10-05 05:37:23)
> This adds a function to set a camera sensor driver a test pattern
> mode.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  include/libcamera/internal/camera_sensor.h |  5 +++
>  src/libcamera/camera_sensor.cpp            | 46 +++++++++++++++++++---
>  2 files changed, 45 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index d25a1165..df35df74 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -24,6 +24,7 @@
>  namespace libcamera {
>  
>  class BayerFormat;
> +struct CameraSensorProperties;

To maintain sort ordering, I'd put this in a separate group after the
class forward declarations.

>  class MediaEntity;
>  
>  class CameraSensor : protected Loggable
> @@ -44,6 +45,7 @@ public:
>         {
>                 return testPatternModes_;
>         }
> +       int setTestPatternMode(int32_t testPatternMode);
>  
>         V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
>                                       const Size &size) const;
> @@ -78,6 +80,8 @@ private:
>         std::unique_ptr<V4L2Subdevice> subdev_;
>         unsigned int pad_;
>  
> +       const CameraSensorProperties *props_;
> +
>         std::string model_;
>         std::string id_;
>  
> @@ -85,6 +89,7 @@ private:
>         std::vector<unsigned int> mbusCodes_;
>         std::vector<Size> sizes_;
>         std::vector<int32_t> testPatternModes_;
> +       int32_t testPatternMode_;
>  
>         Size pixelArraySize_;
>         Rectangle activeArea_;
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 9fdb8c09..25b8c626 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -54,8 +54,8 @@ LOG_DEFINE_CATEGORY(CameraSensor)
>   * Once constructed the instance must be initialized with init().
>   */
>  CameraSensor::CameraSensor(const MediaEntity *entity)
> -       : entity_(entity), pad_(UINT_MAX), bayerFormat_(nullptr),
> -         properties_(properties::properties)
> +       : entity_(entity), pad_(UINT_MAX), testPatternMode_(-1),
> +         bayerFormat_(nullptr), properties_(properties::properties)
>  {
>  }
>  
> @@ -300,14 +300,14 @@ void CameraSensor::initVimcDefaultProperties()
>  
>  void CameraSensor::initStaticProperties()
>  {
> -       const CameraSensorProperties *props = CameraSensorProperties::get(model_);
> -       if (!props)
> +       props_ = CameraSensorProperties::get(model_);
> +       if (!props_)

I know it didn't have one before, but I wonder if we should print a
DEBUG or WARNING level print here? It seems that the properties are
going to be used more, so if they arent' available for a given sensor, a
warning could be helpful.

>                 return;
>  
>         /* Register the properties retrieved from the sensor database. */
> -       properties_.set(properties::UnitCellSize, props->unitCellSize);
> +       properties_.set(properties::UnitCellSize, props_->unitCellSize);
>  
> -       initTestPatternModes(props->testPatternModes);
> +       initTestPatternModes(props_->testPatternModes);
>  }
>  
>  void CameraSensor::initTestPatternModes(
> @@ -531,6 +531,40 @@ Size CameraSensor::resolution() const
>   * \return The list of test pattern modes
>   */
>  
> +/**
> + * \brief Set the camera sensor a specified controls::TestPatternMode
> + * \param[in] testPatternMode test pattern mode control value to set the camera
> + * sensor
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::setTestPatternMode(int32_t testPatternMode)
> +{
> +       if (testPatternMode_ == testPatternMode)
> +               return 0;
> +
> +       if (!props_)

Unless it's better to print only when the properties that were needed
were not found?

But other than that, it looks quite straightforward.

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

> +               return -EINVAL;
> +
> +       auto it = props_->testPatternModes.find(testPatternMode);
> +       if (it == props_->testPatternModes.end()) {
> +               LOG(CameraSensor, Error) << "Unsupported test pattern mode: "
> +                                        << testPatternMode;
> +               return -EINVAL;
> +       }
> +
> +       ControlList ctrls{ controls() };
> +       ctrls.set(V4L2_CID_TEST_PATTERN, it->second);
> +
> +       int ret = setControls(&ctrls);
> +       if (ret)
> +               return ret;
> +
> +       testPatternMode_ = testPatternMode;
> +
> +       return 0;
> +}
> +
>  /**
>   * \brief Retrieve the best sensor format for a desired output
>   * \param[in] mbusCodes The list of acceptable media bus codes
> -- 
> 2.33.0.800.g4c38ced690-goog
>