[libcamera-devel] [PATCH v1 2/2] ipa: vc4: Implement the StatsOutputEnable vendor control

[David Plowman]

Hi Naush

Thanks for this. Looks good!

On Mon, 4 Dec 2023 at 16:18, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Implement the StatsOutputEnable control for the VC4 IPA. When set,
> this outputs the ISP statistics as a uint8_t span through the Bcm2835StatsOutput
> metadata control.
>
> To get this working, IpaBase::libcameraMetadata_ is moved from a private
> to a protected member variable. This makes it accessable to the VC4
> derived IPA class.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp | 12 +++++++++---
>  src/ipa/rpi/common/ipa_base.h   |  3 ++-
>  src/ipa/rpi/vc4/vc4.cpp         |  6 ++++++
>  3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 6ac9d5de2f88..c4d17a2130c8 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -70,7 +70,8 @@ const ControlInfoMap::Map ipaControls{
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>         { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },
> -       { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> +       { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
> +       { &controls::rpi::StatsOutputEnable, ControlInfo(false, true) },
>  };
>
>  /* IPA controls handled conditionally, if the sensor is not mono */
> @@ -100,8 +101,9 @@ LOG_DEFINE_CATEGORY(IPARPI)
>  namespace ipa::RPi {
>
>  IpaBase::IpaBase()
> -       : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), frameCount_(0),
> -         mistrustCount_(0), lastRunTimestamp_(0), firstStart_(true), flickerState_({ 0, 0s })
> +       : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),
> +         frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), firstStart_(true),
> +         flickerState_({ 0, 0s })
>  {
>  }
>
> @@ -1135,6 +1137,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                         break;
>                 }
>
> +               case controls::rpi::STATS_OUTPUT_ENABLE:
> +                       statsMetadataOutput_ = ctrl.second.get<bool>();
> +                       break;
> +
>                 default:
>                         LOG(IPARPI, Warning)
>                                 << "Ctrl " << controls::controls.at(ctrl.first)->name()
> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h
> index eaa9f71182ed..4db4411eed7c 100644
> --- a/src/ipa/rpi/common/ipa_base.h
> +++ b/src/ipa/rpi/common/ipa_base.h
> @@ -61,6 +61,8 @@ protected:
>         /* Track the frame length times over FrameLengthsQueueSize frames. */
>         std::deque<utils::Duration> frameLengths_;
>         utils::Duration lastTimeout_;
> +       ControlList libcameraMetadata_;
> +       bool statsMetadataOutput_;
>
>  private:
>         /* Number of metadata objects available in the context list. */
> @@ -89,7 +91,6 @@ private:
>
>         bool lensPresent_;
>         bool monoSensor_;
> -       ControlList libcameraMetadata_;
>
>         std::array<RPiController::Metadata, numMetadataContexts> rpiMetadata_;
>
> diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp
> index c165a5b8b0b6..ccd4141943dd 100644
> --- a/src/ipa/rpi/vc4/vc4.cpp
> +++ b/src/ipa/rpi/vc4/vc4.cpp
> @@ -11,6 +11,7 @@
>  #include <linux/bcm2835-isp.h>
>
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/span.h>
>  #include <libcamera/control_ids.h>
>  #include <libcamera/ipa/ipa_module_info.h>
>
> @@ -245,6 +246,11 @@ RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span<uint8_t> mem)
>                                                   stats->focus_stats[i].contrast_val_num[1][1],
>                                                   stats->focus_stats[i].contrast_val_num[1][0] });
>
> +       if (statsMetadataOutput_) {
> +               Span<uint8_t> statsSpan((uint8_t *)(stats), sizeof(bcm2835_isp_stats));

I thought folks preferred C++ style casts these days, but honestly...
I really don't care!!

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

> +               libcameraMetadata_.set(controls::rpi::Bcm2835StatsOutput, statsSpan);
> +       }
> +
>         return statistics;
>  }
>
> --
> 2.34.1
>