[v3,08/39] libcamera: swstats_cpu: Add processFrame() method

24653 diff mbox series

From: Hans de Goede <hdegoede@redhat.com>

Add a method to the SwstatsCpu class to process a whole Framebuffer in
one go, rather then line by line. This is useful for gathering stats
when debayering is not necessary or is not done on the CPU.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[bod: various rebase spalts fixed]
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../internal/software_isp/swstats_cpu.h       | 15 ++++-
 src/libcamera/software_isp/software_isp.cpp   |  5 +-
 src/libcamera/software_isp/swstats_cpu.cpp    | 55 ++++++++++++++++++-
 3 files changed, 70 insertions(+), 5 deletions(-)

Quoting Bryan O'Donoghue (2025-10-15 02:22:20)
> From: Hans de Goede <hdegoede@redhat.com>
> 
> Add a method to the SwstatsCpu class to process a whole Framebuffer in
> one go, rather then line by line. This is useful for gathering stats
> when debayering is not necessary or is not done on the CPU.
> 
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> [bod: various rebase spalts fixed]

s/spalts/splats/

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../internal/software_isp/swstats_cpu.h       | 15 ++++-
>  src/libcamera/software_isp/software_isp.cpp   |  5 +-
>  src/libcamera/software_isp/swstats_cpu.cpp    | 55 ++++++++++++++++++-
>  3 files changed, 70 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/swstats_cpu.h b/include/libcamera/internal/software_isp/swstats_cpu.h
> index fae575f8..64b3e23f 100644
> --- a/include/libcamera/internal/software_isp/swstats_cpu.h
> +++ b/include/libcamera/internal/software_isp/swstats_cpu.h
> @@ -18,18 +18,23 @@
>  #include <libcamera/geometry.h>
>  
>  #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/framebuffer.h"
> +#include "libcamera/internal/global_configuration.h"
>  #include "libcamera/internal/shared_mem_object.h"
>  #include "libcamera/internal/software_isp/swisp_stats.h"
>  
> +#include "benchmark.h"
> +
>  namespace libcamera {
>  
>  class PixelFormat;
> +class MappedFrameBuffer;
>  struct StreamConfiguration;
>  
>  class SwStatsCpu
>  {
>  public:
> -       SwStatsCpu();
> +       SwStatsCpu(const GlobalConfiguration &configuration);
>         ~SwStatsCpu() = default;
>  
>         /*
> @@ -50,6 +55,7 @@ public:
>         void setWindow(const Rectangle &window);
>         void startFrame(uint32_t frame);
>         void finishFrame(uint32_t frame, uint32_t bufferId);
> +       void processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input);
>  
>         void processLine0(uint32_t frame, unsigned int y, const uint8_t *src[])
>         {
> @@ -79,6 +85,7 @@ public:
>  
>  private:
>         using statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);
> +       using processFrameFn = void (SwStatsCpu::*)(MappedFrameBuffer &in);
>  
>         int setupStandardBayerOrder(BayerFormat::Order order);
>         /* Bayer 8 bpp unpacked */
> @@ -91,6 +98,10 @@ private:
>         void statsBGGR10PLine0(const uint8_t *src[]);
>         void statsGBRG10PLine0(const uint8_t *src[]);
>  
> +       void processBayerFrame2(MappedFrameBuffer &in);
> +
> +       processFrameFn processFrame_;
> +
>         /* Variables set by configure(), used every line */
>         statsProcessFn stats0_;
>         statsProcessFn stats2_;
> @@ -103,9 +114,11 @@ private:
>         Size patternSize_;
>  
>         unsigned int xShift_;
> +       unsigned int stride_;
>  
>         SharedMemObject<SwIspStats> sharedStats_;
>         SwIspStats stats_;
> +       Benchmark bench_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index b7651b7d..6f1f88fe 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -107,14 +107,15 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>                 return;
>         }
>  
> -       auto stats = std::make_unique<SwStatsCpu>();
> +       const GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();
> +
> +       auto stats = std::make_unique<SwStatsCpu>(configuration);
>         if (!stats->isValid()) {
>                 LOG(SoftwareIsp, Error) << "Failed to create SwStatsCpu object";
>                 return;
>         }
>         stats->statsReady.connect(this, &SoftwareIsp::statsReady);
>  
> -       const GlobalConfiguration &configuration = pipe->cameraManager()->_d()->configuration();
>         debayer_ = std::make_unique<DebayerCpu>(std::move(stats), configuration);
>         debayer_->inputBufferReady.connect(this, &SoftwareIsp::inputReady);
>         debayer_->outputBufferReady.connect(this, &SoftwareIsp::outputReady);
> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
> index 55e764b0..48d12c51 100644
> --- a/src/libcamera/software_isp/swstats_cpu.cpp
> +++ b/src/libcamera/software_isp/swstats_cpu.cpp
> @@ -16,6 +16,7 @@
>  #include <libcamera/stream.h>
>  
>  #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/mapped_framebuffer.h"
>  
>  namespace libcamera {
>  
> @@ -144,8 +145,8 @@ namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(SwStatsCpu)
>  
> -SwStatsCpu::SwStatsCpu()
> -       : sharedStats_("softIsp_stats")
> +SwStatsCpu::SwStatsCpu(const GlobalConfiguration &configuration)
> +       : sharedStats_("softIsp_stats"), bench_(configuration)
>  {
>         if (!sharedStats_)
>                 LOG(SwStatsCpu, Error)
> @@ -386,11 +387,14 @@ int SwStatsCpu::setupStandardBayerOrder(BayerFormat::Order order)
>   */
>  int SwStatsCpu::configure(const StreamConfiguration &inputCfg)
>  {
> +       stride_ = inputCfg.stride;
> +
>         BayerFormat bayerFormat =
>                 BayerFormat::fromPixelFormat(inputCfg.pixelFormat);
>  
>         if (bayerFormat.packing == BayerFormat::Packing::None &&
>             setupStandardBayerOrder(bayerFormat.order) == 0) {
> +               processFrame_ = &SwStatsCpu::processBayerFrame2;
>                 switch (bayerFormat.bitDepth) {
>                 case 8:
>                         stats0_ = &SwStatsCpu::statsBGGR8Line0;
> @@ -411,6 +415,7 @@ int SwStatsCpu::configure(const StreamConfiguration &inputCfg)
>                 /* Skip every 3th and 4th line, sample every other 2x2 block */
>                 ySkipMask_ = 0x02;
>                 xShift_ = 0;
> +               processFrame_ = &SwStatsCpu::processBayerFrame2;

I thought this was duplicated in the same scope - but looking up after
it's applied looks correct here.


>  
>                 switch (bayerFormat.order) {
>                 case BayerFormat::BGGR:
> @@ -475,4 +480,50 @@ void SwStatsCpu::setWindow(const Rectangle &window)
>         window_.height &= ~(patternSize_.height - 1);
>  }
>  
> +void SwStatsCpu::processBayerFrame2(MappedFrameBuffer &in)
> +{
> +       const uint8_t *src = in.planes()[0].data();
> +       const uint8_t *linePointers[3];
> +
> +       /* Adjust src for starting at window_.y */
> +       src += window_.y * stride_;
> +
> +       for (unsigned int y = 0; y < window_.height; y += 2) {
> +               if (y & ySkipMask_) {
> +                       src += stride_ * 2;
> +                       continue;
> +               }
> +
> +               /* linePointers[0] is not used by any stats0_ functions */

Aha this looks bizarre, but I think that's because it's

 - previous line,
 - current line,
 - next line,

So I think this is all fine:


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

> +               linePointers[1] = src;
> +               linePointers[2] = src + stride_;
> +               (this->*stats0_)(linePointers);
> +               src += stride_ * 2;
> +       }
> +}
> +
> +/**
> + * \brief Calculate statistics for a frame in one go
> + * \param[in] frame The frame number
> + * \param[in] bufferId ID of the statistics buffer
> + * \param[in] input The frame to process
> + *
> + * This may only be called after a successful setWindow() call.
> + */
> +void SwStatsCpu::processFrame(uint32_t frame, uint32_t bufferId, FrameBuffer *input)
> +{
> +       bench_.startFrame();
> +       startFrame(frame);
> +
> +       MappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);
> +       if (!in.isValid()) {
> +               LOG(SwStatsCpu, Error) << "mmap-ing buffer(s) failed";
> +               return;
> +       }
> +
> +       (this->*processFrame_)(in);
> +       finishFrame(frame, bufferId);
> +       bench_.finishFrame();
> +}
> +
>  } /* namespace libcamera */
> -- 
> 2.51.0
>

On 15/10/2025 21:14, Kieran Bingham wrote:
>> [bod: various rebase spalts fixed]
> s/spalts/splats/

I done it on purpose your honour.

Spalts is the apotheosis splats you know.

---
bod