<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 23 Jun 2022 at 17:18, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck via libcamera-devel (2022-06-23 15:44:09)<br>
> Add a sensor_temperature field to the DeviceStatus structure. The default value<br>
> of -300.0 indicates a temperature measurement is unavilable. If a temperature<br>
<br>
/unavilable/unavailable/<br>
<br>
> measurement is available for a frame, the value is returned out through the<br>
> SensorTemperature control in the Request metadata.<br>
<br>
Rather than a 'fake default' - I think std::optional<double> could come<br>
to the rescue here.<br></blockquote><div><br></div><div>I did consider std::optional, but decided against it to keep in the "spirit" of the</div><div>DeviceStatus structure. We have lens_position, aperture and flash_intensity fields</div><div>that are optional but set with a default invalid value.</div><div><br></div><div>Having said that, I much prefer the std::optional approach as well, and I will change</div><div>to use it. As a separate patch, I will also switch the other optional fields to use std::optional.</div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> <br>
> Additionally, provide the sensor temperature value from the std::ostream &operator<<<br>
> overload.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> src/ipa/raspberrypi/cam_helper.cpp | 7 ++++---<br>
> src/ipa/raspberrypi/controller/device_status.cpp | 3 +++<br>
> src/ipa/raspberrypi/controller/device_status.h | 4 +++-<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 2 ++<br>
> 4 files changed, 12 insertions(+), 4 deletions(-)<br>
> <br>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp<br>
> index 74179399e86a..046428de8545 100644<br>
> --- a/src/ipa/raspberrypi/cam_helper.cpp<br>
> +++ b/src/ipa/raspberrypi/cam_helper.cpp<br>
> @@ -185,9 +185,9 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,<br>
> metadata.Merge(parsedMetadata);<br>
> <br>
> /*<br>
> - * Overwrite the exposure/gain values in the existing DeviceStatus with<br>
> - * values from the parsed embedded buffer. Fetch it first in case any<br>
> - * other fields were set meaningfully.<br>
> + * Overwrite the exposure/gain, frame length and sensor temperature values<br>
> + * in the existing DeviceStatus with values from the parsed embedded buffer.<br>
> + * Fetch it first in case any other fields were set meaningfully.<br>
> */<br>
> DeviceStatus deviceStatus, parsedDeviceStatus;<br>
> if (metadata.Get("device.status", deviceStatus) ||<br>
> @@ -199,6 +199,7 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,<br>
> deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;<br>
> deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;<br>
> deviceStatus.frame_length = parsedDeviceStatus.frame_length;<br>
> + deviceStatus.sensor_temperature = parsedDeviceStatus.sensor_temperature;<br>
> <br>
> LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;<br>
> <br>
> diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp<br>
> index f052ea8b7bed..1a376eecb1c3 100644<br>
> --- a/src/ipa/raspberrypi/controller/device_status.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/device_status.cpp<br>
> @@ -17,5 +17,8 @@ std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)<br>
> << " Lens: " << d.lens_position<br>
> << " Flash: " << d.flash_intensity;<br>
> <br>
> + if (d.sensor_temperature != -300)<br>
> + out << " Temperature: " << d.sensor_temperature;<br>
> +<br>
> return out;<br>
> }<br>
> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h<br>
> index c4a5d9c8e8c7..53cfeedc2956 100644<br>
> --- a/src/ipa/raspberrypi/controller/device_status.h<br>
> +++ b/src/ipa/raspberrypi/controller/device_status.h<br>
> @@ -19,7 +19,7 @@ struct DeviceStatus {<br>
> DeviceStatus()<br>
> : shutter_speed(std::chrono::seconds(0)), frame_length(0),<br>
> analogue_gain(0.0), lens_position(0.0), aperture(0.0),<br>
> - flash_intensity(0.0)<br>
> + flash_intensity(0.0), sensor_temperature(-300.0)<br>
> {<br>
> }<br>
> <br>
> @@ -36,4 +36,6 @@ struct DeviceStatus {<br>
> double aperture;<br>
> /* proportional to brightness with 0 = no flash, 1 = maximum flash */<br>
> double flash_intensity;<br>
> + /* Sensor reported temperature value (in degrees) */<br>
> + double sensor_temperature;<br>
> };<br>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> index 3b126bb5175e..fb91c8d11f6c 100644<br>
> --- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> @@ -491,6 +491,8 @@ void IPARPi::reportMetadata()<br>
> libcameraMetadata_.set(controls::AnalogueGain, deviceStatus->analogue_gain);<br>
> libcameraMetadata_.set(controls::FrameDuration,<br>
> helper_->Exposure(deviceStatus->frame_length).get<std::micro>());<br>
> + if (deviceStatus->sensor_temperature != -300)<br>
> + libcameraMetadata_.set(controls::SensorTemperature, deviceStatus->sensor_temperature);<br>
> }<br>
> <br>
> AgcStatus *agcStatus = rpiMetadata_.GetLocked<AgcStatus>("agc.status");<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>