[libcamera-devel] [PATCH v2 3/4] ipa: rkisp1: Add support of Demosaicing control

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Jul 21 15:08:39 CEST 2022


Hi Florian,

Quoting Florian Sylvestre via libcamera-devel (2022-07-20 16:42:20)
> During the demosaicing step, rkisp1 ISP is processing denoising and sharpness
> control.
> Add demosaicing algorithm with denoise and sharpness values based on user
> controls.
> 

This one however, is more directly related to where the data is stored.

In my series I've moved the existing 'frame context' to the 'active
state' ... which is an area for algorithms to store data that may be
accessed from other algorithms. This data may not fit there now.

Would you be able to look at how this would operate on top of a 'per
frame, frame context' and see if it still functions for you ?

I've pushed my branch to :


 Repo	github.com:kbingham/libcamera.git
 Branch:	kbingham/pfc

to make it easier for you to access.

--
Kieran




> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
>  src/ipa/rkisp1/algorithms/demosaicing.cpp | 197 ++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/demosaicing.h   |  30 ++++
>  src/ipa/rkisp1/algorithms/meson.build     |   1 +
>  src/ipa/rkisp1/data/ov5640.yaml           |   1 +
>  src/ipa/rkisp1/ipa_context.h              |   6 +
>  src/ipa/rkisp1/rkisp1.cpp                 |   1 +
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp  |   8 +
>  7 files changed, 244 insertions(+)
>  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/demosaicing.cpp b/src/ipa/rkisp1/algorithms/demosaicing.cpp
> new file mode 100644
> index 00000000..7d55eaab
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/demosaicing.cpp
> @@ -0,0 +1,197 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * demosaicing.cpp - RkISP1 Demosaicing control
> + */
> +
> +#include "demosaicing.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +/**
> + * \file demosaicing.h
> + */
> +
> +static constexpr uint32_t kFiltLumWeightDefault = 0x00022040;
> +static constexpr uint32_t kFiltModeDefault = 0x000004f2;
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Demosaicing
> + * \brief RkISP1 Demosaicing control
> + *
> + * The Demosaicing algorithm is responsible for reconstructing a full color
> + * image from the sensor raw data. During the demosaicing step, The RKISP1
> + * will additionally process denoise and sharpness controls.
> + *
> + * /todo In current version the denoise and sharpness control is based on user
> + * controls. In a future version it should be controlled automatically by the
> + * algorithm.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Demosaicing)
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::queueRequest
> + */
> +void Demosaicing::queueRequest([[maybe_unused]] IPAContext &context,
> +                              [[maybe_unused]] const uint32_t frame,
> +                              const ControlList &controls)
> +{

This is the part where we'll have to explore the split between
activeState and the frameContext.

I would envisage storing the 'current active mode' in the Active state,
but the paramters which get applied to the ISP in the FrameContext.

That said, the FrameContext may also need to know the mode at the time
of that frame context. I am envisioning adding a MetaData control list
to the FrameContext that could allow algorithms to update metadata
explicitly. (As opposed to just duplicating the incoming ControlList).


> +       const auto &sharpness = controls.get(controls::Sharpness);
> +       if (sharpness) {
> +               context.frameContext.demosaicing.sharpness = std::clamp(int(*sharpness), 0, 10);
> +               context.frameContext.demosaicing.updateParams = true;
> +
> +               LOG(RkISP1Demosaicing, Debug) << "Set sharpness to " << *sharpness;
> +       }
> +
> +       const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> +       if (denoise) {
> +               LOG(RkISP1Demosaicing, Debug) << "Set denoise to " << *denoise;
> +
> +               switch (*denoise) {
> +               case controls::draft::NoiseReductionModeOff:
> +                       context.frameContext.demosaicing.denoise = 0;
> +                       context.frameContext.demosaicing.updateParams = true;
> +                       break;
> +               case controls::draft::NoiseReductionModeMinimal:
> +                       context.frameContext.demosaicing.denoise = 1;
> +                       context.frameContext.demosaicing.updateParams = true;
> +                       break;
> +               case controls::draft::NoiseReductionModeHighQuality:
> +               case controls::draft::NoiseReductionModeFast:
> +                       context.frameContext.demosaicing.denoise = 3;
> +                       context.frameContext.demosaicing.updateParams = true;
> +                       break;
> +               default:
> +                       LOG(RkISP1Demosaicing, Error)
> +                               << "Unsupported denoise value "
> +                               << *denoise;
> +               }
> +       }
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Demosaicing::prepare([[maybe_unused]] IPAContext &context,
> +                         rkisp1_params_cfg *params)
> +{
> +       /* Check if the algorithm configuration has been updated. */
> +       if (!context.frameContext.demosaicing.updateParams)
> +               return;
> +
> +       context.frameContext.demosaicing.updateParams = false;
> +
> +       static constexpr uint16_t filt_fac_sh0[] = {
> +               0x04, 0x07, 0x0A, 0x0C, 0x10, 0x14, 0x1A, 0x1E, 0x24, 0x2A, 0x30
> +       };
> +
> +       static constexpr uint16_t filt_fac_sh1[] = {
> +               0x04, 0x08, 0x0C, 0x10, 0x16, 0x1B, 0x20, 0x26, 0x2C, 0x30, 0x3F
> +       };
> +
> +       static constexpr uint16_t filt_fac_mid[] = {
> +               0x04, 0x06, 0x08, 0x0A, 0x0C, 0x10, 0x13, 0x17, 0x1D, 0x22, 0x28
> +       };
> +
> +       static constexpr uint16_t filt_fac_bl0[] = {
> +               0x02, 0x02, 0x04, 0x06, 0x08, 0x0A, 0x0C, 0x10, 0x15, 0x1A, 0x24
> +       };
> +
> +       static constexpr uint16_t filt_fac_bl1[] = {
> +               0x00, 0x00, 0x00, 0x02, 0x04, 0x04, 0x06, 0x08, 0x0D, 0x14, 0x20
> +       };
> +
> +       static constexpr uint16_t filt_thresh_sh0[] = {
> +               0, 18, 26, 36, 41, 75, 90, 120, 170, 250, 1023
> +       };
> +
> +       static constexpr uint16_t filt_thresh_sh1[] = {
> +               0, 33, 44, 51, 67, 100, 120, 150, 200, 300, 1023
> +       };
> +
> +       static constexpr uint16_t filt_thresh_bl0[] = {
> +               0, 8, 13, 23, 26, 50, 60, 80, 140, 180, 1023
> +       };
> +
> +       static constexpr uint16_t filt_thresh_bl1[] = {
> +               0, 2, 5, 10, 15, 20, 26, 51, 100, 150, 1023
> +       };
> +
> +       static constexpr uint16_t stage1_select[] = {
> +               6, 6, 4, 4, 3, 3, 2, 2, 2, 2, 2
> +       };
> +
> +       static constexpr uint16_t filt_chr_v_mode[] = {
> +               1, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3
> +       };
> +
> +       static constexpr uint16_t filt_chr_h_mode[] = {
> +               0, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3
> +       };
> +
> +       uint8_t denoise = context.frameContext.demosaicing.denoise;
> +       uint8_t sharpness = context.frameContext.demosaicing.sharpness;
> +

Ok - I now see sharpness is clamped to acceptable ranges, and denoise is
only ever set explicitly.

> +       params->others.flt_config.fac_sh0 = filt_fac_sh0[sharpness];
> +       params->others.flt_config.fac_sh1 = filt_fac_sh1[sharpness];
> +       params->others.flt_config.fac_mid = filt_fac_mid[sharpness];
> +       params->others.flt_config.fac_bl0 = filt_fac_bl0[sharpness];
> +       params->others.flt_config.fac_bl1 = filt_fac_bl1[sharpness];
> +
> +       params->others.flt_config.lum_weight = kFiltLumWeightDefault;
> +       params->others.flt_config.mode = kFiltModeDefault;
> +       params->others.flt_config.thresh_sh0 = filt_thresh_sh0[denoise];
> +       params->others.flt_config.thresh_sh1 = filt_thresh_sh1[denoise];
> +       params->others.flt_config.thresh_bl0 = filt_thresh_bl0[denoise];
> +       params->others.flt_config.thresh_bl1 = filt_thresh_bl1[denoise];
> +       params->others.flt_config.grn_stage1 = stage1_select[denoise];
> +       params->others.flt_config.chr_v_mode = filt_chr_v_mode[denoise];
> +       params->others.flt_config.chr_h_mode = filt_chr_h_mode[denoise];
> +
> +       if (denoise == 9) {

Can this code actually be reached? Is denoise ever set to anything other
than 0, 1 or 3

> +               if (sharpness > 3)
> +                       params->others.flt_config.grn_stage1 = 2;
> +               else
> +                       params->others.flt_config.grn_stage1 = 1;
> +       } else if (denoise == 10) {
> +               if (sharpness > 5)
> +                       params->others.flt_config.grn_stage1 = 2;
> +               else if (sharpness > 3)
> +                       params->others.flt_config.grn_stage1 = 1;
> +               else
> +                       params->others.flt_config.grn_stage1 = 0;
> +       } else if (denoise > 7) {
> +               if (sharpness > 7) {
> +                       params->others.flt_config.fac_bl0 =
> +                               params->others.flt_config.fac_bl0 / 2;
> +                       params->others.flt_config.fac_bl1 =
> +                               params->others.flt_config.fac_bl1 / 4;
> +               } else if (sharpness > 4) {
> +                       params->others.flt_config.fac_bl0 =
> +                               params->others.flt_config.fac_bl0 * 3 / 4;
> +                       params->others.flt_config.fac_bl1 =
> +                               params->others.flt_config.fac_bl1 / 2;
> +               }
> +       }
> +
> +       params->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT;
> +       params->module_ens |= RKISP1_CIF_ISP_MODULE_FLT;
> +       params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT;
> +}
> +
> +REGISTER_IPA_ALGORITHM(Demosaicing, "Demosaicing")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/demosaicing.h b/src/ipa/rkisp1/algorithms/demosaicing.h
> new file mode 100644
> index 00000000..0d0f778f
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/demosaicing.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * demosaicing.h - RkISP1 Demosaicing control
> + */
> +
> +#pragma once
> +
> +#include <sys/types.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Demosaicing : public Algorithm
> +{
> +public:
> +       Demosaicing() = default;
> +       ~Demosaicing() = 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 87007493..7e078b0d 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',
> +    'demosaicing.cpp',
>      'dpcc.cpp',
>      'gsl.cpp',
>      'lsc.cpp',
> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> index 51228218..4ae0ffc0 100644
> --- a/src/ipa/rkisp1/data/ov5640.yaml
> +++ b/src/ipa/rkisp1/data/ov5640.yaml
> @@ -157,4 +157,5 @@ algorithms:
>            rnd-offsets:
>              green: 2
>              red-blue: 2
> +  - Demosaicing:
>  ...
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index f387cace..241a4d04 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -56,6 +56,12 @@ struct IPAFrameContext {
>                 double temperatureK;
>         } awb;
>  
> +       struct {
> +               uint8_t denoise;
> +               uint8_t sharpness;
> +               bool updateParams;
> +       } demosaicing;
> +
>         struct {
>                 uint32_t exposure;
>                 double gain;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 34034526..aeec8f50 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -31,6 +31,7 @@
>  #include "algorithms/algorithm.h"
>  #include "algorithms/awb.h"
>  #include "algorithms/blc.h"
> +#include "algorithms/demosaicing.h"
>  #include "algorithms/dpcc.h"
>  #include "algorithms/gsl.h"
>  #include "algorithms/lsc.h"
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 99eecd44..4e000d31 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -968,6 +968,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>                                                    hasSelfPath_ ? &selfPath_ : nullptr);
>  
>         ControlInfoMap::Map ctrls;
> +       ctrls.emplace(std::piecewise_construct,
> +                     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::draft::NoiseReductionMode),
> +                     std::forward_as_tuple(controls::draft::NoiseReductionModeValues));
> +
>         ctrls.emplace(std::piecewise_construct,
>                       std::forward_as_tuple(&controls::AeEnable),
>                       std::forward_as_tuple(false, true));
> -- 
> 2.34.1
>


More information about the libcamera-devel mailing list