[libcamera-devel] [PATCH v2 2/4] ipa: raspberrypi: Add sensor temperature to DeviceStatus

[David Plowman]

Hi Naush

Thanks for the patch!

All looks fine to me...

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks
David

On Fri, 24 Jun 2022 at 13:32, Kieran Bingham via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Quoting Naushir Patuck via libcamera-devel (2022-06-24 08:35:26)
> > Add an optional sensor_temperature field to the DeviceStatus structure. If a
> > temperature measurement is available for a frame, the value is returned out
> > through the SensorTemperature control in the Request metadata.
> >
> > Additionally, provide the sensor temperature value from the std::ostream &operator<<
> > overload.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > ---
> >  src/ipa/raspberrypi/cam_helper.cpp               | 8 +++++---
> >  src/ipa/raspberrypi/controller/device_status.cpp | 3 +++
> >  src/ipa/raspberrypi/controller/device_status.h   | 3 +++
> >  src/ipa/raspberrypi/raspberrypi.cpp              | 2 ++
> >  4 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> > index 74179399e86a..3f81d4187d4d 100644
> > --- a/src/ipa/raspberrypi/cam_helper.cpp
> > +++ b/src/ipa/raspberrypi/cam_helper.cpp
> > @@ -185,9 +185,9 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
> >         metadata.Merge(parsedMetadata);
> >
> >         /*
> > -        * Overwrite the exposure/gain values in the existing DeviceStatus with
> > -        * values from the parsed embedded buffer. Fetch it first in case any
> > -        * other fields were set meaningfully.
> > +        * Overwrite the exposure/gain, frame length and sensor temperature values
> > +        * in the existing DeviceStatus with values from the parsed embedded buffer.
> > +        * Fetch it first in case any other fields were set meaningfully.
> >          */
> >         DeviceStatus deviceStatus, parsedDeviceStatus;
> >         if (metadata.Get("device.status", deviceStatus) ||
> > @@ -199,6 +199,8 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
> >         deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;
> >         deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;
> >         deviceStatus.frame_length = parsedDeviceStatus.frame_length;
> > +       if (parsedDeviceStatus.sensor_temperature)
> > +               deviceStatus.sensor_temperature = parsedDeviceStatus.sensor_temperature;
> >
> >         LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;
> >
> > diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp
> > index f052ea8b7bed..05897fc15b50 100644
> > --- a/src/ipa/raspberrypi/controller/device_status.cpp
> > +++ b/src/ipa/raspberrypi/controller/device_status.cpp
> > @@ -17,5 +17,8 @@ std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)
> >             << " Lens: " << d.lens_position
> >             << " Flash: " << d.flash_intensity;
> >
> > +       if (d.sensor_temperature)
> > +               out << " Temperature: " << *d.sensor_temperature;
> > +
> >         return out;
> >  }
> > diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h
> > index c4a5d9c8e8c7..eca3bf4b042e 100644
> > --- a/src/ipa/raspberrypi/controller/device_status.h
> > +++ b/src/ipa/raspberrypi/controller/device_status.h
> > @@ -7,6 +7,7 @@
> >  #pragma once
> >
> >  #include <iostream>
> > +#include <optional>
> >
> >  #include <libcamera/base/utils.h>
> >
> > @@ -36,4 +37,6 @@ struct DeviceStatus {
> >         double aperture;
> >         /* proportional to brightness with 0 = no flash, 1 = maximum flash */
> >         double flash_intensity;
> > +       /* Sensor reported temperature value (in degrees) */
> > +       std::optional<double> sensor_temperature;
> >  };
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 3b126bb5175e..cb0025a17aff 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -491,6 +491,8 @@ void IPARPi::reportMetadata()
> >                 libcameraMetadata_.set(controls::AnalogueGain, deviceStatus->analogue_gain);
> >                 libcameraMetadata_.set(controls::FrameDuration,
> >                                        helper_->Exposure(deviceStatus->frame_length).get<std::micro>());
> > +               if (deviceStatus->sensor_temperature)
> > +                       libcameraMetadata_.set(controls::SensorTemperature, *deviceStatus->sensor_temperature);
> >         }
> >
> >         AgcStatus *agcStatus = rpiMetadata_.GetLocked<AgcStatus>("agc.status");
> > --
> > 2.25.1
> >