[libcamera-devel,v2,11/14] ipa: ipu3: Send color temperature in the metadata
diff mbox series

Message ID 20211110195901.85597-12-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • IPA: IPU3: Introduce per-frame controls
Related show

Commit Message

Jean-Michel Hautbois Nov. 10, 2021, 7:58 p.m. UTC
Now that the color temperature is updated per-frame, use the value and
set the corresponding controls::ColourTemperature.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Kieran Bingham Nov. 10, 2021, 10:47 p.m. UTC | #1
Quoting Jean-Michel Hautbois (2021-11-10 19:58:58)
> Now that the color temperature is updated per-frame, use the value and
> set the corresponding controls::ColourTemperature.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 1111f200..eb8c0dc4 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -632,6 +632,14 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>                                 (sensorInfo_.pixelRate / 1e6);
>         ctrls.set(controls::FrameDuration, frameDuration);
>  
> +       ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
> +
> +       /*
> +        * \todo We should be able to add 'anything' (with a Control) in here to
> +        * get information to say.
> +        * It can be for debug purposes (qcam) or for any other HAL.
> +        */

Hrm ... I'm not sure that helps. And if we weren't adding this comment,
the one line could have been in the previous patch.

I think that this comment was based on a discussion we had. I was trying
to say that you could add any arbitrary data you might need to get out
of the IPA for debug while developing. But I don't think that's a
general statement that gets a todo, as we wouldn't merge 'any arbitrary'
data...


If you want to keep it, then it could be something like

"""
	/*
	 * \todo The Metadata provides a path to getting extended data
	 * out to the application. Further data such as a simplifed Histogram
	 * might have value to be exposed, however such data may be
	 * difficult to report in a generically parsable way and we
	 * likely want to avoid putting platform specific metadata in.
	 */
"""

With that, or squashed to the previous patch if you prefer, either way:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +
>         IPU3Action op;
>         op.op = ActionMetadataReady;
>         op.controls = ctrls;
> -- 
> 2.32.0
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 1111f200..eb8c0dc4 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -632,6 +632,14 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 				(sensorInfo_.pixelRate / 1e6);
 	ctrls.set(controls::FrameDuration, frameDuration);
 
+	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
+
+	/*
+	 * \todo We should be able to add 'anything' (with a Control) in here to
+	 * get information to say.
+	 * It can be for debug purposes (qcam) or for any other HAL.
+	 */
+
 	IPU3Action op;
 	op.op = ActionMetadataReady;
 	op.controls = ctrls;