| Message ID | 20250624171223.2181226-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Tue, Jun 24, 2025 at 06:12:21PM +0100, Kieran Bingham wrote: > From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com> > > Presently the colour processing component exposes controls for > brightness, saturation, and contrast to the applications which are > handled and processed accordingly on the ISP. > > The implementation lacks reporting the values that are set back to the > application. > > Provide the inverse conversion functions to convert the values applied > and report them in the completed request metadata. > > Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com> > Signed-off-by: Zenker, Sebastian <sebastian.zenker@karlstorz.com> > [Kieran: Refactor commit message, fix checkstyle warnings] > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/cproc.cpp | 22 ++++++++++++++++++++++ > src/ipa/rkisp1/algorithms/cproc.h | 4 ++++ > 2 files changed, 26 insertions(+) > > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp > index d1fff6990d37..9213ee49b374 100644 > --- a/src/ipa/rkisp1/algorithms/cproc.cpp > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp > @@ -44,11 +44,21 @@ int convertBrightness(const float v) > return std::clamp<int>(std::lround(v * 128), -128, 127); > } > > +float convertBrightness(const int v) > +{ > + return static_cast<float>(v) / 128.0f; > +} > + > int convertContrastOrSaturation(const float v) > { > return std::clamp<int>(std::lround(v * 128), 0, 255); > } > > +float convertContrastOrSaturation(const int v) > +{ > + return static_cast<float>(v) / 128.0f; > +} > + > } /* namespace */ > > /** > @@ -153,6 +163,18 @@ void ColorProcessing::prepare([[maybe_unused]] IPAContext &context, > config->sat = frameContext.cproc.saturation; > } > > +/** > + * \copydoc libcamera::ipa::Algorithm::process > + */ > +void ColorProcessing::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const uint32_t frame, > + IPAFrameContext &frameContext, [[maybe_unused]] const rkisp1_stat_buffer *stats, > + ControlList &metadata) > +{ > + metadata.set(controls::Brightness, convertBrightness(frameContext.cproc.brightness)); > + metadata.set(controls::Contrast, convertContrastOrSaturation(frameContext.cproc.contrast)); > + metadata.set(controls::Saturation, convertContrastOrSaturation(frameContext.cproc.saturation)); Instead of converting this for every frame, could we store the floating point value in the frame context, and only convert them to hardware configuration when .update is set ? > +} > + > REGISTER_IPA_ALGORITHM(ColorProcessing, "ColorProcessing") > > } /* namespace ipa::rkisp1::algorithms */ > diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h > index fd38fd17e8bb..9b589ebd4ad7 100644 > --- a/src/ipa/rkisp1/algorithms/cproc.h > +++ b/src/ipa/rkisp1/algorithms/cproc.h > @@ -30,6 +30,10 @@ public: > void prepare(IPAContext &context, const uint32_t frame, > IPAFrameContext &frameContext, > RkISP1Params *params) override; > + void process(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + const rkisp1_stat_buffer *stats, > + ControlList &metadata) override; > }; > > } /* namespace ipa::rkisp1::algorithms */
diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp index d1fff6990d37..9213ee49b374 100644 --- a/src/ipa/rkisp1/algorithms/cproc.cpp +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -44,11 +44,21 @@ int convertBrightness(const float v) return std::clamp<int>(std::lround(v * 128), -128, 127); } +float convertBrightness(const int v) +{ + return static_cast<float>(v) / 128.0f; +} + int convertContrastOrSaturation(const float v) { return std::clamp<int>(std::lround(v * 128), 0, 255); } +float convertContrastOrSaturation(const int v) +{ + return static_cast<float>(v) / 128.0f; +} + } /* namespace */ /** @@ -153,6 +163,18 @@ void ColorProcessing::prepare([[maybe_unused]] IPAContext &context, config->sat = frameContext.cproc.saturation; } +/** + * \copydoc libcamera::ipa::Algorithm::process + */ +void ColorProcessing::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const uint32_t frame, + IPAFrameContext &frameContext, [[maybe_unused]] const rkisp1_stat_buffer *stats, + ControlList &metadata) +{ + metadata.set(controls::Brightness, convertBrightness(frameContext.cproc.brightness)); + metadata.set(controls::Contrast, convertContrastOrSaturation(frameContext.cproc.contrast)); + metadata.set(controls::Saturation, convertContrastOrSaturation(frameContext.cproc.saturation)); +} + REGISTER_IPA_ALGORITHM(ColorProcessing, "ColorProcessing") } /* namespace ipa::rkisp1::algorithms */ diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h index fd38fd17e8bb..9b589ebd4ad7 100644 --- a/src/ipa/rkisp1/algorithms/cproc.h +++ b/src/ipa/rkisp1/algorithms/cproc.h @@ -30,6 +30,10 @@ public: void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, RkISP1Params *params) override; + void process(IPAContext &context, const uint32_t frame, + IPAFrameContext &frameContext, + const rkisp1_stat_buffer *stats, + ControlList &metadata) override; }; } /* namespace ipa::rkisp1::algorithms */