[libcamera-devel,0/4] Add Demosaicing and Color Processing control for rkisp1
mbox series

Message ID 20220704152318.221213-1-fsylvestre@baylibre.com
Headers show
Series
  • Add Demosaicing and Color Processing control for rkisp1
Related show

Message

Florian Sylvestre July 4, 2022, 3:23 p.m. UTC 
Hello,

This patch series adds support for two rkisp1 algorithms:
        - Demosaicing algorithm (patch 3/4)
        - Color Processing algorithm (patch 4/4)
Demosaicing algorithm is in charge of denoising and sharpness control.
ColorProcessing algorithm is in charge of brightness, contrast and saturation
control.

Demosaicing and ColorProcessing algorithm are currently configurable from the
application by user controls.
So I add support for transfering user controls respectively in the algorithm
class (patch 1/4) and in the IPARkISP1 (patch 2/4).

The series is based on top of "[PATCH v4 12/12] ipa: rkisp1: Add support of
Black Level Correction tuning"

Florian Sylvestre (4):
  ipa: libipa: algorithm: Add queueRequest() to the Algorithm class
  ipa: rkisp1: Transfer queueRequest() call to each algorithm
  ipa: rkisp1: Add support of Demosaicing control
  ipa: rkisp1: Add support of ColorProcessing control

 src/ipa/ipu3/module.h                         |   2 +-
 src/ipa/libipa/algorithm.cpp                  |  13 ++
 src/ipa/libipa/algorithm.h                    |   6 +
 src/ipa/libipa/module.cpp                     |   5 +
 src/ipa/libipa/module.h                       |   3 +-
 .../rkisp1/algorithms/color_processing.cpp    | 101 +++++++++
 src/ipa/rkisp1/algorithms/color_processing.h  |  38 ++++
 src/ipa/rkisp1/algorithms/demosaicing.cpp     | 208 ++++++++++++++++++
 src/ipa/rkisp1/algorithms/demosaicing.h       |  37 ++++
 src/ipa/rkisp1/algorithms/meson.build         |   2 +
 src/ipa/rkisp1/data/ov5640.yaml               |   2 +
 src/ipa/rkisp1/module.h                       |   2 +-
 src/ipa/rkisp1/rkisp1.cpp                     |   7 +-
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  20 ++
 14 files changed, 441 insertions(+), 5 deletions(-)
 create mode 100644 src/ipa/rkisp1/algorithms/color_processing.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/color_processing.h
 create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.h

Comments

Laurent Pinchart July 15, 2022, 12:02 a.m. UTC | #1 
Hello,

CC'ing Naush and David.

On Mon, Jul 04, 2022 at 05:23:14PM +0200, Florian Sylvestre via libcamera-devel wrote:
> Hello,
> 
> This patch series adds support for two rkisp1 algorithms:
>         - Demosaicing algorithm (patch 3/4)
>         - Color Processing algorithm (patch 4/4)
> Demosaicing algorithm is in charge of denoising and sharpness control.
> ColorProcessing algorithm is in charge of brightness, contrast and saturation
> control.
> 
> Demosaicing and ColorProcessing algorithm are currently configurable from the
> application by user controls.
> So I add support for transfering user controls respectively in the algorithm
> class (patch 1/4) and in the IPARkISP1 (patch 2/4).

David, Naush, what do you think of this approach of dispatching the
whole ControlList to each algorithm, compared to the large switch-case
in IPARPi::queueRequest() that calls member functions specific to each
algorithm ?

> The series is based on top of "[PATCH v4 12/12] ipa: rkisp1: Add support of
> Black Level Correction tuning"
> 
> Florian Sylvestre (4):
>   ipa: libipa: algorithm: Add queueRequest() to the Algorithm class
>   ipa: rkisp1: Transfer queueRequest() call to each algorithm
>   ipa: rkisp1: Add support of Demosaicing control
>   ipa: rkisp1: Add support of ColorProcessing control
> 
>  src/ipa/ipu3/module.h                         |   2 +-
>  src/ipa/libipa/algorithm.cpp                  |  13 ++
>  src/ipa/libipa/algorithm.h                    |   6 +
>  src/ipa/libipa/module.cpp                     |   5 +
>  src/ipa/libipa/module.h                       |   3 +-
>  .../rkisp1/algorithms/color_processing.cpp    | 101 +++++++++
>  src/ipa/rkisp1/algorithms/color_processing.h  |  38 ++++
>  src/ipa/rkisp1/algorithms/demosaicing.cpp     | 208 ++++++++++++++++++
>  src/ipa/rkisp1/algorithms/demosaicing.h       |  37 ++++
>  src/ipa/rkisp1/algorithms/meson.build         |   2 +
>  src/ipa/rkisp1/data/ov5640.yaml               |   2 +
>  src/ipa/rkisp1/module.h                       |   2 +-
>  src/ipa/rkisp1/rkisp1.cpp                     |   7 +-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  20 ++
>  14 files changed, 441 insertions(+), 5 deletions(-)
>  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.h
>  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.h
Naushir Patuck July 15, 2022, 7:17 a.m. UTC | #2 
Hi everyone,

On Fri, 15 Jul 2022 at 01:03, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hello,
>
> CC'ing Naush and David.
>
> On Mon, Jul 04, 2022 at 05:23:14PM +0200, Florian Sylvestre via
> libcamera-devel wrote:
> > Hello,
> >
> > This patch series adds support for two rkisp1 algorithms:
> >         - Demosaicing algorithm (patch 3/4)
> >         - Color Processing algorithm (patch 4/4)
> > Demosaicing algorithm is in charge of denoising and sharpness control.
> > ColorProcessing algorithm is in charge of brightness, contrast and
> saturation
> > control.
> >
> > Demosaicing and ColorProcessing algorithm are currently configurable
> from the
> > application by user controls.
> > So I add support for transfering user controls respectively in the
> algorithm
> > class (patch 1/4) and in the IPARkISP1 (patch 2/4).
>
> David, Naush, what do you think of this approach of dispatching the
> whole ControlList to each algorithm, compared to the large switch-case
> in IPARPi::queueRequest() that calls member functions specific to each
> algorithm ?
>

I'm ok with this - in fact, it was always on my to-do list, but never got
round to it.
However, at least for the RPi IPA, we will still need a
IPARPi::queueRequest() to
deal with certain controls.  Off the top of my head, FrameDurations will
need to
be handled by the IPA as various bits of calculations in the IPA need to
know
the limits.  It would require more refactoring and I must gather some
courage to move
this outside the IPA. But otherwise, pretty much all other control handling
could
move into the algorithms.

Regards,
Naush


>
> > The series is based on top of "[PATCH v4 12/12] ipa: rkisp1: Add support
> of
> > Black Level Correction tuning"
> >
> > Florian Sylvestre (4):
> >   ipa: libipa: algorithm: Add queueRequest() to the Algorithm class
> >   ipa: rkisp1: Transfer queueRequest() call to each algorithm
> >   ipa: rkisp1: Add support of Demosaicing control
> >   ipa: rkisp1: Add support of ColorProcessing control
> >
> >  src/ipa/ipu3/module.h                         |   2 +-
> >  src/ipa/libipa/algorithm.cpp                  |  13 ++
> >  src/ipa/libipa/algorithm.h                    |   6 +
> >  src/ipa/libipa/module.cpp                     |   5 +
> >  src/ipa/libipa/module.h                       |   3 +-
> >  .../rkisp1/algorithms/color_processing.cpp    | 101 +++++++++
> >  src/ipa/rkisp1/algorithms/color_processing.h  |  38 ++++
> >  src/ipa/rkisp1/algorithms/demosaicing.cpp     | 208 ++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/demosaicing.h       |  37 ++++
> >  src/ipa/rkisp1/algorithms/meson.build         |   2 +
> >  src/ipa/rkisp1/data/ov5640.yaml               |   2 +
> >  src/ipa/rkisp1/module.h                       |   2 +-
> >  src/ipa/rkisp1/rkisp1.cpp                     |   7 +-
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  20 ++
> >  14 files changed, 441 insertions(+), 5 deletions(-)
> >  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.h
> >  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.h
>
> --
> Regards,
>
> Laurent Pinchart
>
Laurent Pinchart July 15, 2022, 9:59 a.m. UTC | #3 
Hi Naush,

