[libcamera-devel] [RFC PATCH 1/2] ipa: rkisp1: awb: add support for RGB means

[Quentin Schulz]

Hi Laurent,

On 7/17/22 23:50, Laurent Pinchart via libcamera-devel wrote:
> Hi Quention,
> 
> On Thu, May 12, 2022 at 10:42:43AM +0200, Quentin Schulz via libcamera-devel wrote:
>> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>>
>> RkISP actually supports two modes for color means, RGB and YCbCr. The
>> variables where the means are stored are identically named regardless of
>> the color means mode that's been selected.
>>
>> Since the gains are computed in RGB mode, a conversion needs to be done
>> when the mode is YCbCr, which is unnecessary when RGB mode is selected.
>>
>> This adds support for RGB means mode too, by checking at runtime which
>> mode is selected at a given time.
>>
>> Cc: Quentin Schulz <foss+libcamera at 0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>> ---
>>
>> To be decided if we want to keep supporting both modes or only one?
>> (Given that at the moment we harcode which mode is used at compile time,
>>   I'm not sure it's worth keeping dead code around)
> 
> I'm finally testing this, and this patch doesn't compile. params has no
> meas member.
> 

Yeah, I think someone reported it on #libcamera a few days after I 
posted it.

I got confused by the same variable name not being of the same type and 
tried to use it the same way.

I tested it back then by completely swapping YCbCr to RGB means without 
the check in here against meas_config, just assuming it's RGB (selected 
in patch 2/2). I clearly didn't compile it before sending the RFC after 
trying to do "smart" things instead of just sending what I had tested, 
sorry about that.

We could create a new variable in the awb algorithm to hold the color 
mode used to get mean values for the AWB algorithm by the rkisp? Or 
something like that. The plan for this RFC was more about checking if we 
still have an issue with RGB AWB not working on rkisp1 (at least rk3399, 
for some people?) and if so, maybe better document it and offer the 
ability to support both YCbCr and RGB means for AWB (e.g. have PX30 
rkisp1 v1.2 use RGB and RK3399 rkisp1 v1.0 use YCbCr?). If it works 
fine, then maybe we can remove the translation from YCbCr to RGB and 
simplify a bit the logic (and make it more precise too I assume, but 
considering it's still a bit on the green side, I'm not sure that is a 
valid argument :) ).

Cheers,
Quentin