[libcamera-devel,v2,00/18]
mbox series

Message ID 20240113142218.28063-1-hdegoede@redhat.com
Headers show
Series
  • [libcamera-devel,v2,01/18] libcamera: pipeline: simple: fix size adjustment in validate()
Related show

Message

Hans de Goede Jan. 13, 2024, 2:22 p.m. UTC
Hi All,

Here is v2 of the patch-set to add Software ISP support
to libcamera / to the simple pipeline-handler.

Changes in v2 vs v1:
- Integrated Dennis, Martti and Toon's auto-exposure algorithm
  based on the paper which they found which gives us a nice
  relatively simple AEC + AGC algorithm
  TODO: Add link to paper to source + commit-message
- Integrated Dennis' doxygen comments for all new classes, no more warnings!
- Move AWB gain calculations to the IPA (Andrey)
- Added 8 bpp and 10 bpp unpacked raw bayer input support (Hans)
- Use dma-buf for the output FrameBuffer-s (Andrey)
- Memcpy data from input FrameBuffers to a heap buffer on a line by line
  basis to speedup debayering from uncached mem (Hans)
- This addresses all "open issues" from the v1 posting

Changes in v1 vs RFC-v2:
- Add and use SwStats[Cpu] and Debayer[Cpu] classes
- Rename linaro soft-IPA and soft-ISP implementations to simple following
  the simple pipeline hander
- Integrate Pavel's swstats and debayer improvements

This has been tested on:
- Qualcomm RB5 board with a mezzanine board equipped with RPi camera v2 (Andrey)
- Lenovo x13s, sc8280xp (Bryan)
- Pinephone (Pavel)
- Debix Model A (Milan)
- Lenovo Thinkpad Yoga X1 yoga gen7/8, Alder Lake/Raptor Lake IPU6EP +
  ov2740 10bpp packed + unpacked (Dennis / Hans)
- Dell Latitude 9420, Tiger Lake IPU6 + ov01a1s (RGBI) sensor
  (Martti, requires IGIG_GBGR_IGIG_GRGB patches on top)

git branch for this v2 patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v05

git branch for v1 of the patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v04

Old RFC-v2 cover-letter with small updates:

Here is an implementation of Software ISP and Software IPA
which provide debayering and implementation of image processing
algorithms for systems without a hardware ISP, or in cases when
there are no public drivers for the hardware ISP present in the
system.

The implementation of the Software ISP is a reference one.
A naive AWB alorithm is implemented as part of a function which
does debayering and statistics calculations - the algorithm part
is to be moved to the IPA in the next version of the patch set.
And for debayering itself there is already a more efficient
implementation by Hans de Goede. This patch set is currently using
the earlier debayering implementation as it is less lines of code
and is OK for the initial discussion.
Only RAW10P format from the sensor is currently supported, but
other debayering functions can be easily added (which of them to
call is decided based on the input format).

The Software IPA has only auto exposure and AGC. For the AGC
the analogue gain control of the camera sensor is used (if
available). The algorithm is very much simplified, and is
mostly included as a reference code.

The 6th patch renames some variables in the simple pipeline
handler for the Software ISP to use the same buffer handling
code as the Converter currently does. This lets one to
avoid adding extra code to the pipeline handler, but also
makes the Software ISP and the Converter mutually exclusive.

The Software ISP / IPA are intended to be used with the simple
pipeline handler. The pipeline handler doesn't interact with
the Software IPA directly - the Software IPA is hidden behind
the Software ISP interface. To use the Software ISP the build
must be configured with
  -Dpipelines=simple/simple -Dipas=simple/simple
and the Converter must not be used (the latter is hardcoded
in the simple pipeline handler).
If the build is configured with just
  -Dpipelines=simple
the Software ISP / IPA are not used, and the simple pipeline
handler works in the same way as without this patchset.

"simple" in the
  -Dpipelines=simple/simple -Dipas=simple/simple
is the name of the Software ISP / IPA implementation.
  -Dpipelines=simple/<something else> -Dipas=simple/<something else>
should work too if another implementation were added. What
implementation to use is the build time choice, only one
implementation is included into the build.

This patch set uses SharedMemObject class used by the RPi pipeline
handler - the second patch in the series moves the header file
to a common directory.

This patch set has been tested on Qualcomm RB5 board with
a mezzanine board equipped with RPi camera v2 (not the
standard RB5 camera mezzanine).

git branch for the RFC-v2 patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v03

RFC-v1 of the patch set:
https://patchwork.libcamera.org/cover/19262/

Changes in RFC v2 vs RFC v1:
- patches are restructured and reordered
- the Software IPA is hidden behind the Software ISP interface. The
  pipeline handler doesn't interact with the Software IPA directly
- instantiation of the Software ISP / IPA is configurable (at build
  time)
- comment explaining the implementation limitations is added to
  SwIspLinaro::IspWorker::debayerRaw10P()

Regards,

Hans



