<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 23 Jun 2022 at 16:12, Dave Stevenson <<a href="mailto:dave.stevenson@raspberrypi.com">dave.stevenson@raspberrypi.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">Hi Naush and Jean-Michel<br>
<br>
On Thu, 23 Jun 2022 at 15:56, Naushir Patuck via libcamera-devel<br>
<<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br>
><br>
> Hi Jean-Michel,<br>
><br>
> On Thu, 23 Jun 2022 at 15:51, Jean-Michel Hautbois <<a href="mailto:jeanmichel.hautbois@yoseli.org" target="_blank">jeanmichel.hautbois@yoseli.org</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On 23/06/2022 16:50, Jean-Michel Hautbois via libcamera-devel wrote:<br>
>> > Hi Naushir,<br>
>> ><br>
>> > Thanks for the series !<br>
>> ><br>
>> > On 23/06/2022 16:44, Naushir Patuck via libcamera-devel wrote:<br>
>> >> Hi,<br>
>> >><br>
>> >> This series adds a sensor temperature metadata control to libcamera.<br>
>> >> This<br>
>> >> control is returned out through Request metadata when available.<br>
>> >> There is an<br>
>> >> open PR [1] to enable the temperature sensor on the imx477 on the<br>
>> >> Raspberry Pi<br>
>> >> platforms.<br>
>> >><br>
>> >> At present, there is no mechanism in V4L2 to return the temperature<br>
>> >> from the<br>
>> >> sensor device driver, so we rely on the embedded data to fetch the<br>
>> >> values every<br>
>> >> frame.<br>
>> ><br>
>> > I think this PR along with a V4L2 control introduction<br>
>> > (V4L2_CID_TEMPERATURE :-) ?) could be interesting to be sent to the<br>
>> > linux-media ML at the same time ?<br>
><br>
><br>
> I was hoping to decouple this with V4L2 changes for now.  From what I can tell,<br>
> the topic of V4L2_CID_TEMPERATURE is a long ongoing discussion, and my<br>
> change here does not rely on it.  So our users can get this feature earlier!<br>
<br>
I've flagged it for discussion at the proposed Media Summit alongside<br>
ELCE in September -<br>
<a href="https://www.spinics.net/lists/linux-media/msg212772.html" rel="noreferrer" target="_blank">https://www.spinics.net/lists/linux-media/msg212772.html</a><br>
<br>
Laurent has responded in support of V4L2_CID_TEMPERATURE in preference<br>
to hwmon API, but that still doesn't make it a done deal.<br>
When Benjamin brought it up in<br>
<a href="https://lore.kernel.org/linux-media/20220415111845.27130-3-benjamin.mugnier@foss.st.com/" rel="noreferrer" target="_blank">https://lore.kernel.org/linux-media/20220415111845.27130-3-benjamin.mugnier@foss.st.com/</a>,<br>
it became quite a long debate with no resolution, leading to the patch<br>
just being dropped.<br>
<br>
As Naush says, with imx477 (and others) carrying this value in<br>
embedded metadata, we may as well extract it from there and avoid the<br>
complication. It'll actually be a more up to date value than reading<br>
it on demand over I2C.<br></blockquote><div><br></div><div>That's a good point - embedded data values will always be guaranteed to be synced with the</div><div>frame in question.  But sync granularity might not be such a big deal for a temperature measurement...</div><div><br></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>
  Dave<br>
<br>
>><br>
>><br>
>> I forgot to ask the use case though ? :-)<br>
><br>
><br>
> We have had a few requests for this feature, see <a href="https://github.com/raspberrypi/libcamera-apps/issues/306" rel="noreferrer" target="_blank">https://github.com/raspberrypi/libcamera-apps/issues/306</a><br>
> for the most recent one where the temperature value is useful for long exposures to<br>
> help with dark frame subtraction in post processing for astrophotography.<br>
><br>
> Regards,<br>
> Naush<br>
><br>
>><br>
>><br>
>> ><br>
>> > JM<br>
>> ><br>
>> >><br>
>> >> Thanks,<br>
>> >> Naush<br>
>> >><br>
>> >> [1] <a href="https://github.com/raspberrypi/linux/pull/5067" rel="noreferrer" target="_blank">https://github.com/raspberrypi/linux/pull/5067</a><br>
>> >><br>
>> >> Naushir Patuck (3):<br>
>> >>    libcamera: controls: Add SensorTemperature control<br>
>> >>    ipa: raspberrypi: Add sensor temperature to DeviceStatus<br>
>> >>    ipa: raspberrypi: imx477: Get sensor temperature from embedded data<br>
>> >><br>
>> >>   src/ipa/raspberrypi/cam_helper.cpp               |  7 ++++---<br>
>> >>   src/ipa/raspberrypi/cam_helper_imx477.cpp        |  5 ++++-<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>
>> >>   src/libcamera/control_ids.yaml                   | 10 ++++++++++<br>
>> >>   6 files changed, 26 insertions(+), 5 deletions(-)<br>
>> >><br>
</blockquote></div></div>