[PATCH v2 6/6] ipa: rkisp1: Move ov4689 and ov5640 black levels into sensor helpers

[Mikhail Rudenko]

Hi, Kieran!

On 2024-07-03 at 13:20 +01, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:

> Quoting Stefan Klug (2024-07-03 11:39:53)
>> Move black levels for tuning files that contained a BLC block into
>> the camera sensor helpers.
>>
>> ov4689.yaml had 66 at 12bit while the datasheet states 64 at 12bit. Use the
>> value from the datasheet (scaled to 16bit).
>
> I wonder who chose 66 here. Was it measured by someone perhaps?
>
> Git blame states "Mikhail Rudenko" (Added to Cc) Mikhail, any opinion on
> this change?

Back then I measured that number, not knowing that it's specified in the
datasheet. I think we should stick to the datasheet, so

Acked-by: Mikhail Rudenko <mike.rudenko at gmail.com>

> Anyway, I don't object to this right now - I think once we release our
> updates to the IPA and tuning tools, most sensors 'supported' in
> libcamera will likely benefit from being '(re)tuned' correctly and
> 'fully' (and consistently?).
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>> ov5640.yaml had 256 at 12bit while the datasheet states 16 at 10bit. Looking
>> at the commit message the 256 most likely stems from the imx219 tuning
>> file and 16 at 10bit is the same as the 64 at 12bit from the ov4689. This
>> seems more likely and is therefore used.
>>
>> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
>> ---
>>  src/ipa/libipa/camera_sensor_helper.cpp | 2 ++
>>  src/ipa/rkisp1/data/ov4689.yaml         | 4 ----
>>  src/ipa/rkisp1/data/ov5640.yaml         | 4 ----
>>  3 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
>> index 3d0e756927f0..982e35beaa90 100644
>> --- a/src/ipa/libipa/camera_sensor_helper.cpp
>> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
>> @@ -542,6 +542,7 @@ class CameraSensorHelperOv4689 : public CameraSensorHelper
>>  public:
>>         CameraSensorHelperOv4689()
>>         {
>> +               blackLevel_ = 1024;
>>                 gainType_ = AnalogueGainLinear;
>>                 gainConstants_.linear = { 1, 0, 0, 128 };
>>         }
>> @@ -553,6 +554,7 @@ class CameraSensorHelperOv5640 : public CameraSensorHelper
>>  public:
>>         CameraSensorHelperOv5640()
>>         {
>> +               blackLevel_ = 1024;
>>                 gainType_ = AnalogueGainLinear;
>>                 gainConstants_.linear = { 1, 0, 0, 16 };
>>         }
>> diff --git a/src/ipa/rkisp1/data/ov4689.yaml b/src/ipa/rkisp1/data/ov4689.yaml
>> index 2068684cafcd..609012967e02 100644
>> --- a/src/ipa/rkisp1/data/ov4689.yaml
>> +++ b/src/ipa/rkisp1/data/ov4689.yaml
>> @@ -6,8 +6,4 @@ algorithms:
>>    - Agc:
>>    - Awb:
>>    - BlackLevelCorrection:
>> -      R:  66
>> -      Gr: 66
>> -      Gb: 66
>> -      B:  66
>>  ...
>> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
>> index 897b83cb435b..4b21d412e44e 100644
>> --- a/src/ipa/rkisp1/data/ov5640.yaml
>> +++ b/src/ipa/rkisp1/data/ov5640.yaml
>> @@ -6,10 +6,6 @@ algorithms:
>>    - Agc:
>>    - Awb:
>>    - BlackLevelCorrection:
>> -      R:  256
>> -      Gr: 256
>> -      Gb: 256
>> -      B:  256
>>    - ColorProcessing:
>>    - GammaSensorLinearization:
>>        x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
>> --
>> 2.43.0
>>


--
Best regards,
Mikhail Rudenko