Andrey Konovalov (10):
  libcamera: pipeline: simple: fix size adjustment in validate()
  libcamera: internal: Move dma_heaps.[h,cpp] to common directories
  libcamera: dma_heaps: extend DmaHeap class to support system heap
  libcamera: internal: Move SharedMemObject class to a common directory
  libcamera: introduce SoftwareIsp class
  libcamera: ipa: add Soft IPA common files
  libcamera: ipa: Soft IPA: add a Simple Soft IPA implementation
  libcamera: software_isp: add Simple SoftwareIsp implementation
  libcamera: pipeline: simple: rename converterBuffers_ and related vars
  libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA

Dennis Bonke (1):
  libcamera: internal: Document the SharedMemObject class

Hans de Goede (7):
  libcamera: software_isp: Add SwStats base class
  libcamera: software_isp: Add SwStatsCpu class
  libcamera: software_isp: Add Debayer base class
  libcamera: software_isp: Add DebayerCpu class
  libcamera: swstats_cpu: Add support for 8 and 10 bpp unpacked bayer
    input
  libcamera: debayer_cpu: Add support for 8 and 10 bpp unpacked bayer
    input
  libcamera: debayer_cpu: Add BGR888 output support

 Documentation/Doxyfile.in                     |   1 +
 .../libcamera/internal}/dma_heaps.h           |  14 +-
 include/libcamera/internal/meson.build        |   4 +
 .../libcamera/internal}/shared_mem_object.h   |  57 +-
 include/libcamera/internal/software_isp.h     | 231 ++++++
 .../libcamera/internal/software_isp/debayer.h | 132 ++++
 .../internal/software_isp/debayer_cpu.h       | 141 ++++
 .../internal/software_isp/debayer_params.h    |  43 ++
 .../internal/software_isp/meson.build         |  11 +
 .../internal/software_isp/swisp_simple.h      | 163 +++++
 .../internal/software_isp/swisp_stats.h       |  34 +
 .../libcamera/internal/software_isp/swstats.h | 215 ++++++
 .../internal/software_isp/swstats_cpu.h       |  51 ++
 include/libcamera/ipa/meson.build             |   1 +
 include/libcamera/ipa/soft.mojom              |  29 +
 meson_options.txt                             |   3 +-
 src/ipa/simple/common/meson.build             |  17 +
 src/ipa/simple/common/soft_base.cpp           |  73 ++
 src/ipa/simple/common/soft_base.h             |  50 ++
 src/ipa/simple/meson.build                    |  12 +
 src/ipa/simple/simple/data/meson.build        |   9 +
 src/ipa/simple/simple/data/soft.conf          |   3 +
 src/ipa/simple/simple/meson.build             |  26 +
 src/ipa/simple/simple/soft_simple.cpp         | 273 ++++++++
 .../{pipeline/rpi/vc4 => }/dma_heaps.cpp      |  55 +-
 src/libcamera/meson.build                     |   3 +
 src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
 src/libcamera/pipeline/simple/simple.cpp      | 177 +++--
 src/libcamera/software_isp.cpp                |  62 ++
 src/libcamera/software_isp/debayer.cpp        |  22 +
 src/libcamera/software_isp/debayer_cpu.cpp    | 661 ++++++++++++++++++
 src/libcamera/software_isp/meson.build        |  27 +
 src/libcamera/software_isp/swisp_simple.cpp   | 238 +++++++
 src/libcamera/software_isp/swstats.cpp        |  22 +
 src/libcamera/software_isp/swstats_cpu.cpp    | 261 +++++++
 36 files changed, 3035 insertions(+), 92 deletions(-)
 rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
 rename {src/libcamera/pipeline/rpi/common => include/libcamera/internal}/shared_mem_object.h (62%)
 create mode 100644 include/libcamera/internal/software_isp.h
 create mode 100644 include/libcamera/internal/software_isp/debayer.h
 create mode 100644 include/libcamera/internal/software_isp/debayer_cpu.h
 create mode 100644 include/libcamera/internal/software_isp/debayer_params.h
 create mode 100644 include/libcamera/internal/software_isp/meson.build
 create mode 100644 include/libcamera/internal/software_isp/swisp_simple.h
 create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
 create mode 100644 include/libcamera/internal/software_isp/swstats.h
 create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h
 create mode 100644 include/libcamera/ipa/soft.mojom
 create mode 100644 src/ipa/simple/common/meson.build
 create mode 100644 src/ipa/simple/common/soft_base.cpp
 create mode 100644 src/ipa/simple/common/soft_base.h
 create mode 100644 src/ipa/simple/meson.build
 create mode 100644 src/ipa/simple/simple/data/meson.build
 create mode 100644 src/ipa/simple/simple/data/soft.conf
 create mode 100644 src/ipa/simple/simple/meson.build
 create mode 100644 src/ipa/simple/simple/soft_simple.cpp
 rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (54%)
 create mode 100644 src/libcamera/software_isp.cpp
 create mode 100644 src/libcamera/software_isp/debayer.cpp
 create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
 create mode 100644 src/libcamera/software_isp/meson.build
 create mode 100644 src/libcamera/software_isp/swisp_simple.cpp
 create mode 100644 src/libcamera/software_isp/swstats.cpp
 create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp

Comments

Hans de Goede Jan. 13, 2024, 2:35 p.m. UTC | #1
Hi All,

Ugh I forgot to fill in the subject part of the cover-letter, sorry.

I've fixed the subject in this reply, for any replies
to the cover-letter please use this subject.

Regards,

Hans



On 1/13/24 15:22, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of the patch-set to add Software ISP support
> to libcamera / to the simple pipeline-handler.
> 
> Changes in v2 vs v1:
> - Integrated Dennis, Martti and Toon's auto-exposure algorithm
>   based on the paper which they found which gives us a nice
>   relatively simple AEC + AGC algorithm
>   TODO: Add link to paper to source + commit-message
> - Integrated Dennis' doxygen comments for all new classes, no more warnings!
> - Move AWB gain calculations to the IPA (Andrey)
> - Added 8 bpp and 10 bpp unpacked raw bayer input support (Hans)
> - Use dma-buf for the output FrameBuffer-s (Andrey)
> - Memcpy data from input FrameBuffers to a heap buffer on a line by line
>   basis to speedup debayering from uncached mem (Hans)
> - This addresses all "open issues" from the v1 posting
> 
> Changes in v1 vs RFC-v2:
> - Add and use SwStats[Cpu] and Debayer[Cpu] classes
> - Rename linaro soft-IPA and soft-ISP implementations to simple following
>   the simple pipeline hander
> - Integrate Pavel's swstats and debayer improvements
> 
> This has been tested on:
> - Qualcomm RB5 board with a mezzanine board equipped with RPi camera v2 (Andrey)
> - Lenovo x13s, sc8280xp (Bryan)
> - Pinephone (Pavel)
> - Debix Model A (Milan)
> - Lenovo Thinkpad Yoga X1 yoga gen7/8, Alder Lake/Raptor Lake IPU6EP +
>   ov2740 10bpp packed + unpacked (Dennis / Hans)
> - Dell Latitude 9420, Tiger Lake IPU6 + ov01a1s (RGBI) sensor
>   (Martti, requires IGIG_GBGR_IGIG_GRGB patches on top)
> 
> git branch for this v2 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v05
> 
> git branch for v1 of the patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v04
> 
> Old RFC-v2 cover-letter with small updates:
> 
> Here is an implementation of Software ISP and Software IPA
> which provide debayering and implementation of image processing
> algorithms for systems without a hardware ISP, or in cases when
> there are no public drivers for the hardware ISP present in the
> system.
> 
> The implementation of the Software ISP is a reference one.
> A naive AWB alorithm is implemented as part of a function which
> does debayering and statistics calculations - the algorithm part
> is to be moved to the IPA in the next version of the patch set.
> And for debayering itself there is already a more efficient
> implementation by Hans de Goede. This patch set is currently using
> the earlier debayering implementation as it is less lines of code
> and is OK for the initial discussion.
> Only RAW10P format from the sensor is currently supported, but
> other debayering functions can be easily added (which of them to
> call is decided based on the input format).
> 
> The Software IPA has only auto exposure and AGC. For the AGC
> the analogue gain control of the camera sensor is used (if
> available). The algorithm is very much simplified, and is
> mostly included as a reference code.
> 
> The 6th patch renames some variables in the simple pipeline
> handler for the Software ISP to use the same buffer handling
> code as the Converter currently does. This lets one to
> avoid adding extra code to the pipeline handler, but also
> makes the Software ISP and the Converter mutually exclusive.
> 
> The Software ISP / IPA are intended to be used with the simple
> pipeline handler. The pipeline handler doesn't interact with
> the Software IPA directly - the Software IPA is hidden behind
> the Software ISP interface. To use the Software ISP the build
> must be configured with
>   -Dpipelines=simple/simple -Dipas=simple/simple
> and the Converter must not be used (the latter is hardcoded
> in the simple pipeline handler).
> If the build is configured with just
>   -Dpipelines=simple
> the Software ISP / IPA are not used, and the simple pipeline
> handler works in the same way as without this patchset.
> 
> "simple" in the
>   -Dpipelines=simple/simple -Dipas=simple/simple
> is the name of the Software ISP / IPA implementation.
>   -Dpipelines=simple/<something else> -Dipas=simple/<something else>
> should work too if another implementation were added. What
> implementation to use is the build time choice, only one
> implementation is included into the build.
> 
> This patch set uses SharedMemObject class used by the RPi pipeline
> handler - the second patch in the series moves the header file
> to a common directory.
> 
> This patch set has been tested on Qualcomm RB5 board with
> a mezzanine board equipped with RPi camera v2 (not the
> standard RB5 camera mezzanine).
> 
> git branch for the RFC-v2 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v03
> 
> RFC-v1 of the patch set:
> https://patchwork.libcamera.org/cover/19262/
> 
> Changes in RFC v2 vs RFC v1:
> - patches are restructured and reordered
> - the Software IPA is hidden behind the Software ISP interface. The
>   pipeline handler doesn't interact with the Software IPA directly
> - instantiation of the Software ISP / IPA is configurable (at build
>   time)
> - comment explaining the implementation limitations is added to
>   SwIspLinaro::IspWorker::debayerRaw10P()
> 
> Regards,
> 
> Hans
> 
> 
> 
> Andrey Konovalov (10):
>   libcamera: pipeline: simple: fix size adjustment in validate()
>   libcamera: internal: Move dma_heaps.[h,cpp] to common directories
>   libcamera: dma_heaps: extend DmaHeap class to support system heap
>   libcamera: internal: Move SharedMemObject class to a common directory
>   libcamera: introduce SoftwareIsp class
>   libcamera: ipa: add Soft IPA common files
>   libcamera: ipa: Soft IPA: add a Simple Soft IPA implementation
>   libcamera: software_isp: add Simple SoftwareIsp implementation
>   libcamera: pipeline: simple: rename converterBuffers_ and related vars
>   libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA
> 
> Dennis Bonke (1):
>   libcamera: internal: Document the SharedMemObject class
> 
> Hans de Goede (7):
>   libcamera: software_isp: Add SwStats base class
>   libcamera: software_isp: Add SwStatsCpu class
>   libcamera: software_isp: Add Debayer base class
>   libcamera: software_isp: Add DebayerCpu class
>   libcamera: swstats_cpu: Add support for 8 and 10 bpp unpacked bayer
>     input
>   libcamera: debayer_cpu: Add support for 8 and 10 bpp unpacked bayer
>     input
>   libcamera: debayer_cpu: Add BGR888 output support
> 
>  Documentation/Doxyfile.in                     |   1 +
>  .../libcamera/internal}/dma_heaps.h           |  14 +-
>  include/libcamera/internal/meson.build        |   4 +
>  .../libcamera/internal}/shared_mem_object.h   |  57 +-
>  include/libcamera/internal/software_isp.h     | 231 ++++++
>  .../libcamera/internal/software_isp/debayer.h | 132 ++++
>  .../internal/software_isp/debayer_cpu.h       | 141 ++++
>  .../internal/software_isp/debayer_params.h    |  43 ++
>  .../internal/software_isp/meson.build         |  11 +
>  .../internal/software_isp/swisp_simple.h      | 163 +++++
>  .../internal/software_isp/swisp_stats.h       |  34 +
>  .../libcamera/internal/software_isp/swstats.h | 215 ++++++
>  .../internal/software_isp/swstats_cpu.h       |  51 ++
>  include/libcamera/ipa/meson.build             |   1 +
>  include/libcamera/ipa/soft.mojom              |  29 +
>  meson_options.txt                             |   3 +-
>  src/ipa/simple/common/meson.build             |  17 +
>  src/ipa/simple/common/soft_base.cpp           |  73 ++
>  src/ipa/simple/common/soft_base.h             |  50 ++
>  src/ipa/simple/meson.build                    |  12 +
>  src/ipa/simple/simple/data/meson.build        |   9 +
>  src/ipa/simple/simple/data/soft.conf          |   3 +
>  src/ipa/simple/simple/meson.build             |  26 +
>  src/ipa/simple/simple/soft_simple.cpp         | 273 ++++++++
>  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp      |  55 +-
>  src/libcamera/meson.build                     |   3 +
>  src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
>  src/libcamera/pipeline/simple/simple.cpp      | 177 +++--
>  src/libcamera/software_isp.cpp                |  62 ++
>  src/libcamera/software_isp/debayer.cpp        |  22 +
>  src/libcamera/software_isp/debayer_cpu.cpp    | 661 ++++++++++++++++++
>  src/libcamera/software_isp/meson.build        |  27 +
>  src/libcamera/software_isp/swisp_simple.cpp   | 238 +++++++
>  src/libcamera/software_isp/swstats.cpp        |  22 +
>  src/libcamera/software_isp/swstats_cpu.cpp    | 261 +++++++
>  36 files changed, 3035 insertions(+), 92 deletions(-)
>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
>  rename {src/libcamera/pipeline/rpi/common => include/libcamera/internal}/shared_mem_object.h (62%)
>  create mode 100644 include/libcamera/internal/software_isp.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer_cpu.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer_params.h
>  create mode 100644 include/libcamera/internal/software_isp/meson.build
>  create mode 100644 include/libcamera/internal/software_isp/swisp_simple.h
>  create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
>  create mode 100644 include/libcamera/internal/software_isp/swstats.h
>  create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h
>  create mode 100644 include/libcamera/ipa/soft.mojom
>  create mode 100644 src/ipa/simple/common/meson.build
>  create mode 100644 src/ipa/simple/common/soft_base.cpp
>  create mode 100644 src/ipa/simple/common/soft_base.h
>  create mode 100644 src/ipa/simple/meson.build
>  create mode 100644 src/ipa/simple/simple/data/meson.build
>  create mode 100644 src/ipa/simple/simple/data/soft.conf
>  create mode 100644 src/ipa/simple/simple/meson.build
>  create mode 100644 src/ipa/simple/simple/soft_simple.cpp
>  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (54%)
>  create mode 100644 src/libcamera/software_isp.cpp
>  create mode 100644 src/libcamera/software_isp/debayer.cpp
>  create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
>  create mode 100644 src/libcamera/software_isp/meson.build
>  create mode 100644 src/libcamera/software_isp/swisp_simple.cpp
>  create mode 100644 src/libcamera/software_isp/swstats.cpp
>  create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
>
Hans de Goede Jan. 14, 2024, 4:19 p.m. UTC | #2
Hi,

