[libcamera-devel,v3,0/5] Multi-channel AGC
mbox series

Message ID 20230912102442.169001-1-david.plowman@raspberrypi.com
Headers show
Series
  • Multi-channel AGC
Related show

Message

David Plowman Sept. 12, 2023, 10:24 a.m. UTC
Hi everyone

Version 3 of this patch set addresses most of the points that Jacopo
raised in his review, so thank you very much for those!

Mostly I've done all the suggested things, especially the little
"tidy-ups", with a couple of exceptions.

Jacopo suggested moving the addition of the setActiveChannels function
into the first patch where it is used, though I've left it in the
previous patch where AGC channels are introduced. But I don't mind
moving it if folks would prefer that.

I've also not made any of the changes to code that I simply copied
from one file to another - probably best left for another day?

I think there were a couple of things where I couldn't really decide
what was better, but am happy to discuss those again too.

Thanks!

David

David Plowman (4):
  ipa: rpi: agc: Reorganise code for multi-channel AGC
  ipa: rpi: agc: Implementation of multi-channel AGC
  ipa: rpi: agc: Add AgcChannelConstraint class
  ipa: rpi: agc: Use channel constraints in the AGC algorithm

Naushir Patuck (1):
  ipa: rpi: histogram: Add interBinMean()

 src/ipa/rpi/common/ipa_base.cpp            |   20 +-
 src/ipa/rpi/controller/agc_algorithm.h     |   19 +-
 src/ipa/rpi/controller/agc_status.h        |    1 +
 src/ipa/rpi/controller/histogram.cpp       |   22 +-
 src/ipa/rpi/controller/histogram.h         |    2 +
 src/ipa/rpi/controller/meson.build         |    1 +
 src/ipa/rpi/controller/rpi/agc.cpp         |  972 ++++---------------
 src/ipa/rpi/controller/rpi/agc.h           |  125 +--
 src/ipa/rpi/controller/rpi/agc_channel.cpp | 1012 ++++++++++++++++++++
 src/ipa/rpi/controller/rpi/agc_channel.h   |  151 +++
 10 files changed, 1425 insertions(+), 900 deletions(-)
 create mode 100644 src/ipa/rpi/controller/rpi/agc_channel.cpp
 create mode 100644 src/ipa/rpi/controller/rpi/agc_channel.h

Comments

Jacopo Mondi Sept. 12, 2023, 1:57 p.m. UTC | #1
Hi David

On Tue, Sep 12, 2023 at 11:24:37AM +0100, David Plowman via libcamera-devel wrote:
> Hi everyone
>
> Version 3 of this patch set addresses most of the points that Jacopo
> raised in his review, so thank you very much for those!
>
> Mostly I've done all the suggested things, especially the little
> "tidy-ups", with a couple of exceptions.
>
> Jacopo suggested moving the addition of the setActiveChannels function
> into the first patch where it is used, though I've left it in the
> previous patch where AGC channels are introduced. But I don't mind
> moving it if folks would prefer that.
>
> I've also not made any of the changes to code that I simply copied
> from one file to another - probably best left for another day?

I'll reply here instead that on the previous version. For code that
has been copied around, I'm fine with the proposed approach of not
chaning it.

>
> I think there were a couple of things where I couldn't really decide
> what was better, but am happy to discuss those again too.

I'll go with another reivew round but you answers clarified most of my
questions and what's left undecided is mostly internal implementations
of your algorithms, so really up to you!

Thanks
   j

>
> Thanks!
>
> David
>
> David Plowman (4):
>   ipa: rpi: agc: Reorganise code for multi-channel AGC
>   ipa: rpi: agc: Implementation of multi-channel AGC
>   ipa: rpi: agc: Add AgcChannelConstraint class
>   ipa: rpi: agc: Use channel constraints in the AGC algorithm
>
> Naushir Patuck (1):
>   ipa: rpi: histogram: Add interBinMean()
>
>  src/ipa/rpi/common/ipa_base.cpp            |   20 +-
>  src/ipa/rpi/controller/agc_algorithm.h     |   19 +-
>  src/ipa/rpi/controller/agc_status.h        |    1 +
>  src/ipa/rpi/controller/histogram.cpp       |   22 +-
>  src/ipa/rpi/controller/histogram.h         |    2 +
>  src/ipa/rpi/controller/meson.build         |    1 +
>  src/ipa/rpi/controller/rpi/agc.cpp         |  972 ++++---------------
>  src/ipa/rpi/controller/rpi/agc.h           |  125 +--
>  src/ipa/rpi/controller/rpi/agc_channel.cpp | 1012 ++++++++++++++++++++
>  src/ipa/rpi/controller/rpi/agc_channel.h   |  151 +++
>  10 files changed, 1425 insertions(+), 900 deletions(-)
>  create mode 100644 src/ipa/rpi/controller/rpi/agc_channel.cpp
>  create mode 100644 src/ipa/rpi/controller/rpi/agc_channel.h
>
> --
> 2.30.2
>