[v4,14/21] libcamera: controls: Define a new core Hue control
diff mbox series

Message ID 20251114005428.90024-15-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • libipa: Introduce a Quantized type
Related show

Commit Message

Kieran Bingham Nov. 14, 2025, 12:54 a.m. UTC 
From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com>

Define a new control to support configuration of Hue adjustments when
supported by the available platform.

Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com>
[Kieran: Rework to define as a rotation in degrees]
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/control_ids_core.yaml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Isaac Scott Nov. 18, 2025, 12:40 p.m. UTC | #1 
Hi Kieran,

Thank you for the patch!

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>

Quoting Kieran Bingham (2025-11-14 00:54:18)
> From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com>
> 
> Define a new control to support configuration of Hue adjustments when
> supported by the available platform.
> 
> Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com>
> [Kieran: Rework to define as a rotation in degrees]
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/control_ids_core.yaml | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index f781865859ac..4993da14b3f6 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -1346,4 +1346,17 @@ controls:
>          reduces the WdrExposureValue until the amount of pixels that are close
>          to saturation is lower than this value.
>  
> +  - Hue:
> +      type: float
> +      direction: inout
> +      description: |
> +        Adjusts the image hue (colour rotation) in degrees.
> +
> +        The value specifies a rotation around the colour wheel:
> +        positive values rotate hues counter-clockwise (for example a +60 degree
> +        rotation turns Red hues to Magenta hues), and negative values rotate
> +        clockwise (a -60 degree rotation turns Red hues to Yellow hues).
> +
> +        A value of 0 leaves the hue unchanged.
> +
>  ...
> -- 
> 2.51.1
>
Laurent Pinchart Nov. 19, 2025, 4:30 a.m. UTC | #2 
On Fri, Nov 14, 2025 at 12:54:18AM +0000, Kieran Bingham wrote:
> From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com>
> 
> Define a new control to support configuration of Hue adjustments when
> supported by the available platform.
> 
> Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com>
> [Kieran: Rework to define as a rotation in degrees]
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/control_ids_core.yaml | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index f781865859ac..4993da14b3f6 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -1346,4 +1346,17 @@ controls:
>          reduces the WdrExposureValue until the amount of pixels that are close
>          to saturation is lower than this value.
>  
> +  - Hue:
> +      type: float
> +      direction: inout
> +      description: |
> +        Adjusts the image hue (colour rotation) in degrees.
> +
> +        The value specifies a rotation around the colour wheel:

A reference to HSV/HSL would be good. I'm not sure "colour wheel" is
such a standard term.

> +        positive values rotate hues counter-clockwise (for example a +60 degree
> +        rotation turns Red hues to Magenta hues), and negative values rotate
> +        clockwise (a -60 degree rotation turns Red hues to Yellow hues).

What's the expected range for this control, ]-180, 180] or [-180, 180[ ?

> +
> +        A value of 0 leaves the hue unchanged.
> +
>  ...
Kieran Bingham Nov. 19, 2025, 3:27 p.m. UTC | #3 
Quoting Laurent Pinchart (2025-11-19 04:30:33)
> On Fri, Nov 14, 2025 at 12:54:18AM +0000, Kieran Bingham wrote:
> > From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com>
> > 
> > Define a new control to support configuration of Hue adjustments when
> > supported by the available platform.
> > 
> > Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com>
> > [Kieran: Rework to define as a rotation in degrees]
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/control_ids_core.yaml | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > index f781865859ac..4993da14b3f6 100644
> > --- a/src/libcamera/control_ids_core.yaml
> > +++ b/src/libcamera/control_ids_core.yaml
> > @@ -1346,4 +1346,17 @@ controls:
> >          reduces the WdrExposureValue until the amount of pixels that are close
> >          to saturation is lower than this value.
> >  
> > +  - Hue:
> > +      type: float
> > +      direction: inout
> > +      description: |
> > +        Adjusts the image hue (colour rotation) in degrees.
> > +
> > +        The value specifies a rotation around the colour wheel:
> 
> A reference to HSV/HSL would be good. I'm not sure "colour wheel" is
> such a standard term.

Attempt below:
> > +        positive values rotate hues counter-clockwise (for example a +60 degree
> > +        rotation turns Red hues to Magenta hues), and negative values rotate
> > +        clockwise (a -60 degree rotation turns Red hues to Yellow hues).
> 
> What's the expected range for this control, ]-180, 180] or [-180, 180[ ?

