[PATCH v3 0/4] ipa: Move Pwl from Raspberry Pi to libipa

[David Plowman]

Hi Kieran, Paul

Yes, I did actually have a look through and it seemed like a big no-op
for us functionally at least, so

Acked-by: David Plowman <david.plowman at raspberrypi.com>

I did have a couple of super-minor thoughts, but nothing that I think
warrants changing anything.

I wasn't convinced by changing the name "clip" to "clamp". C++
std::clamp clips (or clamps?) at both ends of the range, whereas I
think our "clip" is just the top. So making them the same didn't
really make anything clearer for me, and maybe I preferred "clip". But
I don't really mind.

There was also Naush's point from a while ago that we use our Pwl
implementation elsewhere so we might be wanting to keep the two in
sync. But it's not code we touch very much these days, so we can
probably manage anything that comes up. Maybe we'll just change our
other version to adopt any minor name changes (e.g. Point to PointF).

David

On Fri, 31 May 2024 at 08:54, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Naush, David,
>
> Do you have any objections or concerns here? If not could you provide an
> Acked-by: tag please?
>
> --
> Kieran
>
>
> Quoting Paul Elder (2024-05-29 20:26:07)
> > This patch series moves the piecewise linear function class from
> > the Rasberry Pi IPA to libipa so that all IPAs can use it.
> >
> > First an addition to the geometry header is needed, to add a
> > floating-point version of the Point class, then the pwl is copied over,
> > and the Raspberry Pi IPA is converted to use the libipa Pwl class.
> >
> > The main changes in v2 are s/FPoint/PointF/g and improving the
> > documentation.
> >
> > v3 has almost no change...
> >
> > Paul Elder (4):
> >   libcamera: geometry: Add floating-point version of Point class
> >   ipa: libipa: Copy pwl from rpi
> >   ipa: libipa: pwl: Clean up Pwl class to match libcamera
> >   ipa: rpi: controller: Use libipa's Pwl class
> >
> >  include/libcamera/geometry.h               |  65 ++++
> >  src/ipa/libipa/meson.build                 |   2 +
> >  src/ipa/libipa/pwl.cpp                     | 371 +++++++++++++++++++++
> >  src/ipa/libipa/pwl.h                       |  98 ++++++
> >  src/ipa/rpi/controller/cac_status.h        |   2 -
> >  src/ipa/rpi/controller/contrast_status.h   |   4 +-
> >  src/ipa/rpi/controller/meson.build         |   2 +-
> >  src/ipa/rpi/controller/rpi/af.cpp          |   4 +-
> >  src/ipa/rpi/controller/rpi/af.h            |   5 +-
> >  src/ipa/rpi/controller/rpi/agc_channel.cpp |   8 +-
> >  src/ipa/rpi/controller/rpi/agc_channel.h   |   7 +-
> >  src/ipa/rpi/controller/rpi/awb.cpp         |  40 +--
> >  src/ipa/rpi/controller/rpi/awb.h           |  23 +-
> >  src/ipa/rpi/controller/rpi/ccm.cpp         |   4 +-
> >  src/ipa/rpi/controller/rpi/ccm.h           |   5 +-
> >  src/ipa/rpi/controller/rpi/contrast.cpp    |  14 +-
> >  src/ipa/rpi/controller/rpi/contrast.h      |   5 +-
> >  src/ipa/rpi/controller/rpi/geq.cpp         |   5 +-
> >  src/ipa/rpi/controller/rpi/geq.h           |   4 +-
> >  src/ipa/rpi/controller/rpi/hdr.cpp         |   8 +-
> >  src/ipa/rpi/controller/rpi/hdr.h           |   9 +-
> >  src/ipa/rpi/controller/rpi/tonemap.cpp     |   2 +-
> >  src/ipa/rpi/controller/rpi/tonemap.h       |   5 +-
> >  src/ipa/rpi/controller/tonemap_status.h    |   4 +-
> >  src/libcamera/geometry.cpp                 | 123 ++++++-
> >  test/geometry.cpp                          | 355 ++++++++++++++++++++
> >  26 files changed, 1097 insertions(+), 77 deletions(-)
> >  create mode 100644 src/ipa/libipa/pwl.cpp
> >  create mode 100644 src/ipa/libipa/pwl.h
> >
> > --
> > 2.39.2
> >