On 1/13/24 15:22, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of the patch-set to add Software ISP support
> to libcamera / to the simple pipeline-handler.
> 
> Changes in v2 vs v1:
> - Integrated Dennis, Martti and Toon's auto-exposure algorithm
>   based on the paper which they found which gives us a nice
>   relatively simple AEC + AGC algorithm
>   TODO: Add link to paper to source + commit-message

For those interested the paper is here:
https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf

Regards,

Hans
Pavel Machek Jan. 14, 2024, 5:19 p.m. UTC | #3
Hi!

> Here is v2 of the patch-set to add Software ISP support
> to libcamera / to the simple pipeline-handler.

Thanks for the series. I had some minor comments; stuff I did not
comment on was okay, so you can add Reviewed-by: Pavel Machek
<pavel@ucw.cz>.

Best regards,
							Pavel
Hans de Goede Jan. 22, 2024, 4:32 p.m. UTC | #4
Hi All,

So during our last video conference about the Software ISP Laurent
requested some docs on how the performance of the Software ISP has
been measured so far.

I have written the following patch (to be included in v3) for this:
https://github.com/jwrdegoede/libcamera/commit/94377ed500d8bb031d312c0f88cfa5d0905fd222

And since reading documentation in patch format sucks I have also
just put the full restructured-text below.

