[2/3] libcamera: controls: Define a new core Hue control
diff mbox series

Message ID 20250624171223.2181226-3-kieran.bingham@ideasonboard.com
State New
Headers show
Series
  • rkisp1: cproc - Metadata and Hue developments
Related show

Commit Message

Kieran Bingham June 24, 2025, 5:12 p.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: Split control addition and implementation and refactor commit
messages]
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

---

Open questions

How should we standardise the units on this control? Should this be a
scale of -1, 0, +1 and scaled to the capability of the hardware by the
IPA? Or should we standardise on similar units from a colour wheel and
use values in degrees (for RKISP1 use case this would be -90,0,+87.188).

As the RKISP1 makes a phase shift adjustment, is this expected to be
similar on other pipelines? Or should we also expect anything that would
expose a full 360 degree explicit hue? (I assume not now I write that
:D)

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/control_ids_core.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Laurent Pinchart June 24, 2025, 6:44 p.m. UTC | #1
On Tue, Jun 24, 2025 at 06:12:22PM +0100, 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: Split control addition and implementation and refactor commit
> messages]
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> ---
> 
> Open questions
> 
> How should we standardise the units on this control? Should this be a
> scale of -1, 0, +1 and scaled to the capability of the hardware by the
> IPA? Or should we standardise on similar units from a colour wheel and
> use values in degrees (for RKISP1 use case this would be -90,0,+87.188).

A value in degrees seem more standard.

> As the RKISP1 makes a phase shift adjustment, is this expected to be
> similar on other pipelines? Or should we also expect anything that would
> expose a full 360 degree explicit hue? (I assume not now I write that
> :D)

I wonder why the rkisp1 limits the adjustement to [-90, +90[ range. Any
insight ?

> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/control_ids_core.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index 028919ef3d3e..58abf8434eae 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml
> @@ -1281,4 +1281,14 @@ controls:
>  
>          The FrameWallClock control can only be returned in metadata.
>  
> +  - Hue:
> +      type: float
> +      direction: inout
> +      description:  |
> +        Specify a fixed hue parameter.
> +
> +        Positive values (up to 1.0) produce an increase of the hue angle up to
> +        90 degrees; negative values (up to -1.0) produce a decrease of the hue
> +        angle down to -90 degrees.
> +
>  ...
Kieran Bingham July 7, 2025, 5:31 p.m. UTC | #2
Quoting Laurent Pinchart (2025-06-24 19:44:56)
> On Tue, Jun 24, 2025 at 06:12:22PM +0100, 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: Split control addition and implementation and refactor commit
> > messages]
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> > ---
> > 
> > Open questions
> > 
> > How should we standardise the units on this control? Should this be a
> > scale of -1, 0, +1 and scaled to the capability of the hardware by the
> > IPA? Or should we standardise on similar units from a colour wheel and
> > use values in degrees (for RKISP1 use case this would be -90,0,+87.188).
> 
> A value in degrees seem more standard.
> 
> > As the RKISP1 makes a phase shift adjustment, is this expected to be
> > similar on other pipelines? Or should we also expect anything that would
> > expose a full 360 degree explicit hue? (I assume not now I write that
> > :D)
> 
> I wonder why the rkisp1 limits the adjustement to [-90, +90[ range. Any
> insight ?

No, the datasheets are ... very thin on context...

I also think this implementation is backwards.

Using this https://i.sstatic.net/pSUUV.jpg as a color wheel, I can see
that applying +90 Orange goes to Magenta - which I would expect from a
'-90' Hue, and Blue goes to Green, which is also a '-90' Hue adjustment.

Similarly applying -1.0/-90 to the control - orange becomes green in the
resulting image, which I would consider a +90 hue adjustment.

Still - the resulting colour changes are consistent although reversed
from my expectations.

> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/control_ids_core.yaml | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> > index 028919ef3d3e..58abf8434eae 100644
> > --- a/src/libcamera/control_ids_core.yaml
> > +++ b/src/libcamera/control_ids_core.yaml
> > @@ -1281,4 +1281,14 @@ controls:
> >  
> >          The FrameWallClock control can only be returned in metadata.
> >  
> > +  - Hue:
> > +      type: float
> > +      direction: inout
> > +      description:  |
> > +        Specify a fixed hue parameter.
> > +
> > +        Positive values (up to 1.0) produce an increase of the hue angle up to
> > +        90 degrees; negative values (up to -1.0) produce a decrease of the hue
> > +        angle down to -90 degrees.
> > +
> >  ...
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
index 028919ef3d3e..58abf8434eae 100644
--- a/src/libcamera/control_ids_core.yaml
+++ b/src/libcamera/control_ids_core.yaml
@@ -1281,4 +1281,14 @@  controls:
 
         The FrameWallClock control can only be returned in metadata.
 
+  - Hue:
+      type: float
+      direction: inout
+      description:  |
+        Specify a fixed hue parameter.
+
+        Positive values (up to 1.0) produce an increase of the hue angle up to
+        90 degrees; negative values (up to -1.0) produce a decrease of the hue
+        angle down to -90 degrees.
+
 ...