[libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's RED/BLUE balance controls when present

[Naushir Patuck]

Hi David,

Thank you for your patch.

On Wed, 24 Mar 2021 at 11:44, David Plowman <david.plowman at raspberrypi.com>
wrote:

> If the sensor exposes V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE
> controls, assume it wants to be told the colour gains. We will need to
> add these to the list of delayed controls we can apply.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp             | 17 ++++++++++++++---
>  .../pipeline/raspberrypi/raspberrypi.cpp        |  9 +++++++++
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 1c928b72..7437a77e 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -1030,13 +1030,24 @@ void IPARPi::processStats(unsigned int bufferId)
>         RPiController::StatisticsPtr statistics =
> std::make_shared<bcm2835_isp_stats>(*stats);
>         controller_.Process(statistics, &rpiMetadata_);
>
> +       ControlList ctrls(sensorCtrls_);
> +
>         struct AgcStatus agcStatus;
> -       if (rpiMetadata_.Get("agc.status", agcStatus) == 0) {
> -               ControlList ctrls(sensorCtrls_);
> +       if (rpiMetadata_.Get("agc.status", agcStatus) == 0)
>                 applyAGC(&agcStatus, ctrls);
>
> -               setDelayedControls.emit(ctrls);
> +       struct AwbStatus awbStatus;
> +       if (rpiMetadata_.Get("awb.status", awbStatus) == 0 &&
> +           sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end()
> &&
> +           sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) !=
> sensorCtrls_.end()) {
> +               ctrls.set(V4L2_CID_RED_BALANCE,
> +                         static_cast<int32_t>(awbStatus.gain_r * 256));
> +               ctrls.set(V4L2_CID_BLUE_BALANCE,
> +                         static_cast<int32_t>(awbStatus.gain_b * 256));
>

The 256 const bothers me slightly.  Is this FP multiplier going to be true
for all sensors?
Any way we can deduce this (perhaps from the min value)?


>         }
> +
> +       if (!ctrls.empty())
> +               setDelayedControls.emit(ctrls);
>  }
>
>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList
> &ctrls)
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 2cac802c..7bac1503 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1027,6 +1027,15 @@ bool PipelineHandlerRPi::match(DeviceEnumerator
> *enumerator)
>                 { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false }
> },
>                 { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }
>         };
> +
> +       /* If the sensor exposes red/blue balance controls, we will update
> them. */
> +       const ControlInfoMap &sensorControls =
> data->unicam_[Unicam::Image].dev()->controls();
> +       if (sensorControls.find(V4L2_CID_RED_BALANCE) !=
> sensorControls.end() &&
> +           sensorControls.find(V4L2_CID_BLUE_BALANCE) !=
> sensorControls.end()) {
> +               params[V4L2_CID_RED_BALANCE] = { sensorConfig.vblankDelay,
> false };
> +               params[V4L2_CID_BLUE_BALANCE] = {
> sensorConfig.vblankDelay, false };
>

Should the delay value be 1 here, as the change likely happens on the very
next frame?

Regards,
Naush



> +       }
> +
>         data->delayedCtrls_ =
> std::make_unique<DelayedControls>(data->unicam_[Unicam::Image].dev(),
> params);
>         data->sensorMetadata_ = sensorConfig.sensorMetadata;
>
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210407/0a3ba634/attachment.htm>