[libcamera-devel] [PATCH 2/2] ipa: libipa: Add OV5695 Camera Sensor Helper

[Umang Jain]

On 3/1/23 8:33 PM, Kieran Bingham via libcamera-devel wrote:
> Quoting Kieran Bingham (2023-03-01 14:55:25)
>> Quoting Jacopo Mondi (2023-02-28 15:33:22)
>>> Hi Kieran
>>>
>>> On Mon, Feb 27, 2023 at 08:42:54PM +0000, Kieran Bingham via libcamera-devel wrote:
>>>> Provide a CameraSensorHelper for the OV5695, along with the
>>>> corresponding camera sensor properties.
>>>>
>>>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>>> ---
>>>>   src/ipa/libipa/camera_sensor_helper.cpp    | 11 +++++++++++
>>>>   src/libcamera/camera_sensor_properties.cpp |  8 ++++++++
>>>>   2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
>>>> index d1051cc25656..a38fefc75372 100644
>>>> --- a/src/ipa/libipa/camera_sensor_helper.cpp
>>>> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
>>>> @@ -527,6 +527,17 @@ public:
>>>>   };
>>>>   REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
>>>>
>>>> +class CameraSensorHelperOv5695 : public CameraSensorHelper
>>>> +{
>>>> +public:
>>>> +     CameraSensorHelperOv5695()
>>>> +     {
>>>> +             gainType_ = AnalogueGainLinear;
>>>> +             gainConstants_.linear = { 1, 0, 0, 128 };
>>>> +     }
>>>> +};
>>>> +REGISTER_CAMERA_SENSOR_HELPER("ov5695", CameraSensorHelperOv5695)
>>>> +
>>> I don't have a datasheet, but again, seeing this in action it seems
>>> reasonable
> Me neither. But I wonder if that will be most sensors we have to add
> here. I could also add:
>
> +               /* This has been validated with some empirical testing only. */
>
> But that might be applicable to 'most' of the CameraSensorHelpers?
> Do we need to have a way to mark which ones would benefit from extra
> validation? (Maybe a comment is enough ?)

This will help - it gives a notion that more empirical testing can be 
done(by users/community?)  and validated the information encapsulated 
in  Properties and helpers.  But the point is whether we want to put up 
a global banner comment or be specific with each entry ?


>
>
>>>>   class CameraSensorHelperOv8858 : public CameraSensorHelper
>>>>   {
>>>>   public:
>>>> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
>>>> index 7652c5f3e24c..a92c13b87b7f 100644
>>>> --- a/src/libcamera/camera_sensor_properties.cpp
>>>> +++ b/src/libcamera/camera_sensor_properties.cpp
>>>> @@ -204,6 +204,14 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>>>>                                 */
>>>>                        },
>>>>                } },
>>>> +             { "ov5695", {
>>>> +                     .unitCellSize = { 1400, 1400 },
>>> Can't validate this without a datasheet..
>> It's listed in the publicly available product brief:
>>
>>   pixel size: 1.4 μm x 1.4 μm
>>
>>
>>
>>>> +                     .testPatternModes = {
>>>> +                             { controls::draft::TestPatternModeOff, 0 },
>>>> +                             { controls::draft::TestPatternModeColorBars, 2 },
>>>> +                             { controls::draft::TestPatternModeColorBarsFadeToGray, 4},
>>>> +                     },
>>> The driver reports
>>>
>>> static const char * const ov5695_test_pattern_menu[] = {
>>>          "Disabled",
>>>          "Vertical Color Bar Type 1",
>>>          "Vertical Color Bar Type 2",
>>>          "Vertical Color Bar Type 3",
>>>          "Vertical Color Bar Type 4"
>>> };
>>>
>>> Have you inspected the produced test pattern ?
>>> If so
>> Yes, that was my best identifications from enabling the test patterns.
>> Annoyingly - the AGC/AWB quite quickly destroys the TPG patterns so you
>> have to be quick to see what's output. But that's a separate (and I
>> think known) issue...
>>
>>> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

same,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>>
>> Thanks
>>
>>
>>> Thanks
>>>    j
>>>
>>>> +             } },
>>>>                { "ov8858", {
>>>>                        .unitCellSize = { 1120, 1120 },
>>>>                        .testPatternModes = {
>>>> --
>>>> 2.34.1
>>>>