If you have time please take a look and provide feedback before
v3 eventually gets posted with this included.

Regards,

Hans


.. SPDX-License-Identifier: CC-BY-SA-4.0

.. _software-isp-benchmarking:

Software ISP benchmarking
=========================

The Software ISP is paricular sensitive to performance regressions
therefor it is a good idea to always benchmark the Software ISP
before and after making changes to it and ensure that there are
no performance regressions.

DebayerCpu class builtin benchmark
----------------------------------

The DebayerCpu class has a builtin benchmark. This benchmark
measures the time spend on processing (collecting statistics
and debayering) only, it does not measure the time spend on
capturing or outputting the frames.

The builtin benchmark always runs. So this can be used by simply
running "cam" or "qcam" with a pipeline using the Software ISP.

When it runs it will skip measuring the first 30 frames to
allow the caches and the CPU temperature (turbo-ing) to warm-up
and then it measures 30 fps and shows the total and per frame
processing time using an info level log message:

.. code-block::

   INFO Debayer debayer_cpu.cpp:907 Processed 30 frames in 244317us, 8143 us/frame

To get stable measurements it is advised to disable any other processes which
may cause significant CPU usage (e.g. disable wifi, bluetooth and browsers).
When possible it is also advisable to disable CPU turbo-ing and
frequency-scaling.

For example when benchmarking on a Lenovo ThinkPad X1 Yoga Gen 8, with
the charger plugged in, the CPU can be fixed to run at 2 GHz using:

.. code-block::

   sudo x86_energy_perf_policy --turbo-enable 0
   sudo cpupower frequency-set -d 2GHz -u 2GHz

with these settings the builtin bench reports a processing time of ~7.8ms/frame
on this laptop for FHD SGRBG10 (unpacked) bayer data.

Measuring power consumption
---------------------------

Since the Software ISP is often used on mobile devices it is also
important to measure power consumption and ensure that that does
not regress.

For example to measure power consumption on a Lenovo ThinkPad X1 Yoga Gen 8
it needs to be running on battery and it should be configured with its
platform-profile (/sys/firmware/acpi/platform_profile) set to balanced and
with its default turbo and frequency-scaling behavior to match real world usage.

Then start qcam to capture a FHD picture at 30 fps and position the qcam window
so that it is fully visible. After this run the following command to monitor
the power consumption:

.. code-block::

   watch -n 10 cat /sys/class/power_supply/BAT0/power_now /sys/class/hwmon/hwmon6/fan?_input

Note this not only measures the power consumption in ųW it also monitors
the speed of this laptop's 2 fans. This is important because depending on
the ambient temperature the 2 fans may spin up while testing and this
will cause an additional power consumption of approx. 0.5W messing up
the measurement.

After starting qcam + the watch command let the laptop sit without using
it for 2 minutes for the readings to stabilize. Then check that the fans
have not turned on and manually take a couple of consecutive power readings
and avarage these.

On the example Lenovo ThinkPad X1 Yoga Gen 8 laptop this results in
a measured power consumption of approx. 13W while running qcam versus
approx. 4-5W while setting idle with its OLED panel on.
Milan Zamazal Jan. 23, 2024, 12:45 p.m. UTC | #5
Hans de Goede <hdegoede@redhat.com> writes:

