| Message ID | 20240531123840.713364-6-mzamazal@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@redhat.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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 > > > /**
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 > /**
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@redhat.com> --- src/libcamera/software_isp/TODO | 13 ------------- 1 file changed, 13 deletions(-)