[libcamera-devel,v3,01/19] ipa: ipu3: Document IPAIPU3 class interface

14267 diff mbox series

The IPU3 IPA is maturing to a modular and extensible system capable of
handling specific algorithms for the processing blocks on the ImgU.

Provide a top-level class documentation to provide an overview of the
IPA, detailing what events are used and what algorithms are currently
supported, as well as the limitations currently imposed.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

Hi Jean-Michel,

Thank you for the patch.

On Fri, Oct 22, 2021 at 05:12:00PM +0200, Jean-Michel Hautbois wrote:
> The IPU3 IPA is maturing to a modular and extensible system capable of
> handling specific algorithms for the processing blocks on the ImgU.
> 
> Provide a top-level class documentation to provide an overview of the
> IPA, detailing what events are used and what algorithms are currently
> supported, as well as the limitations currently imposed.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 65d3fd2c..b5438ed1 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;
>  
>  namespace ipa::ipu3 {
>  
> +/**
> + * \brief The IPU3 IPA implementation
> + *
> + * The IPU3 Pipeline defines an IPU3-specific interface for communication
> + * between the PipelineHandler and the IPA module.
> + *
> + * We extend the IPAIPU3Interface to implement our algorithms and handle events
> + * from the IPU3 PipelineHandler to satisfy requests from the application.
> + *
> + * At initialisation time, a CameraSensorHelper is instantiated to support
> + * camera-specific calculations, while the default controls are computed, and
> + * the algorithms are constructed and placed in an ordered list.
> + *
> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into
> + * rectangular cells of pixels. When the IPA is configured, we determine the
> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler
> + * output size.
> + *
> + * Two main events are then handled to operate the IPU3 ImgU by populating its
> + * parameter buffer, and adapting the settings of the sensor attached to the
> + * IPU3 CIO2 through sensor-specific V4L2 controls.
> + *
> + * When the event \a EventFillParams occurs we populate the ImgU parameter
> + * buffer with settings to configure the device in preparation for handling the
> + * frame queued in the Request.
> + *
> + * When the frame has completed processing, the ImgU will generate a statistics
> + * buffer which is given to the IPA as part of the \a EventStatReady event. At
> + * this event we run the algorithms to parse the statistics and cache any
> + * results for the next \a EventFillParams event.
> + *
> + * The individual algorithms are split into modular components that are called
> + * iteratively to allow them to process statistics from the ImgU in a defined
> + * order.
> + *
> + * The current implementation supports three core algorithms:
> + * - Automatic white balance (AWB)
> + * - Automatic gain and exposure control (AGC)
> + * - Tone mapping (Gamma)
> + *
> + * AWB is implemented using a Greyworld algorithm, and calculates the red and
> + * blue gains to apply to generate a neutral grey frame overall.
> + *
> + * AGC is handled by calculating a histogram of the green channel to estimate an
> + * analogue gain and shutter time which will provide a well exposed frame. A
> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce
> + * perceivable steps.
> + *
> + * The Tone mapping algorithm provides a gamma correction table to improve the
> + * contrast of the scene.
> + *
> + * The IPU3 ImgU has further processing blocks to support image quality
> + * improvements through bayer and temporal noise reductions, however those are
> + * not supported in the current implementation, and will use default settings as
> + * provided by the kernel driver.
> + *
> + * Demosaicing is operating with the default parameters and could be further
> + * optimised to provide improved sharpening coefficients, checker artifact
> + * removal, and false color correction.
> + *
> + * Additional image enhancements can be made by providing lens and
> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens
> + * shading correction (SHD) and Color correction (CCM).
> + */
>  class IPAIPU3 : public IPAIPU3Interface
>  {
>  public:

Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)
> The IPU3 IPA is maturing to a modular and extensible system capable of
> handling specific algorithms for the processing blocks on the ImgU.
> 
> Provide a top-level class documentation to provide an overview of the
> IPA, detailing what events are used and what algorithms are currently
> supported, as well as the limitations currently imposed.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 65d3fd2c..b5438ed1 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;
>  
>  namespace ipa::ipu3 {
>  
> +/**
> + * \brief The IPU3 IPA implementation
> + *
> + * The IPU3 Pipeline defines an IPU3-specific interface for communication
> + * between the PipelineHandler and the IPA module.
> + *
> + * We extend the IPAIPU3Interface to implement our algorithms and handle events
> + * from the IPU3 PipelineHandler to satisfy requests from the application.
> + *
> + * At initialisation time, a CameraSensorHelper is instantiated to support
> + * camera-specific calculations, while the default controls are computed, and
> + * the algorithms are constructed and placed in an ordered list.
> + *
> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into
> + * rectangular cells of pixels. When the IPA is configured, we determine the
> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler
> + * output size.
> + *
> + * Two main events are then handled to operate the IPU3 ImgU by populating its
> + * parameter buffer, and adapting the settings of the sensor attached to the
> + * IPU3 CIO2 through sensor-specific V4L2 controls.
> + *
> + * When the event \a EventFillParams occurs we populate the ImgU parameter
> + * buffer with settings to configure the device in preparation for handling the
> + * frame queued in the Request.
> + *
> + * When the frame has completed processing, the ImgU will generate a statistics
> + * buffer which is given to the IPA as part of the \a EventStatReady event. At
> + * this event we run the algorithms to parse the statistics and cache any
> + * results for the next \a EventFillParams event.
> + *
> + * The individual algorithms are split into modular components that are called
> + * iteratively to allow them to process statistics from the ImgU in a defined
> + * order.
> + *
> + * The current implementation supports three core algorithms:
> + * - Automatic white balance (AWB)
> + * - Automatic gain and exposure control (AGC)
> + * - Tone mapping (Gamma)

Has this been extended with BLC now? Or is it not worth mentioning that
explicitly?

> + *
> + * AWB is implemented using a Greyworld algorithm, and calculates the red and
> + * blue gains to apply to generate a neutral grey frame overall.
> + *
> + * AGC is handled by calculating a histogram of the green channel to estimate an
> + * analogue gain and shutter time which will provide a well exposed frame. A
> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce
> + * perceivable steps.
> + *
> + * The Tone mapping algorithm provides a gamma correction table to improve the
> + * contrast of the scene.
> + *
> + * The IPU3 ImgU has further processing blocks to support image quality
> + * improvements through bayer and temporal noise reductions, however those are
> + * not supported in the current implementation, and will use default settings as
> + * provided by the kernel driver.
> + *
> + * Demosaicing is operating with the default parameters and could be further
> + * optimised to provide improved sharpening coefficients, checker artifact
> + * removal, and false color correction.
> + *
> + * Additional image enhancements can be made by providing lens and
> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens

And perhaps check if BLC needs removing from here. If BLC still needs
more wrok, then perhaps it should stay.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> + * shading correction (SHD) and Color correction (CCM).
> + */
>  class IPAIPU3 : public IPAIPU3Interface
>  {
>  public:
> -- 
> 2.32.0
>

Hi Kieran, question for Laurent below ;-)

On 25/10/2021 22:07, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)
>> The IPU3 IPA is maturing to a modular and extensible system capable of
>> handling specific algorithms for the processing blocks on the ImgU.
>>
>> Provide a top-level class documentation to provide an overview of the
>> IPA, detailing what events are used and what algorithms are currently
>> supported, as well as the limitations currently imposed.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>   src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index 65d3fd2c..b5438ed1 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;
>>   
>>   namespace ipa::ipu3 {
>>   
>> +/**
>> + * \brief The IPU3 IPA implementation
>> + *
>> + * The IPU3 Pipeline defines an IPU3-specific interface for communication
>> + * between the PipelineHandler and the IPA module.
>> + *
>> + * We extend the IPAIPU3Interface to implement our algorithms and handle events
>> + * from the IPU3 PipelineHandler to satisfy requests from the application.
>> + *
>> + * At initialisation time, a CameraSensorHelper is instantiated to support
>> + * camera-specific calculations, while the default controls are computed, and
>> + * the algorithms are constructed and placed in an ordered list.
>> + *
>> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into
>> + * rectangular cells of pixels. When the IPA is configured, we determine the
>> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler
>> + * output size.
>> + *
>> + * Two main events are then handled to operate the IPU3 ImgU by populating its
>> + * parameter buffer, and adapting the settings of the sensor attached to the
>> + * IPU3 CIO2 through sensor-specific V4L2 controls.
>> + *
>> + * When the event \a EventFillParams occurs we populate the ImgU parameter
>> + * buffer with settings to configure the device in preparation for handling the
>> + * frame queued in the Request.
>> + *
>> + * When the frame has completed processing, the ImgU will generate a statistics
>> + * buffer which is given to the IPA as part of the \a EventStatReady event. At
>> + * this event we run the algorithms to parse the statistics and cache any
>> + * results for the next \a EventFillParams event.
>> + *
>> + * The individual algorithms are split into modular components that are called
>> + * iteratively to allow them to process statistics from the ImgU in a defined
>> + * order.
>> + *
>> + * The current implementation supports three core algorithms:
>> + * - Automatic white balance (AWB)
>> + * - Automatic gain and exposure control (AGC)
>> + * - Tone mapping (Gamma)
> 
> Has this been extended with BLC now? Or is it not worth mentioning that
> explicitly?

Well, according to me, it is not mature enough to be considered 
supported (but neither is tone mapping, ahah :-)).

> 
>> + *
>> + * AWB is implemented using a Greyworld algorithm, and calculates the red and
>> + * blue gains to apply to generate a neutral grey frame overall.
>> + *
>> + * AGC is handled by calculating a histogram of the green channel to estimate an
>> + * analogue gain and shutter time which will provide a well exposed frame. A
>> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce
>> + * perceivable steps.
>> + *
>> + * The Tone mapping algorithm provides a gamma correction table to improve the
>> + * contrast of the scene.
>> + *
>> + * The IPU3 ImgU has further processing blocks to support image quality
>> + * improvements through bayer and temporal noise reductions, however those are
>> + * not supported in the current implementation, and will use default settings as
>> + * provided by the kernel driver.
>> + *
>> + * Demosaicing is operating with the default parameters and could be further
>> + * optimised to provide improved sharpening coefficients, checker artifact
>> + * removal, and false color correction.
>> + *
>> + * Additional image enhancements can be made by providing lens and
>> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens
> 
> And perhaps check if BLC needs removing from here. If BLC still needs
> more wrok, then perhaps it should stay.

Laurent, what do you think, should I add it now or later on top ?

> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
>> + * shading correction (SHD) and Color correction (CCM).
>> + */
>>   class IPAIPU3 : public IPAIPU3Interface
>>   {
>>   public:
>> -- 
>> 2.32.0
>>

Hello JM,

Thank you for the patch.

On 10/22/21 8:42 PM, Jean-Michel Hautbois wrote:
> The IPU3 IPA is maturing to a modular and extensible system capable of
> handling specific algorithms for the processing blocks on the ImgU.
>
> Provide a top-level class documentation to provide an overview of the
> IPA, detailing what events are used and what algorithms are currently
> supported, as well as the limitations currently imposed.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 65d3fd2c..b5438ed1 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;
>   
>   namespace ipa::ipu3 {
>   
> +/**
> + * \brief The IPU3 IPA implementation
> + *
> + * The IPU3 Pipeline defines an IPU3-specific interface for communication
> + * between the PipelineHandler and the IPA module.
> + *
> + * We extend the IPAIPU3Interface to implement our algorithms and handle events
> + * from the IPU3 PipelineHandler to satisfy requests from the application.
> + *
> + * At initialisation time, a CameraSensorHelper is instantiated to support
> + * camera-specific calculations, while the default controls are computed, and
> + * the algorithms are constructed and placed in an ordered list.
> + *
> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into
> + * rectangular cells of pixels. When the IPA is configured, we determine the
> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler
> + * output size.
> + *
> + * Two main events are then handled to operate the IPU3 ImgU by populating its
> + * parameter buffer, and adapting the settings of the sensor attached to the
> + * IPU3 CIO2 through sensor-specific V4L2 controls.
> + *
> + * When the event \a EventFillParams occurs we populate the ImgU parameter
> + * buffer with settings to configure the device in preparation for handling the
> + * frame queued in the Request.
> + *
> + * When the frame has completed processing, the ImgU will generate a statistics
> + * buffer which is given to the IPA as part of the \a EventStatReady event. At
> + * this event we run the algorithms to parse the statistics and cache any
> + * results for the next \a EventFillParams event.
> + *
> + * The individual algorithms are split into modular components that are called
> + * iteratively to allow them to process statistics from the ImgU in a defined
> + * order.
> + *
> + * The current implementation supports three core algorithms:
> + * - Automatic white balance (AWB)
> + * - Automatic gain and exposure control (AGC)
> + * - Tone mapping (Gamma)
> + *
> + * AWB is implemented using a Greyworld algorithm, and calculates the red and
> + * blue gains to apply to generate a neutral grey frame overall.
> + *
> + * AGC is handled by calculating a histogram of the green channel to estimate an
> + * analogue gain and shutter time which will provide a well exposed frame. A
> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce
> + * perceivable steps.
> + *
> + * The Tone mapping algorithm provides a gamma correction table to improve the
> + * contrast of the scene.
> + *
> + * The IPU3 ImgU has further processing blocks to support image quality
> + * improvements through bayer and temporal noise reductions, however those are
> + * not supported in the current implementation, and will use default settings as
> + * provided by the kernel driver.
> + *
> + * Demosaicing is operating with the default parameters and could be further
> + * optimised to provide improved sharpening coefficients, checker artifact
> + * removal, and false color correction.
> + *
> + * Additional image enhancements can be made by providing lens and
> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens
> + * shading correction (SHD) and Color correction (CCM).
> + */
>   class IPAIPU3 : public IPAIPU3Interface
>   {
>   public:

On Tue, Oct 26, 2021 at 07:34:50AM +0200, Jean-Michel Hautbois wrote:
> Hi Kieran, question for Laurent below ;-)
> 
> On 25/10/2021 22:07, Kieran Bingham wrote:
> > Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)
> >> The IPU3 IPA is maturing to a modular and extensible system capable of
> >> handling specific algorithms for the processing blocks on the ImgU.
> >>
> >> Provide a top-level class documentation to provide an overview of the
> >> IPA, detailing what events are used and what algorithms are currently
> >> supported, as well as the limitations currently imposed.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>   src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 64 insertions(+)
> >>
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index 65d3fd2c..b5438ed1 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;
> >>   
> >>   namespace ipa::ipu3 {
> >>   
> >> +/**
> >> + * \brief The IPU3 IPA implementation
> >> + *
> >> + * The IPU3 Pipeline defines an IPU3-specific interface for communication
> >> + * between the PipelineHandler and the IPA module.
> >> + *
> >> + * We extend the IPAIPU3Interface to implement our algorithms and handle events
> >> + * from the IPU3 PipelineHandler to satisfy requests from the application.
> >> + *
> >> + * At initialisation time, a CameraSensorHelper is instantiated to support
> >> + * camera-specific calculations, while the default controls are computed, and
> >> + * the algorithms are constructed and placed in an ordered list.
> >> + *
> >> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into
> >> + * rectangular cells of pixels. When the IPA is configured, we determine the
> >> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler
> >> + * output size.
> >> + *
> >> + * Two main events are then handled to operate the IPU3 ImgU by populating its
> >> + * parameter buffer, and adapting the settings of the sensor attached to the
> >> + * IPU3 CIO2 through sensor-specific V4L2 controls.
> >> + *
> >> + * When the event \a EventFillParams occurs we populate the ImgU parameter
> >> + * buffer with settings to configure the device in preparation for handling the
> >> + * frame queued in the Request.
> >> + *
> >> + * When the frame has completed processing, the ImgU will generate a statistics
> >> + * buffer which is given to the IPA as part of the \a EventStatReady event. At
> >> + * this event we run the algorithms to parse the statistics and cache any
> >> + * results for the next \a EventFillParams event.
> >> + *
> >> + * The individual algorithms are split into modular components that are called
> >> + * iteratively to allow them to process statistics from the ImgU in a defined
> >> + * order.
> >> + *
> >> + * The current implementation supports three core algorithms:
> >> + * - Automatic white balance (AWB)
> >> + * - Automatic gain and exposure control (AGC)
> >> + * - Tone mapping (Gamma)
> > 
> > Has this been extended with BLC now? Or is it not worth mentioning that
> > explicitly?
> 
> Well, according to me, it is not mature enough to be considered 
> supported (but neither is tone mapping, ahah :-)).

The implementation is really a skeleton, but it's there, so I'd list it
here as it's not really worse than the tone mapping.

> >> + *
> >> + * AWB is implemented using a Greyworld algorithm, and calculates the red and
> >> + * blue gains to apply to generate a neutral grey frame overall.
> >> + *
> >> + * AGC is handled by calculating a histogram of the green channel to estimate an
> >> + * analogue gain and shutter time which will provide a well exposed frame. A
> >> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce
> >> + * perceivable steps.
> >> + *
> >> + * The Tone mapping algorithm provides a gamma correction table to improve the

As a general rule, we need to capitalize class names to get doxygen to
generate links (this is also why capitalized class names must not be in
plural form, Framebuffers or Cameras won't generate a link to the
corresponding class). When the name of a class is used as a normal word,
it should not be capitalized (in particular here, "tone mapping" with a
space isn't the name of the class). Specific terms are also usually
capitalized the first time they are introduced in a text, and they can
be capitalized when we want to emphasize that they are terms with a
specific meaning in the context, not just English words.

Bottom line, here it should be either "tone mapping" or "Tone Mapping".

> >> + * contrast of the scene.

 * The black level compensation algorithm subtracts a hardcoded black level from
 * all pixels.

(depending on how you capitalize tone mapping, this may need to be Black
Level Compensation)

> >> + *
> >> + * The IPU3 ImgU has further processing blocks to support image quality
> >> + * improvements through bayer and temporal noise reductions, however those are
> >> + * not supported in the current implementation, and will use default settings as
> >> + * provided by the kernel driver.
> >> + *
> >> + * Demosaicing is operating with the default parameters and could be further
> >> + * optimised to provide improved sharpening coefficients, checker artifact
> >> + * removal, and false color correction.
> >> + *
> >> + * Additional image enhancements can be made by providing lens and
> >> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens
> > 
> > And perhaps check if BLC needs removing from here. If BLC still needs
> > more wrok, then perhaps it should stay.
> 
> Laurent, what do you think, should I add it now or later on top ?

I'd keep it in the list here, as we hardcode the value and need to move
it to camera tuning data.

> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > 
> >> + * shading correction (SHD) and Color correction (CCM).
> >> + */
> >>   class IPAIPU3 : public IPAIPU3Interface
> >>   {
> >>   public: