[PATCH v6 5/5] libcamera: software_isp: Remove TODO about internal representation

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Fri, May 31, 2024 at 02:38:40PM +0200, Milan Zamazal wrote:
> TODO #4 was recorded at a time where the IPA module computed gain values
> and the ISP computed the look up tables.  The gains were higher-level
> parameters.  Now that the look up tables are computed in the IPA module,
> the IPA and ISP are more tightly coupled and the TODO item is less
> relevant.
> 
> Let's drop the TODO item.  We may or may not need to switch to a
> different representation in future but there is currently no good need
> for this and the conversion of the values would be just waste of CPU
> cycles.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/libcamera/software_isp/TODO | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO
> index 4fcee39b..6bdc5905 100644
> --- a/src/libcamera/software_isp/TODO
> +++ b/src/libcamera/software_isp/TODO
> @@ -72,19 +72,6 @@ stats in hardware, such as the i.MX7), but please keep it on your radar.
>  
>  ---
>  
> -4. Hide internal representation of gains from callers
> -
> -> struct DebayerParams {
> -> 	static constexpr unsigned int kGain10 = 256;
> -
> -Forcing the caller to deal with the internal representation of gains
> -isn't nice, especially given that it precludes implementing gains of
> -different precisions in different backend. Wouldn't it be better to pass
> -the values as floating point numbers, and convert them to the internal
> -representation in the implementation of process() before using them ?
> -
> ----
> -
>  5. Store ISP parameters in per-frame buffers
>  
>  > /**

-- 
Regards,

Laurent Pinchart