On Fri, Jul 15, 2022 at 08:17:07AM +0100, Naushir Patuck wrote:
> On Fri, 15 Jul 2022 at 01:03, Laurent Pinchart wrote:
> > On Mon, Jul 04, 2022 at 05:23:14PM +0200, Florian Sylvestre via libcamera-devel wrote:
> > > Hello,
> > >
> > > This patch series adds support for two rkisp1 algorithms:
> > >         - Demosaicing algorithm (patch 3/4)
> > >         - Color Processing algorithm (patch 4/4)
> > > Demosaicing algorithm is in charge of denoising and sharpness control.
> > > ColorProcessing algorithm is in charge of brightness, contrast and saturation
> > > control.
> > >
> > > Demosaicing and ColorProcessing algorithm are currently configurable from the
> > > application by user controls.
> > > So I add support for transfering user controls respectively in the algorithm
> > > class (patch 1/4) and in the IPARkISP1 (patch 2/4).
> >
> > David, Naush, what do you think of this approach of dispatching the
> > whole ControlList to each algorithm, compared to the large switch-case
> > in IPARPi::queueRequest() that calls member functions specific to each
> > algorithm ?
> 
> I'm ok with this - in fact, it was always on my to-do list, but never got
> round to it.

I'm glad we think alike :-)

> However, at least for the RPi IPA, we will still need a IPARPi::queueRequest() to
> deal with certain controls.  Off the top of my head, FrameDurations will need to
> be handled by the IPA as various bits of calculations in the IPA need to know
> the limits.  It would require more refactoring and I must gather some courage to move
> this outside the IPA. But otherwise, pretty much all other control handling could
> move into the algorithms.

That's fine, the entry point will always be the queueRequest() function
of the IPA module, which will dispatch to algorithms but can also
perform additional centralized processing of some controls.

> > > The series is based on top of "[PATCH v4 12/12] ipa: rkisp1: Add support of
> > > Black Level Correction tuning"
> > >
> > > Florian Sylvestre (4):
> > >   ipa: libipa: algorithm: Add queueRequest() to the Algorithm class
> > >   ipa: rkisp1: Transfer queueRequest() call to each algorithm
> > >   ipa: rkisp1: Add support of Demosaicing control
> > >   ipa: rkisp1: Add support of ColorProcessing control
> > >
> > >  src/ipa/ipu3/module.h                         |   2 +-
> > >  src/ipa/libipa/algorithm.cpp                  |  13 ++
> > >  src/ipa/libipa/algorithm.h                    |   6 +
> > >  src/ipa/libipa/module.cpp                     |   5 +
> > >  src/ipa/libipa/module.h                       |   3 +-
> > >  .../rkisp1/algorithms/color_processing.cpp    | 101 +++++++++
> > >  src/ipa/rkisp1/algorithms/color_processing.h  |  38 ++++
> > >  src/ipa/rkisp1/algorithms/demosaicing.cpp     | 208 ++++++++++++++++++
> > >  src/ipa/rkisp1/algorithms/demosaicing.h       |  37 ++++
> > >  src/ipa/rkisp1/algorithms/meson.build         |   2 +
> > >  src/ipa/rkisp1/data/ov5640.yaml               |   2 +
> > >  src/ipa/rkisp1/module.h                       |   2 +-
> > >  src/ipa/rkisp1/rkisp1.cpp                     |   7 +-
> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  20 ++
> > >  14 files changed, 441 insertions(+), 5 deletions(-)
> > >  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.cpp
> > >  create mode 100644 src/ipa/rkisp1/algorithms/color_processing.h
> > >  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.cpp
> > >  create mode 100644 src/ipa/rkisp1/algorithms/demosaicing.h