> Hi All,
>
> So during our last video conference about the Software ISP Laurent
> requested some docs on how the performance of the Software ISP has
> been measured so far.
>
> I have written the following patch (to be included in v3) for this:
> https://github.com/jwrdegoede/libcamera/commit/94377ed500d8bb031d312c0f88cfa5d0905fd222
>
> And since reading documentation in patch format sucks I have also
> just put the full restructured-text below.
>
> If you have time please take a look and provide feedback before
> v3 eventually gets posted with this included.

Thank you for writing it down, a couple of comments below.

> Regards,
>
> Hans
>
>
> .. SPDX-License-Identifier: CC-BY-SA-4.0
>
> .. _software-isp-benchmarking:
>
> Software ISP benchmarking
> =========================
>
> The Software ISP is paricular sensitive to performance regressions
                      ^^^^^^^^^
> therefor it is a good idea to always benchmark the Software ISP
  ^^^^^^^^

It would be useful to run a spell checker on the document.

> before and after making changes to it and ensure that there are
> no performance regressions.
>
> DebayerCpu class builtin benchmark
> ----------------------------------
>
> The DebayerCpu class has a builtin benchmark. This benchmark
> measures the time spend on processing (collecting statistics
> and debayering) only, it does not measure the time spend on
                                                     ^^^^^

spent

> capturing or outputting the frames.
>
> The builtin benchmark always runs. So this can be used by simply
> running "cam" or "qcam" with a pipeline using the Software ISP.
>
> When it runs it will skip measuring the first 30 frames to
> allow the caches and the CPU temperature (turbo-ing) to warm-up
> and then it measures 30 fps and shows the total and per frame
> processing time using an info level log message:
>
> .. code-block::
>
>    INFO Debayer debayer_cpu.cpp:907 Processed 30 frames in 244317us, 8143 us/frame
>
> To get stable measurements it is advised to disable any other processes which
> may cause significant CPU usage (e.g. disable wifi, bluetooth and browsers).
> When possible it is also advisable to disable CPU turbo-ing and
> frequency-scaling.
>
> For example when benchmarking on a Lenovo ThinkPad X1 Yoga Gen 8, with
> the charger plugged in, the CPU can be fixed to run at 2 GHz using:
>
> .. code-block::
>
>    sudo x86_energy_perf_policy --turbo-enable 0
>    sudo cpupower frequency-set -d 2GHz -u 2GHz
>
> with these settings the builtin bench reports a processing time of ~7.8ms/frame
> on this laptop for FHD SGRBG10 (unpacked) bayer data.
>
> Measuring power consumption
> ---------------------------
>
> Since the Software ISP is often used on mobile devices it is also
> important to measure power consumption and ensure that that does
> not regress.
>
> For example to measure power consumption on a Lenovo ThinkPad X1 Yoga Gen 8
> it needs to be running on battery and it should be configured with its
> platform-profile (/sys/firmware/acpi/platform_profile) set to balanced and
> with its default turbo and frequency-scaling behavior to match real world usage.
>
> Then start qcam to capture a FHD picture at 30 fps and position the qcam window
> so that it is fully visible. After this run the following command to monitor
> the power consumption:
>
> .. code-block::
>
>    watch -n 10 cat /sys/class/power_supply/BAT0/power_now /sys/class/hwmon/hwmon6/fan?_input
>
> Note this not only measures the power consumption in ųW it also monitors
> the speed of this laptop's 2 fans. This is important because depending on
> the ambient temperature the 2 fans may spin up while testing and this
> will cause an additional power consumption of approx. 0.5W messing up
> the measurement.
>
> After starting qcam + the watch command let the laptop sit without using
> it for 2 minutes for the readings to stabilize. Then check that the fans
> have not turned on and manually take a couple of consecutive power readings
> and avarage these.

I think this kind of test can be influenced by many circumstances (besides those
already mentioned the current battery state and its interpretation by the
battery firmware).  Which means it's important to measure before and after
changes alternately several times and to see whether the measurements are
stable.  Maybe it was meant to be done this way but it's not clear from the
text.

Is using a watt meter an option or another measurable external source?  At least
such high values as those cited below should be reasonably measurable and it
would allow to make measurements on devices without battery (if it is any useful
there).  Of course, the same constraints and advice apply and it may not be
measurable automatically this way.

And doesn't modern hardware provide means to get such values?

