[libcamera-devel] [PATCH v2 2/2] libcamera: raspberrypi: Fetch correct value for sensor's modeSensitivity

[Naushir Patuck]

Hi David,

Thank you for your patch.

On Wed, 22 Sept 2021 at 14:29, David Plowman <david.plowman at raspberrypi.com>
wrote:

> These changes retrieve the correct value for sensitivity of the mode
> selected for the sensor. This value is known to the CamHelper which
> passes it across to the pipeline handler so that it can be set
> correctly in the CameraConfiguration.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>

I agree with Kieran that we might want to consider passing libcamera
controls from the IPA
instead of v4l2 controls.  The mode sensitivity can then be put into a
control list. But that is
not for this series.

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>


> ---
>  include/libcamera/ipa/raspberrypi.mojom            |  7 ++++++-
>  src/ipa/raspberrypi/raspberrypi.cpp                |  7 +++++--
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++---
>  3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom
> b/include/libcamera/ipa/raspberrypi.mojom
> index e453d46c..a92a76f8 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -38,6 +38,10 @@ struct IPAConfig {
>         libcamera.FileDescriptor lsTableHandle;
>  };
>
> +struct IPAConfigResult {
> +       float modeSensitivity;
> +};
> +
>  struct StartConfig {
>         libcamera.ControlList controls;
>         int32 dropFrameCount;
> @@ -57,6 +61,7 @@ interface IPARPiInterface {
>          * \param[in] entityControls Controls provided by the pipeline
> entities
>          * \param[in] ipaConfig Pipeline-handler-specific configuration
> data
>          * \param[out] controls Controls to apply by the pipeline entity
> +        * \param[out] result Other results that the pipeline handler may
> require
>          *
>          * This function shall be called when the camera is configured to
> inform
>          * the IPA of the camera's streams and the sensor settings.
> @@ -71,7 +76,7 @@ interface IPARPiInterface {
>                   map<uint32, libcamera.IPAStream> streamConfig,
>                   map<uint32, libcamera.ControlInfoMap> entityControls,
>                   IPAConfig ipaConfig)
> -               => (int32 ret, libcamera.ControlList controls);
> +               => (int32 ret, libcamera.ControlList controls,
> IPAConfigResult result);
>
>         /**
>          * \fn mapBuffers()
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 047123ab..796e6d15 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -97,7 +97,7 @@ public:
>                       const std::map<unsigned int, IPAStream>
> &streamConfig,
>                       const std::map<unsigned int, ControlInfoMap>
> &entityControls,
>                       const ipa::RPi::IPAConfig &data,
> -                     ControlList *controls) override;
> +                     ControlList *controls, ipa::RPi::IPAConfigResult
> *result) override;
>         void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>         void unmapBuffers(const std::vector<unsigned int> &ids) override;
>         void signalStatReady(const uint32_t bufferId) override;
> @@ -337,7 +337,7 @@ int IPARPi::configure(const IPACameraSensorInfo
> &sensorInfo,
>                       [[maybe_unused]] const std::map<unsigned int,
> IPAStream> &streamConfig,
>                       const std::map<unsigned int, ControlInfoMap>
> &entityControls,
>                       const ipa::RPi::IPAConfig &ipaConfig,
> -                     ControlList *controls)
> +                     ControlList *controls, ipa::RPi::IPAConfigResult
> *result)
>  {
>         if (entityControls.size() != 2) {
>                 LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> @@ -389,6 +389,9 @@ int IPARPi::configure(const IPACameraSensorInfo
> &sensorInfo,
>         /* Pass the camera mode to the CamHelper to setup algorithms. */
>         helper_->SetCameraMode(mode_);
>
> +       /* The pipeline handler passes out the mode's sensitivity. */
> +       result->modeSensitivity = mode_.sensitivity;
> +
>         if (firstStart_) {
>                 /* Supply initial values for frame durations. */
>                 applyFrameDurations(defaultMinFrameDuration,
> defaultMaxFrameDuration);
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 0bdfa727..caf0030e 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -147,7 +147,7 @@ public:
>         void frameStarted(uint32_t sequence);
>
>         int loadIPA(ipa::RPi::SensorConfig *sensorConfig);
> -       int configureIPA(const CameraConfiguration *config);
> +       int configureIPA(CameraConfiguration *config);
>
>         void statsMetadataComplete(uint32_t bufferId, const ControlList
> &controls);
>         void runIsp(uint32_t bufferId);
> @@ -1250,7 +1250,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig
> *sensorConfig)
>         return ipa_->init(settings, sensorConfig);
>  }
>
> -int RPiCameraData::configureIPA(const CameraConfiguration *config)
> +int RPiCameraData::configureIPA(CameraConfiguration *config)
>  {
>         /* We know config must be an RPiCameraConfiguration. */
>         const RPiCameraConfiguration *rpiConfig =
> @@ -1299,13 +1299,17 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
>
>         /* Ready the IPA - it must know about the sensor resolution. */
>         ControlList controls;
> +       ipa::RPi::IPAConfigResult result;
>         ret = ipa_->configure(sensorInfo_, streamConfig, entityControls,
> ipaConfig,
> -                             &controls);
> +                             &controls, &result);
>         if (ret < 0) {
>                 LOG(RPI, Error) << "IPA configuration failed!";
>                 return -EPIPE;
>         }
>
> +       /* Store the mode sensitivity for the application. */
> +       config->modeSensitivity = result.modeSensitivity;
> +
>         /*
>          * Configure the H/V flip controls based on the combination of
>          * the sensor and user transform.
> --
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220413/216df81f/attachment.htm>