[libcamera-devel] [PATCH 0/3] Add SensorTemperature control

[Jean-Michel Hautbois]

Hi Dave !

On 23/06/2022 17:12, Dave Stevenson wrote:
> Hi Naush and Jean-Michel
> 
> On Thu, 23 Jun 2022 at 15:56, Naushir Patuck via libcamera-devel
> <libcamera-devel at lists.libcamera.org> wrote:
>>
>> Hi Jean-Michel,
>>
>> On Thu, 23 Jun 2022 at 15:51, Jean-Michel Hautbois <jeanmichel.hautbois at yoseli.org> wrote:
>>>
>>>
>>>
>>> On 23/06/2022 16:50, Jean-Michel Hautbois via libcamera-devel wrote:
>>>> Hi Naushir,
>>>>
>>>> Thanks for the series !
>>>>
>>>> On 23/06/2022 16:44, Naushir Patuck via libcamera-devel wrote:
>>>>> Hi,
>>>>>
>>>>> This series adds a sensor temperature metadata control to libcamera.
>>>>> This
>>>>> control is returned out through Request metadata when available.
>>>>> There is an
>>>>> open PR [1] to enable the temperature sensor on the imx477 on the
>>>>> Raspberry Pi
>>>>> platforms.
>>>>>
>>>>> At present, there is no mechanism in V4L2 to return the temperature
>>>>> from the
>>>>> sensor device driver, so we rely on the embedded data to fetch the
>>>>> values every
>>>>> frame.
>>>>
>>>> I think this PR along with a V4L2 control introduction
>>>> (V4L2_CID_TEMPERATURE :-) ?) could be interesting to be sent to the
>>>> linux-media ML at the same time ?
>>
>>
>> I was hoping to decouple this with V4L2 changes for now.  From what I can tell,
>> the topic of V4L2_CID_TEMPERATURE is a long ongoing discussion, and my
>> change here does not rely on it.  So our users can get this feature earlier!
> 
> I've flagged it for discussion at the proposed Media Summit alongside
> ELCE in September -
> https://www.spinics.net/lists/linux-media/msg212772.html
> 
> Laurent has responded in support of V4L2_CID_TEMPERATURE in preference
> to hwmon API, but that still doesn't make it a done deal.
> When Benjamin brought it up in
> https://lore.kernel.org/linux-media/20220415111845.27130-3-benjamin.mugnier@foss.st.com/,
> it became quite a long debate with no resolution, leading to the patch
> just being dropped.

Thanks for the discussion :-) !

> 
> As Naush says, with imx477 (and others) carrying this value in
> embedded metadata, we may as well extract it from there and avoid the
> complication. It'll actually be a more up to date value than reading
> it on demand over I2C.

Yep, I get the point ;-).

> 
>    Dave
> 
>>>
>>>
>>> I forgot to ask the use case though ? :-)
>>
>>
>> We have had a few requests for this feature, see https://github.com/raspberrypi/libcamera-apps/issues/306
>> for the most recent one where the temperature value is useful for long exposures to
>> help with dark frame subtraction in post processing for astrophotography.

Thanks for the reference :-).

JM

>>
>> Regards,
>> Naush
>>
>>>
>>>
>>>>
>>>> JM
>>>>
>>>>>
>>>>> Thanks,
>>>>> Naush
>>>>>
>>>>> [1] https://github.com/raspberrypi/linux/pull/5067
>>>>>
>>>>> Naushir Patuck (3):
>>>>>     libcamera: controls: Add SensorTemperature control
>>>>>     ipa: raspberrypi: Add sensor temperature to DeviceStatus
>>>>>     ipa: raspberrypi: imx477: Get sensor temperature from embedded data
>>>>>
>>>>>    src/ipa/raspberrypi/cam_helper.cpp               |  7 ++++---
>>>>>    src/ipa/raspberrypi/cam_helper_imx477.cpp        |  5 ++++-
>>>>>    src/ipa/raspberrypi/controller/device_status.cpp |  3 +++
>>>>>    src/ipa/raspberrypi/controller/device_status.h   |  4 +++-
>>>>>    src/ipa/raspberrypi/raspberrypi.cpp              |  2 ++
>>>>>    src/libcamera/control_ids.yaml                   | 10 ++++++++++
>>>>>    6 files changed, 26 insertions(+), 5 deletions(-)
>>>>>