| Message ID | 20251125000848.4103786-4-rui.wang@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Rui On Mon, Nov 24, 2025 at 07:08:40PM -0500, Rui Wang wrote: > Reserve the next free vendor allocation for rkisp1 and add the dedicated > control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the > generated control headers expose the rkisp1 control namespace. > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > --- > include/libcamera/meson.build | 1 + > src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ > src/libcamera/control_ranges.yaml | 4 +- > 3 files changed, 77 insertions(+), 1 deletion(-) > create mode 100644 src/libcamera/control_ids_rkisp1.yaml > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 30ea76f9..3fd7ef94 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -39,6 +39,7 @@ controls_map = { > 'draft': 'control_ids_draft.yaml', > 'rpi/pisp': 'control_ids_rpi.yaml', > 'rpi/vc4': 'control_ids_rpi.yaml', > + 'rkisp1': 'control_ids_rkisp1.yaml', > }, > > 'properties': { > diff --git a/src/libcamera/control_ids_rkisp1.yaml b/src/libcamera/control_ids_rkisp1.yaml > new file mode 100644 > index 00000000..3e008ee4 > --- /dev/null > +++ b/src/libcamera/control_ids_rkisp1.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: LGPL-2.1-or-later > +# > +# Copyright (C) 2025, Ideas On Board > +# > +# RkISP1 specific control definitions Why are these controls RkISP1 specific ? Ideally we want all controls to be supported by all platforms, but even if a single platform supports them, at least their definition should be as generic as possible. Move these to control_ids_core please Or better, we have draft::NoiseReductionMode from Android. Time to make it a core control as well ? > +--- > +# Unless otherwise stated, all controls are bi-directional, i.e. they can be > +# set through Request::controls() and returned out through Request::metadata(). > +vendor: rkisp1 > +controls: > + # --- RkISP1 DPF controls --- > + - DenoiseMode: > + type: int32_t > + direction: inout > + description: | > + Controls the operating mode of the rkisp1 denoise pipeline. This setting is over 80 cols. Please reflow > + consumed by the DPF module and any additional denoise algorithms that honour > + the common mode state exposed here. The assumption is that all algorithms implement support for the core controls. You can remove the last sentence. > + > + In Disabled mode (default) all denoise processing stages are disabled. This > + bypasses noise reduction and surfaces raw sensor data. Other denoise-related > + controls are ignored in this mode. > + > + In Auto mode each denoise algorithm selects parameters automatically from the > + sensor tuning data and (if provided) the exposure index level table. Changes in > + scene brightness (analogue gain / exposure index) may cause algorithms to > + re-evaluate and reprogram hardware when an exposure index band boundary is > + crossed. > + > + In Manual mode the automatically selected parameters are frozen at the moment > + the mode switch occurs (a snapshot is taken). Subsequent per-frame automatic > + updates are disabled until the mode is set back to Auto. While in Manual mode > + the application may modify any algorithm-specific override controls (for DPF: > + DpfChannelStrengths, DpfGreenSpatialCoefficients, DpfRedBlueSpatialCoefficients, > + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, DpfNoiseLevelLookupScaleMode). Where are these controls defined ? I would start simple and add Manual later on. > + Individual denoise modules apply changes immediately to hardware. > + > + In Reduction mode the algorithms use predefined noise reduction settings based > + on the active NoiseReductionMode control. This provides optimized presets for > + different noise reduction levels (e.g., minimal, high-quality, fast) without > + manual parameter tuning. Ah! Question is, you have an "auto" mode here, while NoiseReductionMode specifies profiles (Off, Fast, HighQuality, Minimal, ZSL). Those profiles comes straight from the Android's definition and raspberry pi is the only IPA that supports them. Can we express "auto" (and "manual") as one of the "profiles" already existing in NoiseReductionMode ? Or maybe we'll need an additional control to specify "auto, off, manual, profile" like you did. But it really should be a core control not a platform specific one. Something like: NoiseReductionMode: enum: - Off - Auto: - Manual - Profiles NoiseReductionProfiles: enum: - Fast - HighQuality - Minimal - ZSL Or maybe: NoiseReductionMode: enum: - Off - Auto - Manual - Fast - HighQuality - Minimal - ZSL What do you think ? I've cc-ed Naush and David to know what they think and have a bit of an insight on how they handle those profiles. > + > + Transition Rules: > + * Auto -> Manual: The current effective (auto-selected) parameters > + are captured as the initial manual override values. > + * Manual -> Auto: All manual override state is discarded and the > + algorithm immediately reverts to automatic tuning selection, > + potentially reprogramming hardware that same frame. > + * Auto/Manual -> Reduction: Switches to using reduction mode configs > + based on the current NoiseReductionMode setting. > + * Reduction -> Auto/Manual: Reverts to the specified mode, discarding > + reduction-specific state. > + > + If the application switches to Manual but supplies no overrides, > + the previously auto-derived parameters continue to be used unchanged. > + enum: > + - name: DenoiseModeDisabled > + value: 0 > + description: | > + Disabled mode - DPF processing is completely disabled. > + - name: DenoiseModeAuto > + value: 1 > + description: | > + Automatic mode - algorithm selects parameters based on tuning data. > + - name: DenoiseModeManual > + value: 2 > + description: | > + Manual mode - parameters are frozen and can be overridden. > + - name: DenoiseModeReduction > + value: 3 > + description: | > + Reduction mode - uses predefined noise reduction settings from tuning data. > +... > diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml > index 6752eb98..fc60de80 100644 > --- a/src/libcamera/control_ranges.yaml > +++ b/src/libcamera/control_ranges.yaml > @@ -15,6 +15,8 @@ ranges: > rpi: 20000 > # Controls for debug metadata > debug: 30000 > - # Next range starts at 40000 > + # RkISP1 vendor controls > + rkisp1: 40000 > + # Next range starts at 50000 > > ... > -- > 2.43.0 >
Hi. On Fri, 28 Nov 2025 at 16:04, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Rui > > On Mon, Nov 24, 2025 at 07:08:40PM -0500, Rui Wang wrote: > > Reserve the next free vendor allocation for rkisp1 and add the dedicated > > control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the > > generated control headers expose the rkisp1 control namespace. > > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > > --- > > include/libcamera/meson.build | 1 + > > src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ > > src/libcamera/control_ranges.yaml | 4 +- > > 3 files changed, 77 insertions(+), 1 deletion(-) > > create mode 100644 src/libcamera/control_ids_rkisp1.yaml > > > > diff --git a/include/libcamera/meson.build > b/include/libcamera/meson.build > > index 30ea76f9..3fd7ef94 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -39,6 +39,7 @@ controls_map = { > > 'draft': 'control_ids_draft.yaml', > > 'rpi/pisp': 'control_ids_rpi.yaml', > > 'rpi/vc4': 'control_ids_rpi.yaml', > > + 'rkisp1': 'control_ids_rkisp1.yaml', > > }, > > > > 'properties': { > > diff --git a/src/libcamera/control_ids_rkisp1.yaml > b/src/libcamera/control_ids_rkisp1.yaml > > new file mode 100644 > > index 00000000..3e008ee4 > > --- /dev/null > > +++ b/src/libcamera/control_ids_rkisp1.yaml > > @@ -0,0 +1,73 @@ > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > +# > > +# Copyright (C) 2025, Ideas On Board > > +# > > +# RkISP1 specific control definitions > > Why are these controls RkISP1 specific ? > > Ideally we want all controls to be supported by all platforms, but > even if a single platform supports them, at least their definition > should be as generic as possible. > > Move these to control_ids_core please > > Or better, we have draft::NoiseReductionMode from Android. > Time to make it a core control as well ? > > > +--- > > +# Unless otherwise stated, all controls are bi-directional, i.e. they > can be > > +# set through Request::controls() and returned out through > Request::metadata(). > > +vendor: rkisp1 > > +controls: > > + # --- RkISP1 DPF controls --- > > + - DenoiseMode: > > + type: int32_t > > + direction: inout > > + description: | > > + Controls the operating mode of the rkisp1 denoise pipeline. > This setting is > > over 80 cols. Please reflow > > > + consumed by the DPF module and any additional denoise > algorithms that honour > > + the common mode state exposed here. > > The assumption is that all algorithms implement support for the core > controls. You can remove the last sentence. > > + > > + In Disabled mode (default) all denoise processing stages are > disabled. This > > + bypasses noise reduction and surfaces raw sensor data. Other > denoise-related > > + controls are ignored in this mode. > > + > > + In Auto mode each denoise algorithm selects parameters > automatically from the > > + sensor tuning data and (if provided) the exposure index level > table. Changes in > > + scene brightness (analogue gain / exposure index) may cause > algorithms to > > + re-evaluate and reprogram hardware when an exposure index band > boundary is > > + crossed. > > + > > + In Manual mode the automatically selected parameters are frozen > at the moment > > + the mode switch occurs (a snapshot is taken). Subsequent > per-frame automatic > > + updates are disabled until the mode is set back to Auto. While > in Manual mode > > + the application may modify any algorithm-specific override > controls (for DPF: > > + DpfChannelStrengths, DpfGreenSpatialCoefficients, > DpfRedBlueSpatialCoefficients, > > + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, > DpfNoiseLevelLookupScaleMode). > > Where are these controls defined ? > > I would start simple and add Manual later on. > > > + Individual denoise modules apply changes immediately to > hardware. > > + > > + In Reduction mode the algorithms use predefined noise reduction > settings based > > + on the active NoiseReductionMode control. This provides > optimized presets for > > + different noise reduction levels (e.g., minimal, high-quality, > fast) without > > + manual parameter tuning. > > Ah! > > Question is, you have an "auto" mode here, while NoiseReductionMode > specifies profiles (Off, Fast, HighQuality, Minimal, ZSL). Those > profiles comes straight from the Android's definition and raspberry pi > is the only IPA that supports them. > > Can we express "auto" (and "manual") as one of the "profiles" already > existing in NoiseReductionMode ? > > Or maybe we'll need an additional control to specify "auto, off, > manual, profile" like you did. But it really should be a core > control not a platform specific one. > > Something like: > > NoiseReductionMode: > enum: > - Off > - Auto: > - Manual > - Profiles > > NoiseReductionProfiles: > enum: > - Fast > - HighQuality > - Minimal > - ZSL > > Or maybe: > > NoiseReductionMode: > enum: > - Off > - Auto > - Manual > - Fast > - HighQuality > - Minimal > - ZSL > > What do you think ? > > I've cc-ed Naush and David to know what they think and have a bit of > an insight on how they handle those profiles. > I agree with Jacopo, this work probably wants to be folded into the draft::NoiseReductionMode controls. We probably also should promote that control out of draft:: as well :) However, I do wonder if such low level controls for the filters (DpfChannelStrengths, DpfGreenSpatialCoefficients, etc.) as listed above is useful to an application? Perhaps pre-set values could be assigned to one of the NR modes/profiles instead? Naush > > > + > > + Transition Rules: > > + * Auto -> Manual: The current effective (auto-selected) > parameters > > + are captured as the initial manual override values. > > + * Manual -> Auto: All manual override state is discarded and > the > > + algorithm immediately reverts to automatic tuning selection, > > + potentially reprogramming hardware that same frame. > > + * Auto/Manual -> Reduction: Switches to using reduction mode > configs > > + based on the current NoiseReductionMode setting. > > + * Reduction -> Auto/Manual: Reverts to the specified mode, > discarding > > + reduction-specific state. > > + > > + If the application switches to Manual but supplies no overrides, > > + the previously auto-derived parameters continue to be used > unchanged. > > + enum: > > + - name: DenoiseModeDisabled > > + value: 0 > > + description: | > > + Disabled mode - DPF processing is completely disabled. > > + - name: DenoiseModeAuto > > + value: 1 > > + description: | > > + Automatic mode - algorithm selects parameters based on > tuning data. > > + - name: DenoiseModeManual > > + value: 2 > > + description: | > > + Manual mode - parameters are frozen and can be overridden. > > + - name: DenoiseModeReduction > > + value: 3 > > + description: | > > + Reduction mode - uses predefined noise reduction settings > from tuning data. > > +... > > diff --git a/src/libcamera/control_ranges.yaml > b/src/libcamera/control_ranges.yaml > > index 6752eb98..fc60de80 100644 > > --- a/src/libcamera/control_ranges.yaml > > +++ b/src/libcamera/control_ranges.yaml > > @@ -15,6 +15,8 @@ ranges: > > rpi: 20000 > > # Controls for debug metadata > > debug: 30000 > > - # Next range starts at 40000 > > + # RkISP1 vendor controls > > + rkisp1: 40000 > > + # Next range starts at 50000 > > > > ... > > -- > > 2.43.0 > > >
Quoting Naushir Patuck (2025-12-01 09:28:41) > Hi. > > On Fri, 28 Nov 2025 at 16:04, Jacopo Mondi <jacopo.mondi@ideasonboard.com> > wrote: > > > Hi Rui > > > > On Mon, Nov 24, 2025 at 07:08:40PM -0500, Rui Wang wrote: > > > Reserve the next free vendor allocation for rkisp1 and add the dedicated > > > control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the > > > generated control headers expose the rkisp1 control namespace. > > > > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > > > --- > > > include/libcamera/meson.build | 1 + > > > src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ > > > src/libcamera/control_ranges.yaml | 4 +- > > > 3 files changed, 77 insertions(+), 1 deletion(-) > > > create mode 100644 src/libcamera/control_ids_rkisp1.yaml > > > > > > diff --git a/include/libcamera/meson.build > > b/include/libcamera/meson.build > > > index 30ea76f9..3fd7ef94 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -39,6 +39,7 @@ controls_map = { > > > 'draft': 'control_ids_draft.yaml', > > > 'rpi/pisp': 'control_ids_rpi.yaml', > > > 'rpi/vc4': 'control_ids_rpi.yaml', > > > + 'rkisp1': 'control_ids_rkisp1.yaml', > > > }, > > > > > > 'properties': { > > > diff --git a/src/libcamera/control_ids_rkisp1.yaml > > b/src/libcamera/control_ids_rkisp1.yaml > > > new file mode 100644 > > > index 00000000..3e008ee4 > > > --- /dev/null > > > +++ b/src/libcamera/control_ids_rkisp1.yaml > > > @@ -0,0 +1,73 @@ > > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > > +# > > > +# Copyright (C) 2025, Ideas On Board > > > +# > > > +# RkISP1 specific control definitions > > > > Why are these controls RkISP1 specific ? > > > > Ideally we want all controls to be supported by all platforms, but > > even if a single platform supports them, at least their definition > > should be as generic as possible. > > > > Move these to control_ids_core please > > > > Or better, we have draft::NoiseReductionMode from Android. > > Time to make it a core control as well ? > > > > > +--- > > > +# Unless otherwise stated, all controls are bi-directional, i.e. they > > can be > > > +# set through Request::controls() and returned out through > > Request::metadata(). > > > +vendor: rkisp1 > > > +controls: > > > + # --- RkISP1 DPF controls --- > > > + - DenoiseMode: > > > + type: int32_t > > > + direction: inout > > > + description: | > > > + Controls the operating mode of the rkisp1 denoise pipeline. > > This setting is > > > > over 80 cols. Please reflow > > > > > + consumed by the DPF module and any additional denoise > > algorithms that honour > > > + the common mode state exposed here. > > > > The assumption is that all algorithms implement support for the core > > controls. You can remove the last sentence. > > > + > > > + In Disabled mode (default) all denoise processing stages are > > disabled. This > > > + bypasses noise reduction and surfaces raw sensor data. Other > > denoise-related > > > + controls are ignored in this mode. > > > + > > > + In Auto mode each denoise algorithm selects parameters > > automatically from the > > > + sensor tuning data and (if provided) the exposure index level > > table. Changes in > > > + scene brightness (analogue gain / exposure index) may cause > > algorithms to > > > + re-evaluate and reprogram hardware when an exposure index band > > boundary is > > > + crossed. > > > + > > > + In Manual mode the automatically selected parameters are frozen > > at the moment > > > + the mode switch occurs (a snapshot is taken). Subsequent > > per-frame automatic > > > + updates are disabled until the mode is set back to Auto. While > > in Manual mode > > > + the application may modify any algorithm-specific override > > controls (for DPF: > > > + DpfChannelStrengths, DpfGreenSpatialCoefficients, > > DpfRedBlueSpatialCoefficients, > > > + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, > > DpfNoiseLevelLookupScaleMode). > > > > Where are these controls defined ? > > > > I would start simple and add Manual later on. > > > > > + Individual denoise modules apply changes immediately to > > hardware. > > > + > > > + In Reduction mode the algorithms use predefined noise reduction > > settings based > > > + on the active NoiseReductionMode control. This provides > > optimized presets for > > > + different noise reduction levels (e.g., minimal, high-quality, > > fast) without > > > + manual parameter tuning. > > > > Ah! > > > > Question is, you have an "auto" mode here, while NoiseReductionMode > > specifies profiles (Off, Fast, HighQuality, Minimal, ZSL). Those > > profiles comes straight from the Android's definition and raspberry pi > > is the only IPA that supports them. > > > > Can we express "auto" (and "manual") as one of the "profiles" already > > existing in NoiseReductionMode ? > > > > Or maybe we'll need an additional control to specify "auto, off, > > manual, profile" like you did. But it really should be a core > > control not a platform specific one. > > > > Something like: > > > > NoiseReductionMode: > > enum: > > - Off > > - Auto: > > - Manual > > - Profiles > > > > NoiseReductionProfiles: > > enum: > > - Fast > > - HighQuality > > - Minimal > > - ZSL > > > > Or maybe: > > > > NoiseReductionMode: > > enum: > > - Off > > - Auto > > - Manual > > - Fast > > - HighQuality > > - Minimal > > - ZSL > > > > What do you think ? > > > > I've cc-ed Naush and David to know what they think and have a bit of > > an insight on how they handle those profiles. > > > > I agree with Jacopo, this work probably wants to be folded into the > draft::NoiseReductionMode controls. We probably also should promote that > control out of draft:: as well :) > > However, I do wonder if such low level controls for the filters > (DpfChannelStrengths, DpfGreenSpatialCoefficients, etc.) as listed above is > useful to an application? Perhaps pre-set values could be assigned to one > of the NR modes/profiles instead? I suspect some of the rkisp1 specific low level controls could be kept in the rkisp1 namespace for runtime tuning controls (perhaps disabled by default) - but yes this should definitely be targetting 'global' controls and I would really like to see those moved out of draft. -- Kieran > > Naush > > > > > > > + > > > + Transition Rules: > > > + * Auto -> Manual: The current effective (auto-selected) > > parameters > > > + are captured as the initial manual override values. > > > + * Manual -> Auto: All manual override state is discarded and > > the > > > + algorithm immediately reverts to automatic tuning selection, > > > + potentially reprogramming hardware that same frame. > > > + * Auto/Manual -> Reduction: Switches to using reduction mode > > configs > > > + based on the current NoiseReductionMode setting. > > > + * Reduction -> Auto/Manual: Reverts to the specified mode, > > discarding > > > + reduction-specific state. > > > + > > > + If the application switches to Manual but supplies no overrides, > > > + the previously auto-derived parameters continue to be used > > unchanged. > > > + enum: > > > + - name: DenoiseModeDisabled > > > + value: 0 > > > + description: | > > > + Disabled mode - DPF processing is completely disabled. > > > + - name: DenoiseModeAuto > > > + value: 1 > > > + description: | > > > + Automatic mode - algorithm selects parameters based on > > tuning data. > > > + - name: DenoiseModeManual > > > + value: 2 > > > + description: | > > > + Manual mode - parameters are frozen and can be overridden. > > > + - name: DenoiseModeReduction > > > + value: 3 > > > + description: | > > > + Reduction mode - uses predefined noise reduction settings > > from tuning data. > > > +... > > > diff --git a/src/libcamera/control_ranges.yaml > > b/src/libcamera/control_ranges.yaml > > > index 6752eb98..fc60de80 100644 > > > --- a/src/libcamera/control_ranges.yaml > > > +++ b/src/libcamera/control_ranges.yaml > > > @@ -15,6 +15,8 @@ ranges: > > > rpi: 20000 > > > # Controls for debug metadata > > > debug: 30000 > > > - # Next range starts at 40000 > > > + # RkISP1 vendor controls > > > + rkisp1: 40000 > > > + # Next range starts at 50000 > > > > > > ... > > > -- > > > 2.43.0 > > > > >
Hi everyone On Mon, 1 Dec 2025 at 09:34, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Naushir Patuck (2025-12-01 09:28:41) > > Hi. > > > > On Fri, 28 Nov 2025 at 16:04, Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > wrote: > > > > > Hi Rui > > > > > > On Mon, Nov 24, 2025 at 07:08:40PM -0500, Rui Wang wrote: > > > > Reserve the next free vendor allocation for rkisp1 and add the dedicated > > > > control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the > > > > generated control headers expose the rkisp1 control namespace. > > > > > > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > > > > --- > > > > include/libcamera/meson.build | 1 + > > > > src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ > > > > src/libcamera/control_ranges.yaml | 4 +- > > > > 3 files changed, 77 insertions(+), 1 deletion(-) > > > > create mode 100644 src/libcamera/control_ids_rkisp1.yaml > > > > > > > > diff --git a/include/libcamera/meson.build > > > b/include/libcamera/meson.build > > > > index 30ea76f9..3fd7ef94 100644 > > > > --- a/include/libcamera/meson.build > > > > +++ b/include/libcamera/meson.build > > > > @@ -39,6 +39,7 @@ controls_map = { > > > > 'draft': 'control_ids_draft.yaml', > > > > 'rpi/pisp': 'control_ids_rpi.yaml', > > > > 'rpi/vc4': 'control_ids_rpi.yaml', > > > > + 'rkisp1': 'control_ids_rkisp1.yaml', > > > > }, > > > > > > > > 'properties': { > > > > diff --git a/src/libcamera/control_ids_rkisp1.yaml > > > b/src/libcamera/control_ids_rkisp1.yaml > > > > new file mode 100644 > > > > index 00000000..3e008ee4 > > > > --- /dev/null > > > > +++ b/src/libcamera/control_ids_rkisp1.yaml > > > > @@ -0,0 +1,73 @@ > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > > > +# > > > > +# Copyright (C) 2025, Ideas On Board > > > > +# > > > > +# RkISP1 specific control definitions > > > > > > Why are these controls RkISP1 specific ? > > > > > > Ideally we want all controls to be supported by all platforms, but > > > even if a single platform supports them, at least their definition > > > should be as generic as possible. > > > > > > Move these to control_ids_core please > > > > > > Or better, we have draft::NoiseReductionMode from Android. > > > Time to make it a core control as well ? > > > > > > > +--- > > > > +# Unless otherwise stated, all controls are bi-directional, i.e. they > > > can be > > > > +# set through Request::controls() and returned out through > > > Request::metadata(). > > > > +vendor: rkisp1 > > > > +controls: > > > > + # --- RkISP1 DPF controls --- > > > > + - DenoiseMode: > > > > + type: int32_t > > > > + direction: inout > > > > + description: | > > > > + Controls the operating mode of the rkisp1 denoise pipeline. > > > This setting is > > > > > > over 80 cols. Please reflow > > > > > > > + consumed by the DPF module and any additional denoise > > > algorithms that honour > > > > + the common mode state exposed here. > > > > > > The assumption is that all algorithms implement support for the core > > > controls. You can remove the last sentence. > > > > + > > > > + In Disabled mode (default) all denoise processing stages are > > > disabled. This > > > > + bypasses noise reduction and surfaces raw sensor data. Other > > > denoise-related > > > > + controls are ignored in this mode. > > > > + > > > > + In Auto mode each denoise algorithm selects parameters > > > automatically from the > > > > + sensor tuning data and (if provided) the exposure index level > > > table. Changes in > > > > + scene brightness (analogue gain / exposure index) may cause > > > algorithms to > > > > + re-evaluate and reprogram hardware when an exposure index band > > > boundary is > > > > + crossed. > > > > + > > > > + In Manual mode the automatically selected parameters are frozen > > > at the moment > > > > + the mode switch occurs (a snapshot is taken). Subsequent > > > per-frame automatic > > > > + updates are disabled until the mode is set back to Auto. While > > > in Manual mode > > > > + the application may modify any algorithm-specific override > > > controls (for DPF: > > > > + DpfChannelStrengths, DpfGreenSpatialCoefficients, > > > DpfRedBlueSpatialCoefficients, > > > > + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, > > > DpfNoiseLevelLookupScaleMode). > > > > > > Where are these controls defined ? > > > > > > I would start simple and add Manual later on. > > > > > > > + Individual denoise modules apply changes immediately to > > > hardware. > > > > + > > > > + In Reduction mode the algorithms use predefined noise reduction > > > settings based > > > > + on the active NoiseReductionMode control. This provides > > > optimized presets for > > > > + different noise reduction levels (e.g., minimal, high-quality, > > > fast) without > > > > + manual parameter tuning. > > > > > > Ah! > > > > > > Question is, you have an "auto" mode here, while NoiseReductionMode > > > specifies profiles (Off, Fast, HighQuality, Minimal, ZSL). Those > > > profiles comes straight from the Android's definition and raspberry pi > > > is the only IPA that supports them. > > > > > > Can we express "auto" (and "manual") as one of the "profiles" already > > > existing in NoiseReductionMode ? > > > > > > Or maybe we'll need an additional control to specify "auto, off, > > > manual, profile" like you did. But it really should be a core > > > control not a platform specific one. > > > > > > Something like: > > > > > > NoiseReductionMode: > > > enum: > > > - Off > > > - Auto: > > > - Manual > > > - Profiles > > > > > > NoiseReductionProfiles: > > > enum: > > > - Fast > > > - HighQuality > > > - Minimal > > > - ZSL > > > > > > Or maybe: > > > > > > NoiseReductionMode: > > > enum: > > > - Off > > > - Auto > > > - Manual > > > - Fast > > > - HighQuality > > > - Minimal > > > - ZSL > > > > > > What do you think ? Yes, I think I probably like this approach the most, that is, fold everything into the NoiseReductionMode. We (RPi) don't have any manual controls, so I guess the "Manual" would want to be optional. I'm also not 100% sure about "Auto". What does that mean? Do you have to have some idea about your use case before "Auto" can decide what to do (which effectively means, selecting one of the other modes)? As regards the manual mode, having platform-specific controls for that seems OK to me, though of course that does make it difficult to create properly portable applications. Is there any value in considering a "NoiseReductionStrength" control? This might be as simple as a "low", "medium", "high" setting, defaulting to "medium". David > > > > > > I've cc-ed Naush and David to know what they think and have a bit of > > > an insight on how they handle those profiles. > > > > > > > I agree with Jacopo, this work probably wants to be folded into the > > draft::NoiseReductionMode controls. We probably also should promote that > > control out of draft:: as well :) > > > > However, I do wonder if such low level controls for the filters > > (DpfChannelStrengths, DpfGreenSpatialCoefficients, etc.) as listed above is > > useful to an application? Perhaps pre-set values could be assigned to one > > of the NR modes/profiles instead? > > I suspect some of the rkisp1 specific low level controls could be kept > in the rkisp1 namespace for runtime tuning controls (perhaps disabled by > default) - but yes this should definitely be targetting 'global' > controls and I would really like to see those moved out of draft. > > -- > Kieran > > > > > > Naush > > > > > > > > > > > + > > > > + Transition Rules: > > > > + * Auto -> Manual: The current effective (auto-selected) > > > parameters > > > > + are captured as the initial manual override values. > > > > + * Manual -> Auto: All manual override state is discarded and > > > the > > > > + algorithm immediately reverts to automatic tuning selection, > > > > + potentially reprogramming hardware that same frame. > > > > + * Auto/Manual -> Reduction: Switches to using reduction mode > > > configs > > > > + based on the current NoiseReductionMode setting. > > > > + * Reduction -> Auto/Manual: Reverts to the specified mode, > > > discarding > > > > + reduction-specific state. > > > > + > > > > + If the application switches to Manual but supplies no overrides, > > > > + the previously auto-derived parameters continue to be used > > > unchanged. > > > > + enum: > > > > + - name: DenoiseModeDisabled > > > > + value: 0 > > > > + description: | > > > > + Disabled mode - DPF processing is completely disabled. > > > > + - name: DenoiseModeAuto > > > > + value: 1 > > > > + description: | > > > > + Automatic mode - algorithm selects parameters based on > > > tuning data. > > > > + - name: DenoiseModeManual > > > > + value: 2 > > > > + description: | > > > > + Manual mode - parameters are frozen and can be overridden. > > > > + - name: DenoiseModeReduction > > > > + value: 3 > > > > + description: | > > > > + Reduction mode - uses predefined noise reduction settings > > > from tuning data. > > > > +... > > > > diff --git a/src/libcamera/control_ranges.yaml > > > b/src/libcamera/control_ranges.yaml > > > > index 6752eb98..fc60de80 100644 > > > > --- a/src/libcamera/control_ranges.yaml > > > > +++ b/src/libcamera/control_ranges.yaml > > > > @@ -15,6 +15,8 @@ ranges: > > > > rpi: 20000 > > > > # Controls for debug metadata > > > > debug: 30000 > > > > - # Next range starts at 40000 > > > > + # RkISP1 vendor controls > > > > + rkisp1: 40000 > > > > + # Next range starts at 50000 > > > > > > > > ... > > > > -- > > > > 2.43.0 > > > > > > >
Hi David, Naush On Mon, Dec 01, 2025 at 10:20:53AM +0000, David Plowman wrote: > Hi everyone > > > On Mon, 1 Dec 2025 at 09:34, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: > > > > Quoting Naushir Patuck (2025-12-01 09:28:41) > > > Hi. > > > > > > On Fri, 28 Nov 2025 at 16:04, Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > wrote: > > > > > > > Hi Rui > > > > > > > > On Mon, Nov 24, 2025 at 07:08:40PM -0500, Rui Wang wrote: > > > > > Reserve the next free vendor allocation for rkisp1 and add the dedicated > > > > > control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the > > > > > generated control headers expose the rkisp1 control namespace. > > > > > > > > > > Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> > > > > > --- > > > > > include/libcamera/meson.build | 1 + > > > > > src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ > > > > > src/libcamera/control_ranges.yaml | 4 +- > > > > > 3 files changed, 77 insertions(+), 1 deletion(-) > > > > > create mode 100644 src/libcamera/control_ids_rkisp1.yaml > > > > > > > > > > diff --git a/include/libcamera/meson.build > > > > b/include/libcamera/meson.build > > > > > index 30ea76f9..3fd7ef94 100644 > > > > > --- a/include/libcamera/meson.build > > > > > +++ b/include/libcamera/meson.build > > > > > @@ -39,6 +39,7 @@ controls_map = { > > > > > 'draft': 'control_ids_draft.yaml', > > > > > 'rpi/pisp': 'control_ids_rpi.yaml', > > > > > 'rpi/vc4': 'control_ids_rpi.yaml', > > > > > + 'rkisp1': 'control_ids_rkisp1.yaml', > > > > > }, > > > > > > > > > > 'properties': { > > > > > diff --git a/src/libcamera/control_ids_rkisp1.yaml > > > > b/src/libcamera/control_ids_rkisp1.yaml > > > > > new file mode 100644 > > > > > index 00000000..3e008ee4 > > > > > --- /dev/null > > > > > +++ b/src/libcamera/control_ids_rkisp1.yaml > > > > > @@ -0,0 +1,73 @@ > > > > > +# SPDX-License-Identifier: LGPL-2.1-or-later > > > > > +# > > > > > +# Copyright (C) 2025, Ideas On Board > > > > > +# > > > > > +# RkISP1 specific control definitions > > > > > > > > Why are these controls RkISP1 specific ? > > > > > > > > Ideally we want all controls to be supported by all platforms, but > > > > even if a single platform supports them, at least their definition > > > > should be as generic as possible. > > > > > > > > Move these to control_ids_core please > > > > > > > > Or better, we have draft::NoiseReductionMode from Android. > > > > Time to make it a core control as well ? > > > > > > > > > +--- > > > > > +# Unless otherwise stated, all controls are bi-directional, i.e. they > > > > can be > > > > > +# set through Request::controls() and returned out through > > > > Request::metadata(). > > > > > +vendor: rkisp1 > > > > > +controls: > > > > > + # --- RkISP1 DPF controls --- > > > > > + - DenoiseMode: > > > > > + type: int32_t > > > > > + direction: inout > > > > > + description: | > > > > > + Controls the operating mode of the rkisp1 denoise pipeline. > > > > This setting is > > > > > > > > over 80 cols. Please reflow > > > > > > > > > + consumed by the DPF module and any additional denoise > > > > algorithms that honour > > > > > + the common mode state exposed here. > > > > > > > > The assumption is that all algorithms implement support for the core > > > > controls. You can remove the last sentence. > > > > > + > > > > > + In Disabled mode (default) all denoise processing stages are > > > > disabled. This > > > > > + bypasses noise reduction and surfaces raw sensor data. Other > > > > denoise-related > > > > > + controls are ignored in this mode. > > > > > + > > > > > + In Auto mode each denoise algorithm selects parameters > > > > automatically from the > > > > > + sensor tuning data and (if provided) the exposure index level > > > > table. Changes in > > > > > + scene brightness (analogue gain / exposure index) may cause > > > > algorithms to > > > > > + re-evaluate and reprogram hardware when an exposure index band > > > > boundary is > > > > > + crossed. > > > > > + > > > > > + In Manual mode the automatically selected parameters are frozen > > > > at the moment > > > > > + the mode switch occurs (a snapshot is taken). Subsequent > > > > per-frame automatic > > > > > + updates are disabled until the mode is set back to Auto. While > > > > in Manual mode > > > > > + the application may modify any algorithm-specific override > > > > controls (for DPF: > > > > > + DpfChannelStrengths, DpfGreenSpatialCoefficients, > > > > DpfRedBlueSpatialCoefficients, > > > > > + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, > > > > DpfNoiseLevelLookupScaleMode). > > > > > > > > Where are these controls defined ? > > > > > > > > I would start simple and add Manual later on. > > > > > > > > > + Individual denoise modules apply changes immediately to > > > > hardware. > > > > > + > > > > > + In Reduction mode the algorithms use predefined noise reduction > > > > settings based > > > > > + on the active NoiseReductionMode control. This provides > > > > optimized presets for > > > > > + different noise reduction levels (e.g., minimal, high-quality, > > > > fast) without > > > > > + manual parameter tuning. > > > > > > > > Ah! > > > > > > > > Question is, you have an "auto" mode here, while NoiseReductionMode > > > > specifies profiles (Off, Fast, HighQuality, Minimal, ZSL). Those > > > > profiles comes straight from the Android's definition and raspberry pi > > > > is the only IPA that supports them. > > > > > > > > Can we express "auto" (and "manual") as one of the "profiles" already > > > > existing in NoiseReductionMode ? > > > > > > > > Or maybe we'll need an additional control to specify "auto, off, > > > > manual, profile" like you did. But it really should be a core > > > > control not a platform specific one. > > > > > > > > Something like: > > > > > > > > NoiseReductionMode: > > > > enum: > > > > - Off > > > > - Auto: > > > > - Manual > > > > - Profiles > > > > > > > > NoiseReductionProfiles: > > > > enum: > > > > - Fast > > > > - HighQuality > > > > - Minimal > > > > - ZSL > > > > > > > > Or maybe: > > > > > > > > NoiseReductionMode: > > > > enum: > > > > - Off > > > > - Auto > > > > - Manual > > > > - Fast > > > > - HighQuality > > > > - Minimal > > > > - ZSL > > > > > > > > What do you think ? > > Yes, I think I probably like this approach the most, that is, fold > everything into the NoiseReductionMode. > > We (RPi) don't have any manual controls, so I guess the "Manual" would > want to be optional. I guess which modes to support is always platform-optional (this applies to most, if not all controls). > > I'm also not 100% sure about "Auto". What does that mean? Do you have > to have some idea about your use case before "Auto" can decide what to > do (which effectively means, selecting one of the other modes)? Are you suggesting that "Auto" doesn't really mean anything and we have to select one of these profiles anyway ? I'll let Rui expand on this as he certainly knows better. > > As regards the manual mode, having platform-specific controls for that > seems OK to me, though of course that does make it difficult to create > properly portable applications. Is there any value in considering a > "NoiseReductionStrength" control? This might be as simple as a "low", > "medium", "high" setting, defaulting to "medium". I guess these are the DpfChannelStrengths, DpfGreenSpatialCoefficients, DpfRedBlueSpatialCoefficients, DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, DpfNoiseLevelLookupScaleMode controls introduced later in the series, and which Naush is discussing below as well > > David > > > > > > > > > I've cc-ed Naush and David to know what they think and have a bit of > > > > an insight on how they handle those profiles. > > > > > > > > > > I agree with Jacopo, this work probably wants to be folded into the > > > draft::NoiseReductionMode controls. We probably also should promote that > > > control out of draft:: as well :) > > > > > > However, I do wonder if such low level controls for the filters > > > (DpfChannelStrengths, DpfGreenSpatialCoefficients, etc.) as listed above is > > > useful to an application? Perhaps pre-set values could be assigned to one > > > of the NR modes/profiles instead? > > > > I suspect some of the rkisp1 specific low level controls could be kept > > in the rkisp1 namespace for runtime tuning controls (perhaps disabled by > > default) - but yes this should definitely be targetting 'global' > > controls and I would really like to see those moved out of draft. > > DpfChannelStrengths, DpfGreenSpatialCoefficients, DpfRedBlueSpatialCoefficients, DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, DpfNoiseLevelLookupScaleMode looks very low level and maybe platform specific as well. I wouldn't be against reserving low levels controls for Manual mode to be platform-specific ones.. Could it be a way forward ? > > -- > > Kieran > > > > > > > > > > Naush > > > > > > > > > > > > > > > + > > > > > + Transition Rules: > > > > > + * Auto -> Manual: The current effective (auto-selected) > > > > parameters > > > > > + are captured as the initial manual override values. > > > > > + * Manual -> Auto: All manual override state is discarded and > > > > the > > > > > + algorithm immediately reverts to automatic tuning selection, > > > > > + potentially reprogramming hardware that same frame. > > > > > + * Auto/Manual -> Reduction: Switches to using reduction mode > > > > configs > > > > > + based on the current NoiseReductionMode setting. > > > > > + * Reduction -> Auto/Manual: Reverts to the specified mode, > > > > discarding > > > > > + reduction-specific state. > > > > > + > > > > > + If the application switches to Manual but supplies no overrides, > > > > > + the previously auto-derived parameters continue to be used > > > > unchanged. > > > > > + enum: > > > > > + - name: DenoiseModeDisabled > > > > > + value: 0 > > > > > + description: | > > > > > + Disabled mode - DPF processing is completely disabled. > > > > > + - name: DenoiseModeAuto > > > > > + value: 1 > > > > > + description: | > > > > > + Automatic mode - algorithm selects parameters based on > > > > tuning data. > > > > > + - name: DenoiseModeManual > > > > > + value: 2 > > > > > + description: | > > > > > + Manual mode - parameters are frozen and can be overridden. > > > > > + - name: DenoiseModeReduction > > > > > + value: 3 > > > > > + description: | > > > > > + Reduction mode - uses predefined noise reduction settings > > > > from tuning data. > > > > > +... > > > > > diff --git a/src/libcamera/control_ranges.yaml > > > > b/src/libcamera/control_ranges.yaml > > > > > index 6752eb98..fc60de80 100644 > > > > > --- a/src/libcamera/control_ranges.yaml > > > > > +++ b/src/libcamera/control_ranges.yaml > > > > > @@ -15,6 +15,8 @@ ranges: > > > > > rpi: 20000 > > > > > # Controls for debug metadata > > > > > debug: 30000 > > > > > - # Next range starts at 40000 > > > > > + # RkISP1 vendor controls > > > > > + rkisp1: 40000 > > > > > + # Next range starts at 50000 > > > > > > > > > > ... > > > > > -- > > > > > 2.43.0 > > > > > > > > >
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 30ea76f9..3fd7ef94 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -39,6 +39,7 @@ controls_map = { 'draft': 'control_ids_draft.yaml', 'rpi/pisp': 'control_ids_rpi.yaml', 'rpi/vc4': 'control_ids_rpi.yaml', + 'rkisp1': 'control_ids_rkisp1.yaml', }, 'properties': { diff --git a/src/libcamera/control_ids_rkisp1.yaml b/src/libcamera/control_ids_rkisp1.yaml new file mode 100644 index 00000000..3e008ee4 --- /dev/null +++ b/src/libcamera/control_ids_rkisp1.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2025, Ideas On Board +# +# RkISP1 specific control definitions +--- +# Unless otherwise stated, all controls are bi-directional, i.e. they can be +# set through Request::controls() and returned out through Request::metadata(). +vendor: rkisp1 +controls: + # --- RkISP1 DPF controls --- + - DenoiseMode: + type: int32_t + direction: inout + description: | + Controls the operating mode of the rkisp1 denoise pipeline. This setting is + consumed by the DPF module and any additional denoise algorithms that honour + the common mode state exposed here. + + In Disabled mode (default) all denoise processing stages are disabled. This + bypasses noise reduction and surfaces raw sensor data. Other denoise-related + controls are ignored in this mode. + + In Auto mode each denoise algorithm selects parameters automatically from the + sensor tuning data and (if provided) the exposure index level table. Changes in + scene brightness (analogue gain / exposure index) may cause algorithms to + re-evaluate and reprogram hardware when an exposure index band boundary is + crossed. + + In Manual mode the automatically selected parameters are frozen at the moment + the mode switch occurs (a snapshot is taken). Subsequent per-frame automatic + updates are disabled until the mode is set back to Auto. While in Manual mode + the application may modify any algorithm-specific override controls (for DPF: + DpfChannelStrengths, DpfGreenSpatialCoefficients, DpfRedBlueSpatialCoefficients, + DpfRbFilterSize, DpfNoiseLevelLookupCoefficients, DpfNoiseLevelLookupScaleMode). + Individual denoise modules apply changes immediately to hardware. + + In Reduction mode the algorithms use predefined noise reduction settings based + on the active NoiseReductionMode control. This provides optimized presets for + different noise reduction levels (e.g., minimal, high-quality, fast) without + manual parameter tuning. + + Transition Rules: + * Auto -> Manual: The current effective (auto-selected) parameters + are captured as the initial manual override values. + * Manual -> Auto: All manual override state is discarded and the + algorithm immediately reverts to automatic tuning selection, + potentially reprogramming hardware that same frame. + * Auto/Manual -> Reduction: Switches to using reduction mode configs + based on the current NoiseReductionMode setting. + * Reduction -> Auto/Manual: Reverts to the specified mode, discarding + reduction-specific state. + + If the application switches to Manual but supplies no overrides, + the previously auto-derived parameters continue to be used unchanged. + enum: + - name: DenoiseModeDisabled + value: 0 + description: | + Disabled mode - DPF processing is completely disabled. + - name: DenoiseModeAuto + value: 1 + description: | + Automatic mode - algorithm selects parameters based on tuning data. + - name: DenoiseModeManual + value: 2 + description: | + Manual mode - parameters are frozen and can be overridden. + - name: DenoiseModeReduction + value: 3 + description: | + Reduction mode - uses predefined noise reduction settings from tuning data. +... diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml index 6752eb98..fc60de80 100644 --- a/src/libcamera/control_ranges.yaml +++ b/src/libcamera/control_ranges.yaml @@ -15,6 +15,8 @@ ranges: rpi: 20000 # Controls for debug metadata debug: 30000 - # Next range starts at 40000 + # RkISP1 vendor controls + rkisp1: 40000 + # Next range starts at 50000 ...
Reserve the next free vendor allocation for rkisp1 and add the dedicated control_ids_rkisp1.yaml. Hook the new YAML into the Meson build so the generated control headers expose the rkisp1 control namespace. Signed-off-by: Rui Wang <rui.wang@ideasonboard.com> --- include/libcamera/meson.build | 1 + src/libcamera/control_ids_rkisp1.yaml | 73 +++++++++++++++++++++++++++ src/libcamera/control_ranges.yaml | 4 +- 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 src/libcamera/control_ids_rkisp1.yaml