Being circular - I think it shouldn't matter. So [-180, 180] where both
-180 and 180 are an identical adjustment. Looking at how fixed point
types get represented, if you force me to pick one of the above I'd take
[-180, 180[.

Or maybe someone might really prefer a syntax of ]-360, 360[ and who are
we to stop them only going round in one direction?

But it's likely hardware specific, and should be conveyed through the
ControlInfo as is done by the implementation in RKISP1 which is only
[-90, 90[

So I do not really want to specify limits or expected range here, but if
it's helpful:

> > +
> > +        A value of 0 leaves the hue unchanged.
> > +


How about:

Adjusts the image hue (colour rotation) in degrees, as defined in
the HSL/HSV colour model.

The value represents a rotation around the hue circle in HSL/HSV space:
positive values rotate hues counter-clockwise (for example a +60° turns
Red hues to Magenta hues), and negative values rotate clockwise (a -60°
turns Red hues to Yellow hues).

The nominal range is [-180, 180[, where 0° leaves hues unchanged and the
range wraps around continuously, with 180° == -180°.

A value of 0 leaves the hue unchanged.



> >  ...
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Nov. 20, 2025, 1:44 a.m. UTC | #4 
On Wed, Nov 19, 2025 at 03:27:55PM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2025-11-19 04:30:33)
> > On Fri, Nov 14, 2025 at 12:54:18AM +0000, Kieran Bingham wrote:
> > > From: "van Veen, Stephan" <stephan.vanveen@karlstorz.com>
> > > 
> > > Define a new control to support configuration of Hue adjustments when
> > > supported by the available platform.
> > > 
> > > Signed-off-by: van Veen, Stephan <stephan.vanveen@karlstorz.com>
> > > [Kieran: Rework to define as a rotation in degrees]
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  src/libcamera/control_ids_core.yaml | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > > index f781865859ac..4993da14b3f6 100644
> > > --- a/src/libcamera/control_ids_core.yaml
> > > +++ b/src/libcamera/control_ids_core.yaml
> > > @@ -1346,4 +1346,17 @@ controls:
> > >          reduces the WdrExposureValue until the amount of pixels that are close
> > >          to saturation is lower than this value.
> > >  
> > > +  - Hue:
> > > +      type: float
> > > +      direction: inout
> > > +      description: |
> > > +        Adjusts the image hue (colour rotation) in degrees.
> > > +
> > > +        The value specifies a rotation around the colour wheel:
> > 
> > A reference to HSV/HSL would be good. I'm not sure "colour wheel" is
> > such a standard term.
> 
> Attempt below:
> > > +        positive values rotate hues counter-clockwise (for example a +60 degree
> > > +        rotation turns Red hues to Magenta hues), and negative values rotate
> > > +        clockwise (a -60 degree rotation turns Red hues to Yellow hues).
> > 
> > What's the expected range for this control, ]-180, 180] or [-180, 180[ ?
> 
> Being circular - I think it shouldn't matter. So [-180, 180] where both
> -180 and 180 are an identical adjustment. Looking at how fixed point
> types get represented, if you force me to pick one of the above I'd take
> [-180, 180[.
> 
> Or maybe someone might really prefer a syntax of ]-360, 360[ and who are
> we to stop them only going round in one direction?
> 
> But it's likely hardware specific, and should be conveyed through the
> ControlInfo as is done by the implementation in RKISP1 which is only
> [-90, 90[

The limits will be hardware-specific, as you mentioned the RKISP1 can't
shift by more than 90° in each direction (not entirely sure why). I
think it's best if we standardize the maximum range though, as that will
make things easier for applications. It's trivial for an IPA module to
then convert to hardware values.

[-180, 180[ seems to be a good range.

> So I do not really want to specify limits or expected range here, but if
> it's helpful:
> 
> > > +
> > > +        A value of 0 leaves the hue unchanged.
> > > +
> 
> How about:
> 
> Adjusts the image hue (colour rotation) in degrees, as defined in
> the HSL/HSV colour model.
> 
> The value represents a rotation around the hue circle in HSL/HSV space:
> positive values rotate hues counter-clockwise (for example a +60° turns
> Red hues to Magenta hues), and negative values rotate clockwise (a -60°
> turns Red hues to Yellow hues).

Where did you get that direction from ? The formulas in
https://en.wikipedia.org/wiki/HSL_and_HSV map 0° to red and 60° to
yellow.

> The nominal range is [-180, 180[, where 0° leaves hues unchanged and the
> range wraps around continuously, with 180° == -180°.
> 
> A value of 0 leaves the hue unchanged.

This duplicates the previous sentence.

> > >  ...

Patch
diff mbox series 

diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
index f781865859ac..4993da14b3f6 100644
--- a/src/libcamera/control_ids_core.yaml
+++ b/src/libcamera/control_ids_core.yaml
@@ -1346,4 +1346,17 @@  controls:
         reduces the WdrExposureValue until the amount of pixels that are close
         to saturation is lower than this value.
 
+  - Hue:
+      type: float
+      direction: inout
+      description: |
+        Adjusts the image hue (colour rotation) in degrees.
+
+        The value specifies a rotation around the colour wheel:
+        positive values rotate hues counter-clockwise (for example a +60 degree
+        rotation turns Red hues to Magenta hues), and negative values rotate
+        clockwise (a -60 degree rotation turns Red hues to Yellow hues).
+
+        A value of 0 leaves the hue unchanged.
+
 ...