(IIRC it has been discussed at one of the meetings but I don't remember what has
been said there; I'm sure others can provide better informed comments.)

> On the example Lenovo ThinkPad X1 Yoga Gen 8 laptop this results in
> a measured power consumption of approx. 13W while running qcam versus
> approx. 4-5W while setting idle with its OLED panel on.
Hans de Goede Feb. 8, 2024, 4:26 p.m. UTC | #6
Hi All,

Just a quick headsup. Unfortunately I have not been able to make
much time to work on preparing v3 of this series today, so I won't
be able to post a v3 today.

I plan to continue working on preparing v3 on Monday and I'll
hopefully post a v3 series upstream on Tuesday.

Regards,

Hans





On 1/13/24 15:22, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of the patch-set to add Software ISP support
> to libcamera / to the simple pipeline-handler.
> 
> Changes in v2 vs v1:
> - Integrated Dennis, Martti and Toon's auto-exposure algorithm
>   based on the paper which they found which gives us a nice
>   relatively simple AEC + AGC algorithm
>   TODO: Add link to paper to source + commit-message
> - Integrated Dennis' doxygen comments for all new classes, no more warnings!
> - Move AWB gain calculations to the IPA (Andrey)
> - Added 8 bpp and 10 bpp unpacked raw bayer input support (Hans)
> - Use dma-buf for the output FrameBuffer-s (Andrey)
> - Memcpy data from input FrameBuffers to a heap buffer on a line by line
>   basis to speedup debayering from uncached mem (Hans)
> - This addresses all "open issues" from the v1 posting
> 
> Changes in v1 vs RFC-v2:
> - Add and use SwStats[Cpu] and Debayer[Cpu] classes
> - Rename linaro soft-IPA and soft-ISP implementations to simple following
>   the simple pipeline hander
> - Integrate Pavel's swstats and debayer improvements
> 
> This has been tested on:
> - Qualcomm RB5 board with a mezzanine board equipped with RPi camera v2 (Andrey)
> - Lenovo x13s, sc8280xp (Bryan)
> - Pinephone (Pavel)
> - Debix Model A (Milan)
> - Lenovo Thinkpad Yoga X1 yoga gen7/8, Alder Lake/Raptor Lake IPU6EP +
>   ov2740 10bpp packed + unpacked (Dennis / Hans)
> - Dell Latitude 9420, Tiger Lake IPU6 + ov01a1s (RGBI) sensor
>   (Martti, requires IGIG_GBGR_IGIG_GRGB patches on top)
> 
> git branch for this v2 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v05
> 
> git branch for v1 of the patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v04
> 
> Old RFC-v2 cover-letter with small updates:
> 
> Here is an implementation of Software ISP and Software IPA
> which provide debayering and implementation of image processing
> algorithms for systems without a hardware ISP, or in cases when
> there are no public drivers for the hardware ISP present in the
> system.
> 
> The implementation of the Software ISP is a reference one.
> A naive AWB alorithm is implemented as part of a function which
> does debayering and statistics calculations - the algorithm part
> is to be moved to the IPA in the next version of the patch set.
> And for debayering itself there is already a more efficient
> implementation by Hans de Goede. This patch set is currently using
> the earlier debayering implementation as it is less lines of code
> and is OK for the initial discussion.
> Only RAW10P format from the sensor is currently supported, but
> other debayering functions can be easily added (which of them to
> call is decided based on the input format).
> 
> The Software IPA has only auto exposure and AGC. For the AGC
> the analogue gain control of the camera sensor is used (if
> available). The algorithm is very much simplified, and is
> mostly included as a reference code.
> 
> The 6th patch renames some variables in the simple pipeline
> handler for the Software ISP to use the same buffer handling
> code as the Converter currently does. This lets one to
> avoid adding extra code to the pipeline handler, but also
> makes the Software ISP and the Converter mutually exclusive.
> 
> The Software ISP / IPA are intended to be used with the simple
> pipeline handler. The pipeline handler doesn't interact with
> the Software IPA directly - the Software IPA is hidden behind
> the Software ISP interface. To use the Software ISP the build
> must be configured with
>   -Dpipelines=simple/simple -Dipas=simple/simple
> and the Converter must not be used (the latter is hardcoded
> in the simple pipeline handler).
> If the build is configured with just
>   -Dpipelines=simple
> the Software ISP / IPA are not used, and the simple pipeline
> handler works in the same way as without this patchset.
> 
> "simple" in the
>   -Dpipelines=simple/simple -Dipas=simple/simple
> is the name of the Software ISP / IPA implementation.
>   -Dpipelines=simple/<something else> -Dipas=simple/<something else>
> should work too if another implementation were added. What
> implementation to use is the build time choice, only one
> implementation is included into the build.
> 
> This patch set uses SharedMemObject class used by the RPi pipeline
> handler - the second patch in the series moves the header file
> to a common directory.
> 
> This patch set has been tested on Qualcomm RB5 board with
> a mezzanine board equipped with RPi camera v2 (not the
> standard RB5 camera mezzanine).
> 
> git branch for the RFC-v2 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v03
> 
> RFC-v1 of the patch set:
> https://patchwork.libcamera.org/cover/19262/
> 
> Changes in RFC v2 vs RFC v1:
> - patches are restructured and reordered
> - the Software IPA is hidden behind the Software ISP interface. The
>   pipeline handler doesn't interact with the Software IPA directly
> - instantiation of the Software ISP / IPA is configurable (at build
>   time)
> - comment explaining the implementation limitations is added to
>   SwIspLinaro::IspWorker::debayerRaw10P()
> 
> Regards,
> 
> Hans
> 
> 
> 
> Andrey Konovalov (10):
>   libcamera: pipeline: simple: fix size adjustment in validate()
>   libcamera: internal: Move dma_heaps.[h,cpp] to common directories
>   libcamera: dma_heaps: extend DmaHeap class to support system heap
>   libcamera: internal: Move SharedMemObject class to a common directory
>   libcamera: introduce SoftwareIsp class
>   libcamera: ipa: add Soft IPA common files
>   libcamera: ipa: Soft IPA: add a Simple Soft IPA implementation
>   libcamera: software_isp: add Simple SoftwareIsp implementation
>   libcamera: pipeline: simple: rename converterBuffers_ and related vars
>   libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA
> 
> Dennis Bonke (1):
>   libcamera: internal: Document the SharedMemObject class
> 
> Hans de Goede (7):
>   libcamera: software_isp: Add SwStats base class
>   libcamera: software_isp: Add SwStatsCpu class
>   libcamera: software_isp: Add Debayer base class
>   libcamera: software_isp: Add DebayerCpu class
>   libcamera: swstats_cpu: Add support for 8 and 10 bpp unpacked bayer
>     input
>   libcamera: debayer_cpu: Add support for 8 and 10 bpp unpacked bayer
>     input
>   libcamera: debayer_cpu: Add BGR888 output support
> 
>  Documentation/Doxyfile.in                     |   1 +
>  .../libcamera/internal}/dma_heaps.h           |  14 +-
>  include/libcamera/internal/meson.build        |   4 +
>  .../libcamera/internal}/shared_mem_object.h   |  57 +-
>  include/libcamera/internal/software_isp.h     | 231 ++++++
>  .../libcamera/internal/software_isp/debayer.h | 132 ++++
>  .../internal/software_isp/debayer_cpu.h       | 141 ++++
>  .../internal/software_isp/debayer_params.h    |  43 ++
>  .../internal/software_isp/meson.build         |  11 +
>  .../internal/software_isp/swisp_simple.h      | 163 +++++
>  .../internal/software_isp/swisp_stats.h       |  34 +
>  .../libcamera/internal/software_isp/swstats.h | 215 ++++++
>  .../internal/software_isp/swstats_cpu.h       |  51 ++
>  include/libcamera/ipa/meson.build             |   1 +
>  include/libcamera/ipa/soft.mojom              |  29 +
>  meson_options.txt                             |   3 +-
>  src/ipa/simple/common/meson.build             |  17 +
>  src/ipa/simple/common/soft_base.cpp           |  73 ++
>  src/ipa/simple/common/soft_base.h             |  50 ++
>  src/ipa/simple/meson.build                    |  12 +
>  src/ipa/simple/simple/data/meson.build        |   9 +
>  src/ipa/simple/simple/data/soft.conf          |   3 +
>  src/ipa/simple/simple/meson.build             |  26 +
>  src/ipa/simple/simple/soft_simple.cpp         | 273 ++++++++
>  .../{pipeline/rpi/vc4 => }/dma_heaps.cpp      |  55 +-
>  src/libcamera/meson.build                     |   3 +
>  src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
>  src/libcamera/pipeline/simple/simple.cpp      | 177 +++--
>  src/libcamera/software_isp.cpp                |  62 ++
>  src/libcamera/software_isp/debayer.cpp        |  22 +
>  src/libcamera/software_isp/debayer_cpu.cpp    | 661 ++++++++++++++++++
>  src/libcamera/software_isp/meson.build        |  27 +
>  src/libcamera/software_isp/swisp_simple.cpp   | 238 +++++++
>  src/libcamera/software_isp/swstats.cpp        |  22 +
>  src/libcamera/software_isp/swstats_cpu.cpp    | 261 +++++++
>  36 files changed, 3035 insertions(+), 92 deletions(-)
>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
>  rename {src/libcamera/pipeline/rpi/common => include/libcamera/internal}/shared_mem_object.h (62%)
>  create mode 100644 include/libcamera/internal/software_isp.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer_cpu.h
>  create mode 100644 include/libcamera/internal/software_isp/debayer_params.h
>  create mode 100644 include/libcamera/internal/software_isp/meson.build
>  create mode 100644 include/libcamera/internal/software_isp/swisp_simple.h
>  create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
>  create mode 100644 include/libcamera/internal/software_isp/swstats.h
>  create mode 100644 include/libcamera/internal/software_isp/swstats_cpu.h
>  create mode 100644 include/libcamera/ipa/soft.mojom
>  create mode 100644 src/ipa/simple/common/meson.build
>  create mode 100644 src/ipa/simple/common/soft_base.cpp
>  create mode 100644 src/ipa/simple/common/soft_base.h
>  create mode 100644 src/ipa/simple/meson.build
>  create mode 100644 src/ipa/simple/simple/data/meson.build
>  create mode 100644 src/ipa/simple/simple/data/soft.conf
>  create mode 100644 src/ipa/simple/simple/meson.build
>  create mode 100644 src/ipa/simple/simple/soft_simple.cpp
>  rename src/libcamera/{pipeline/rpi/vc4 => }/dma_heaps.cpp (54%)
>  create mode 100644 src/libcamera/software_isp.cpp
>  create mode 100644 src/libcamera/software_isp/debayer.cpp
>  create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
>  create mode 100644 src/libcamera/software_isp/meson.build
>  create mode 100644 src/libcamera/software_isp/swisp_simple.cpp
>  create mode 100644 src/libcamera/software_isp/swstats.cpp
>  create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
>