| Message ID | 20220726143211.517231-3-fsylvestre@baylibre.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Florian, Thank you for the patch. On Tue, Jul 26, 2022 at 04:32:11PM +0200, Florian Sylvestre wrote: > Add ColorProcessing algorithm that is in charge to manage brightness, contrast > and saturation controls. > These controls are currently based on user controls. > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/cproc.cpp | 97 ++++++++++++++++++++++++ > src/ipa/rkisp1/algorithms/cproc.h | 30 ++++++++ > src/ipa/rkisp1/algorithms/meson.build | 1 + > src/ipa/rkisp1/data/ov5640.yaml | 1 + > src/ipa/rkisp1/ipa_context.cpp | 17 +++++ > src/ipa/rkisp1/ipa_context.h | 7 ++ > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++ > 7 files changed, 165 insertions(+) > create mode 100644 src/ipa/rkisp1/algorithms/cproc.cpp > create mode 100644 src/ipa/rkisp1/algorithms/cproc.h > > diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp > new file mode 100644 > index 00000000..fa989497 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/cproc.cpp > @@ -0,0 +1,97 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021-2022, Ideas On Board > + * > + * cproc.cpp - RkISP1 Color Processing control > + */ > + > +#include "cproc.h" > + > +#include <algorithm> > +#include <cmath> > + > +#include <libcamera/base/log.h> > + > +#include <libcamera/control_ids.h> > + > +/** > + * \file cproc.h > + */ > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +/** > + * \class ColorProcessing > + * \brief RkISP1 Color Processing control > + * > + * The ColorProcessing algorithm is responsible for applying brightness, > + * contrast and saturation corrections. The values are directly provided > + * through requests by the corresponding controls. > + */ > + > +LOG_DEFINE_CATEGORY(RkISP1CProc) > + > +/** > + * \copydoc libcamera::ipa::Algorithm::queueRequest > + */ > +void ColorProcessing::queueRequest([[maybe_unused]] IPAContext &context, context is used. > + [[maybe_unused]] const uint32_t frame, > + const ControlList &controls) > +{ > + auto &cproc = context.frameContext.cproc; > + > + const auto &brightness = controls.get(controls::Brightness); > + if (brightness) { > + cproc.brightness = std::round(std::clamp(*brightness * 128.0f, -128.0f, 127.0f)); Would it be a problem to round before clamping, in order to clamp on integers, which would be slightly more efficient ? I think you could use cproc.brightness = std::clamp(std::lround(*brightness * 128.0f), -128, 127); Same for the contrast and saturation. > + cproc.updateParams = true; > + > + LOG(RkISP1CProc, Debug) << "Set brightness to " << *brightness; > + } > + > + const auto &contrast = controls.get(controls::Contrast); > + if (contrast) { > + cproc.contrast = std::round(std::clamp(*contrast * 128.0f, 0.0f, 255.0f)); > + cproc.updateParams = true; > + > + LOG(RkISP1CProc, Debug) << "Set contrast to " << *contrast; > + } > + > + const auto saturation = controls.get(controls::Saturation); > + if (saturation) { > + cproc.saturation = std::round(std::clamp(*saturation * 128.0f, 0.0f, 255.0f)); > + cproc.updateParams = true; > + > + LOG(RkISP1CProc, Debug) << "Set saturation to " << *saturation; > + } > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::prepare > + */ > +void ColorProcessing::prepare([[maybe_unused]] IPAContext &context, context is used. > + rkisp1_params_cfg *params) > +{ > + auto &cproc = context.frameContext.cproc; > + > + /* Check if the algorithm configuration has been updated. */ > + if (!cproc.updateParams) > + return; > + > + cproc.updateParams = false; > + > + params->others.cproc_config.brightness = cproc.brightness; > + params->others.cproc_config.contrast = cproc.contrast; > + params->others.cproc_config.sat = cproc.saturation; > + > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_CPROC; > + params->module_ens |= RKISP1_CIF_ISP_MODULE_CPROC; > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CPROC; > +} > + > +REGISTER_IPA_ALGORITHM(ColorProcessing, "ColorProcessing") > + > +} /* namespace ipa::rkisp1::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h > new file mode 100644 > index 00000000..4b7e4064 > --- /dev/null > +++ b/src/ipa/rkisp1/algorithms/cproc.h > @@ -0,0 +1,30 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021-2022, Ideas On Board > + * > + * cproc.h - RkISP1 Color Processing control > + */ > + > +#pragma once > + > +#include <sys/types.h> > + > +#include "algorithm.h" > + > +namespace libcamera { > + > +namespace ipa::rkisp1::algorithms { > + > +class ColorProcessing : public Algorithm > +{ > +public: > + ColorProcessing() = default; > + ~ColorProcessing() = default; > + > + void queueRequest(IPAContext &context, const uint32_t frame, > + const ControlList &controls) override; > + void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > +}; > + > +} /* namespace ipa::rkisp1::algorithms */ > +} /* namespace libcamera */ > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build > index dcd24fe0..e48974b4 100644 > --- a/src/ipa/rkisp1/algorithms/meson.build > +++ b/src/ipa/rkisp1/algorithms/meson.build > @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([ > 'agc.cpp', > 'awb.cpp', > 'blc.cpp', > + 'cproc.cpp', > 'dpcc.cpp', > 'filter.cpp', > 'gsl.cpp', > diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml > index 99529481..93d7d1e7 100644 > --- a/src/ipa/rkisp1/data/ov5640.yaml > +++ b/src/ipa/rkisp1/data/ov5640.yaml > @@ -10,6 +10,7 @@ algorithms: > Gr: 256 > Gb: 256 > B: 256 > + - ColorProcessing: > - GammaSensorLinearization: > x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ] > y: > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp > index 4b117186..ef8bb8e9 100644 > --- a/src/ipa/rkisp1/ipa_context.cpp > +++ b/src/ipa/rkisp1/ipa_context.cpp > @@ -136,6 +136,23 @@ namespace libcamera::ipa::rkisp1 { > * \brief Estimated color temperature > */ > > +/** > + * \var IPAFrameContext::cproc > + * \brief Context for the Color Processing algorithm > + * > + * \struct IPAFrameContext::cproc.brightness > + * \brief Brightness level > + * > + * \var IPAFrameContext::cproc.contrast > + * \brief Contrast level > + * > + * \var IPAFrameContext::cproc.saturation > + * \brief Saturation level > + * > + * \var IPAFrameContext::cproc.updateParams > + * \brief Indicates if ISP parameters need to be updated > + */ > + > /** > * \var IPAFrameContext::filter > * \brief Context for the Filter algorithm > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 3b2f6af1..2bdb6a81 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -57,6 +57,13 @@ struct IPAFrameContext { > double temperatureK; > } awb; > > + struct { > + int8_t brightness; > + uint8_t contrast; > + uint8_t saturation; > + bool updateParams; > + } cproc; > + > struct { > uint8_t denoise; > uint8_t sharpness; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4e000d31..de687f4d 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -972,6 +972,18 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) > std::forward_as_tuple(&controls::Sharpness), > std::forward_as_tuple(0.0f, 10.0f, 1.0f)); > > + ctrls.emplace(std::piecewise_construct, > + std::forward_as_tuple(&controls::Brightness), > + std::forward_as_tuple(-1.0f, 0.993f)); > + > + ctrls.emplace(std::piecewise_construct, > + std::forward_as_tuple(&controls::Contrast), > + std::forward_as_tuple(0.0f, 1.993f)); > + > + ctrls.emplace(std::piecewise_construct, > + std::forward_as_tuple(&controls::Saturation), > + std::forward_as_tuple(0.0f, 1.993f)); > + > ctrls.emplace(std::piecewise_construct, > std::forward_as_tuple(&controls::draft::NoiseReductionMode), > std::forward_as_tuple(controls::draft::NoiseReductionModeValues));
diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp new file mode 100644 index 00000000..fa989497 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -0,0 +1,97 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021-2022, Ideas On Board + * + * cproc.cpp - RkISP1 Color Processing control + */ + +#include "cproc.h" + +#include <algorithm> +#include <cmath> + +#include <libcamera/base/log.h> + +#include <libcamera/control_ids.h> + +/** + * \file cproc.h + */ + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +/** + * \class ColorProcessing + * \brief RkISP1 Color Processing control + * + * The ColorProcessing algorithm is responsible for applying brightness, + * contrast and saturation corrections. The values are directly provided + * through requests by the corresponding controls. + */ + +LOG_DEFINE_CATEGORY(RkISP1CProc) + +/** + * \copydoc libcamera::ipa::Algorithm::queueRequest + */ +void ColorProcessing::queueRequest([[maybe_unused]] IPAContext &context, + [[maybe_unused]] const uint32_t frame, + const ControlList &controls) +{ + auto &cproc = context.frameContext.cproc; + + const auto &brightness = controls.get(controls::Brightness); + if (brightness) { + cproc.brightness = std::round(std::clamp(*brightness * 128.0f, -128.0f, 127.0f)); + cproc.updateParams = true; + + LOG(RkISP1CProc, Debug) << "Set brightness to " << *brightness; + } + + const auto &contrast = controls.get(controls::Contrast); + if (contrast) { + cproc.contrast = std::round(std::clamp(*contrast * 128.0f, 0.0f, 255.0f)); + cproc.updateParams = true; + + LOG(RkISP1CProc, Debug) << "Set contrast to " << *contrast; + } + + const auto saturation = controls.get(controls::Saturation); + if (saturation) { + cproc.saturation = std::round(std::clamp(*saturation * 128.0f, 0.0f, 255.0f)); + cproc.updateParams = true; + + LOG(RkISP1CProc, Debug) << "Set saturation to " << *saturation; + } +} + +/** + * \copydoc libcamera::ipa::Algorithm::prepare + */ +void ColorProcessing::prepare([[maybe_unused]] IPAContext &context, + rkisp1_params_cfg *params) +{ + auto &cproc = context.frameContext.cproc; + + /* Check if the algorithm configuration has been updated. */ + if (!cproc.updateParams) + return; + + cproc.updateParams = false; + + params->others.cproc_config.brightness = cproc.brightness; + params->others.cproc_config.contrast = cproc.contrast; + params->others.cproc_config.sat = cproc.saturation; + + params->module_en_update |= RKISP1_CIF_ISP_MODULE_CPROC; + params->module_ens |= RKISP1_CIF_ISP_MODULE_CPROC; + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CPROC; +} + +REGISTER_IPA_ALGORITHM(ColorProcessing, "ColorProcessing") + +} /* namespace ipa::rkisp1::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h new file mode 100644 index 00000000..4b7e4064 --- /dev/null +++ b/src/ipa/rkisp1/algorithms/cproc.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021-2022, Ideas On Board + * + * cproc.h - RkISP1 Color Processing control + */ + +#pragma once + +#include <sys/types.h> + +#include "algorithm.h" + +namespace libcamera { + +namespace ipa::rkisp1::algorithms { + +class ColorProcessing : public Algorithm +{ +public: + ColorProcessing() = default; + ~ColorProcessing() = default; + + void queueRequest(IPAContext &context, const uint32_t frame, + const ControlList &controls) override; + void prepare(IPAContext &context, rkisp1_params_cfg *params) override; +}; + +} /* namespace ipa::rkisp1::algorithms */ +} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build index dcd24fe0..e48974b4 100644 --- a/src/ipa/rkisp1/algorithms/meson.build +++ b/src/ipa/rkisp1/algorithms/meson.build @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([ 'agc.cpp', 'awb.cpp', 'blc.cpp', + 'cproc.cpp', 'dpcc.cpp', 'filter.cpp', 'gsl.cpp', diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml index 99529481..93d7d1e7 100644 --- a/src/ipa/rkisp1/data/ov5640.yaml +++ b/src/ipa/rkisp1/data/ov5640.yaml @@ -10,6 +10,7 @@ algorithms: Gr: 256 Gb: 256 B: 256 + - ColorProcessing: - GammaSensorLinearization: x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ] y: diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 4b117186..ef8bb8e9 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -136,6 +136,23 @@ namespace libcamera::ipa::rkisp1 { * \brief Estimated color temperature */ +/** + * \var IPAFrameContext::cproc + * \brief Context for the Color Processing algorithm + * + * \struct IPAFrameContext::cproc.brightness + * \brief Brightness level + * + * \var IPAFrameContext::cproc.contrast + * \brief Contrast level + * + * \var IPAFrameContext::cproc.saturation + * \brief Saturation level + * + * \var IPAFrameContext::cproc.updateParams + * \brief Indicates if ISP parameters need to be updated + */ + /** * \var IPAFrameContext::filter * \brief Context for the Filter algorithm diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 3b2f6af1..2bdb6a81 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -57,6 +57,13 @@ struct IPAFrameContext { double temperatureK; } awb; + struct { + int8_t brightness; + uint8_t contrast; + uint8_t saturation; + bool updateParams; + } cproc; + struct { uint8_t denoise; uint8_t sharpness; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4e000d31..de687f4d 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -972,6 +972,18 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) std::forward_as_tuple(&controls::Sharpness), std::forward_as_tuple(0.0f, 10.0f, 1.0f)); + ctrls.emplace(std::piecewise_construct, + std::forward_as_tuple(&controls::Brightness), + std::forward_as_tuple(-1.0f, 0.993f)); + + ctrls.emplace(std::piecewise_construct, + std::forward_as_tuple(&controls::Contrast), + std::forward_as_tuple(0.0f, 1.993f)); + + ctrls.emplace(std::piecewise_construct, + std::forward_as_tuple(&controls::Saturation), + std::forward_as_tuple(0.0f, 1.993f)); + ctrls.emplace(std::piecewise_construct, std::forward_as_tuple(&controls::draft::NoiseReductionMode), std::forward_as_tuple(controls::draft::NoiseReductionModeValues));