[libcamera-devel] [PATCH v3 03/13] ipa: rkisp1: Support raw capture in IPA operations
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 24 09:47:15 CEST 2022
Hi Paul,
On Mon, Oct 24, 2022 at 11:02:20AM +0900, paul.elder at ideasonboard.com wrote:
> On Mon, Oct 24, 2022 at 03:03:46AM +0300, Laurent Pinchart wrote:
> > The RkISP1 can capture raw frames by bypassing the ISP. In that mode,
> > the ISP will not produce statistics nor consume parameters. Most
> > algorithms will thus be unable to run, with one exception: the AGC will
> > still be able to configure the sensor exposure time and analog gain in
> > manual mode.
> >
> > To prepare for this, add the ability to disable algorithms for the
> > duration of the capture session based on the camera configuration.
> > Individual algorithms report whether they support raw formats at
> > construction time, and the IPA module disables algorithms in configure()
> > based on the stream configurations.
> >
> > Disabled algorithms are skipped during the capture session in the
> > processStatsBuffer() operation. As the ISP doesn't produce statistics,
> > don't try to access the stats buffer. There is no need for similar logic
> > in fillParamsBuffer() as that operation won't be called for raw capture.
> >
> > All algorithms report not supporting raw capture by default. Raw support
> > in AGC will be added separately.
> >
> > The feature is implemented in the RkISP1 module without any support from
> > libipa at this point to avoid designing a generic API based on a single
> > user. This may be changed in the future.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/ipa/rkisp1/algorithms/algorithm.h | 12 +++++++-
> > src/ipa/rkisp1/ipa_context.cpp | 5 ++++
> > src/ipa/rkisp1/ipa_context.h | 2 ++
> > src/ipa/rkisp1/rkisp1.cpp | 42 +++++++++++++++++++++++----
> > 4 files changed, 54 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> > index c3212cff76fe..9454c9a1fc06 100644
> > --- a/src/ipa/rkisp1/algorithms/algorithm.h
> > +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> > @@ -15,7 +15,17 @@ namespace libcamera {
> >
> > namespace ipa::rkisp1 {
> >
> > -using Algorithm = libcamera::ipa::Algorithm<Module>;
> > +class Algorithm : public libcamera::ipa::Algorithm<Module>
> > +{
> > +public:
> > + Algorithm()
> > + : disabled_(false), supportsRaw_(false)
> > + {
> > + }
> > +
> > + bool disabled_;
> > + bool supportsRaw_;
> > +};
> >
> > } /* namespace ipa::rkisp1 */
> >
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 7a987497bd03..9bbf368432fa 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -89,6 +89,11 @@ namespace libcamera::ipa::rkisp1 {
> > * \brief Sensor output resolution
> > */
> >
> > +/**
> > + * \var IPASessionConfiguration::raw
> > + * \brief Indicates if the camera is configured to capture raw frames
> > + */
> > +
> > /**
> > * \struct IPAActiveState
> > * \brief Active state for algorithms
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index bb60ab9eab72..3e47ac663c58 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -48,6 +48,8 @@ struct IPASessionConfiguration {
> > struct {
> > rkisp1_cif_isp_version revision;
> > } hw;
> > +
> > + bool raw;
> > };
> >
> > struct IPAActiveState {
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index fcb9dacccc3c..538e42cb6f7f 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -24,6 +24,7 @@
> > #include <libcamera/ipa/rkisp1_ipa_interface.h>
> > #include <libcamera/request.h>
> >
> > +#include "libcamera/internal/formats.h"
> > #include "libcamera/internal/mapped_framebuffer.h"
> > #include "libcamera/internal/yaml_parser.h"
> >
> > @@ -207,7 +208,7 @@ void IPARkISP1::stop()
> > * before accessing them.
> > */
> > int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
> > - [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,
> > + const std::map<uint32_t, IPAStream> &streamConfig,
> > const std::map<uint32_t, ControlInfoMap> &entityControls)
> > {
> > if (entityControls.empty())
> > @@ -264,7 +265,21 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
> > context_.configuration.sensor.minAnalogueGain = camHelper_->gain(minGain);
> > context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);
> >
> > - for (auto const &algo : algorithms()) {
> > + context_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),
> > + [](auto &cfg) -> bool {
> > + PixelFormat pixelFormat{ cfg.second.pixelFormat };
> > + const PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);
> > + return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> > + });
> > +
> > + for (auto const &a : algorithms()) {
> > + Algorithm *algo = static_cast<Algorithm *>(a.get());
> > +
> > + /* Disable algorithms that don't support raw formats. */
> > + algo->disabled_ = context_.configuration.raw && !algo->supportsRaw_;
> > + if (algo->disabled_)
> > + continue;
> > +
>
> On one hand I was thinking if the algorithms should internally nop if
> they're in raw mode, but I think that indeed it's the job of the IPA to
> control its algorithms based on its own config, so this is fine.
>
> > int ret = algo->configure(context_, info);
> > if (ret)
> > return ret;
> > @@ -307,8 +322,12 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
> > {
> > IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
> >
> > - for (auto const &algo : algorithms())
> > + for (auto const &a : algorithms()) {
> > + Algorithm *algo = static_cast<Algorithm *>(a.get());
> > + if (algo->disabled_)
> > + continue;
> > algo->queueRequest(context_, frame, frameContext, controls);
> > + }
> > }
> >
> > void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
> > @@ -333,9 +352,16 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
> > {
> > IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> >
> > - const rkisp1_stat_buffer *stats =
> > - reinterpret_cast<rkisp1_stat_buffer *>(
> > + /*
> > + * In raw capture mode, the ISP is bypassed and no statistics buffer is
> > + * provided.
> > + */
> > + const rkisp1_stat_buffer *stats;
> > + if (!context_.configuration.raw)
> > + stats = reinterpret_cast<rkisp1_stat_buffer *>(
> > mappedBuffers_.at(bufferId).planes()[0].data());
> > + else
> > + stats = nullptr;
>
> processStatsBuffer() won't ever be called in raw mode *except* for
> (before the pipeline hander is adjusted (I expect it to be later in the
> series)) the first buffer that got queued at start time, and which would
> be bufferReady-dequeued at streamoff. At least this protects against
> dereferencing the null stats buffer, plus we don't report supporting raw
> formats yet anyway, so I guess it's fine.
The function will actually be called to fill metadata for the request.
It will not receive statistics, but it will receive the effective values
of the camera sensor controls for that frame.
> Just thinking aloud :)
>
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> >
> > frameContext.sensor.exposure =
> > sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > @@ -344,8 +370,12 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
> >
> > ControlList metadata(controls::controls);
> >
> > - for (auto const &algo : algorithms())
> > + for (auto const &a : algorithms()) {
> > + Algorithm *algo = static_cast<Algorithm *>(a.get());
> > + if (algo->disabled_)
> > + continue;
> > algo->process(context_, frame, frameContext, stats, metadata);
> > + }
> >
> > setControls(frame);
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list