[v8,00/18] libcamera: introduce Software ISP and Software IPA
mbox series

Message ID 20240416091357.211951-1-mzamazal@redhat.com
Headers show
Series
  • libcamera: introduce Software ISP and Software IPA
Related show

Message

Milan Zamazal April 16, 2024, 9:13 a.m. UTC
Here is v8 of the patch-set to add Software ISP support
to libcamera / to the simple pipeline-handler.

Changes in v8 vs v7:
- Doc fixes in dma_heaps.cpp.
- SharedMemObject::size -> SharedMemObject::kSize
  (+ commit message expanded as suggested by Kieran).
- My commit messages limited to the line length of 72.
- All commit messages capitalized
  (for consistency both within the branch and with other libcamera
  commits).
- Using Span directly rather than defining SharedMem::span.
- Minor code changes in shared_mem_object.cpp as suggested by Laurent.
- Forgotten use of a type alias added in black_level.h
  (+ the type name capitalized).
- Rebased on current master.
- The dummy parameter dropped from setIspParams.

git branch for this v8 patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v11
CI pipeline:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1154685
(The debian10 failure looks unrelated.)

Changes in v7 vs v6:
Laurent's comments addressed, most notably:
* Formatting changes.
* Docstrings moved from *.h to *.cpp files.
* Improvements of docstrings.
* Miscellaneous refactorings without changing the code itself.
* Added src/libcamera/software_isp/TODO.
* Variables renamed:
  - DmaHeapFlag flag -> type
  - DmaHeapInfo.name -> deviceNodeName
  - generally renames to snakeCase where applicable
* DmaHeap no longer officially prefers CMA heap.
* SharedMem::mem_ is a Span now and private; size_ dropped.
* SharedMemObject template is limited to standard layout objects now.
* A more specific #include used in soft_simple.cpp.
* DebayerCpu::setDebayerFunctions no longer uses goto.
* The benchmarking document reformated to 80 columns.
* CameraSensorHelper patch integrated into preceding patches.

git branch for this v7 patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v10
CI pipeline:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1145462

Changes in v6 vs v5:
- [06/18] libcamera: software_isp: Add SwStatsCpu class
  Resolved a rebase conflict in meson.build.
- [16/18] libcamera: Add "Software ISP benchmarking"
  Fixed a trivial typo (Stefan).
- [17/18] libcamera: software_isp: Apply black level compensation
  A comment style adjusted (Kieran).
  A common value extracted into a variable.

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

Changes in v5 vs v4:
- CI fully passes now!
- Fix some small typos / using unnamed const values pointed
  out in Milan's reviews

Since there have been no significant review comments on v4
and since this fully passes CI now I believe that this version
is ready for merging.

git branch for this v5 patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v08-for-ci

Older changelogs and links to git branches:

Changes in v4 vs v3:
- Andrey:
  [05/18] "libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class"
  - documentation fixes
  [09/18] "libcamera: ipa: add Soft IPA"
  - Use int32_t for again*_ and exposure*_ as this matches
    the value the corresponding ControlValue::get() returns
  - Check for mmap() returning MAP_FAILED on error
  - Drop std::move() called on const SharedFD & argument
  - Replace #defines (EXPOSURE_OPTIMAL_VALUE etc) with const expressions
  - Use std::clamp() to keep exposure_ and again_ between the *_min_ and
    the *_max_
  - add comment on non-linear gain value vs gain code
  [10/18] "libcamera: introduce SoftwareIsp"
  - Fix the check of if sharedParams_ were created OK
  new [18/18] "libcamera: Soft IPA: use CameraSensorHelper for analogue gain"
  - One can check if his camera sensor has the CameraSensorHelper implemented with something
    like 'grep ^REGISTER_CAMERA_SENSOR_HELPER src/ipa/libipa/camera_sensor_helper.cpp'. If
    the camera sensor used has no CameraSensorHelper, the Soft IPA works as before this patch.
    This fallback option doesn't make the code nicer, and makes it harder to change the AE/AGC
    algorithm to work faster, but I decided to keep it for now.
- Milan:
  [17/18] "libcamera: software_isp: Apply black level compensation"
  - New patch adding black-level compensation autmatically detecting the
    sensor blacklevel
- Hans:
  - Small typo changes in various comments/docs
  - Replace typedef-s with using

Changes in v3 vs v2:
- The Software ISP now is always build when building the simple pipeline
  handler, no more -Dpipelines=simple/simple. Instead whether the SoftISP
  is used or not is based on the media-controller driver. For now it is
  only enabled for the "qcom-camss" driver (Andrey)
- SoftISP factory has been removed, there is a just a single SoftwareISP class now
- Fix the multi-threading issues (Andrey)
- Fully document the SharedMemObject and DmaHeap classes (Andrey)
- Drop SwStats base (integrate into SwStatsCpu class)
- Move headers for classes only used by the SoftISP into src/libcamera/software_isp
- Move SwStats / SwDebayer docs into .cpp and extend it
- Rename many foo_bar symbols to fooBar
- Add constexpr kFooBar values for varies hardcoded sizes like
  the yHistoGram having 16 bins and the gammalookuptable having 1024 entries
- Make startFrame() and finishFrame() normal methods instead of
  using function pointers for these
- Document how/why an array of src pointers is passed to
  the debayer functions
- 12bpp unpacked raw bayer input support (Kieran)
- Add "Software ISP benchmarking" doc

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 v4 of the patch set:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v07

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

git branch for v2 of the 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

Andrei Konovalov (1):
  libcamera: shared_mem_object: Reorganize the code and document the
    SharedMemObject class

Andrey Konovalov (8):
  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: ipa: Add Soft IPA
  libcamera: Introduce SoftwareIsp
  libcamera: pipeline: simple: Rename converterBuffers_ and related vars
  libcamera: pipeline: simple: Enable use of Soft ISP and Soft IPA

Hans de Goede (7):
  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, 10 and 12 bpp unpacked
    bayer input
  libcamera: debayer_cpu: Add support for 8, 10 and 12 bpp unpacked
    bayer input
  libcamera: debayer_cpu: Add BGR888 output support
  libcamera: Add "Software ISP benchmarking" documentation

Milan Zamazal (2):
  libcamera: shared_mem_object: Rename SIZE constant to `size'
  libcamera: software_isp: Apply black level compensation

 Documentation/Doxyfile.in                     |   1 +
 Documentation/index.rst                       |   1 +
 Documentation/meson.build                     |   1 +
 Documentation/software-isp-benchmarking.rst   |  77 ++
 .../libcamera/internal}/dma_heaps.h           |  14 +-
 include/libcamera/internal/meson.build        |   3 +
 .../libcamera/internal/shared_mem_object.h    | 127 +++
 .../internal/software_isp/debayer_params.h    |  29 +
 .../internal/software_isp/meson.build         |   7 +
 .../internal/software_isp/software_isp.h      |  99 +++
 .../internal/software_isp/swisp_stats.h       |  49 ++
 include/libcamera/ipa/meson.build             |   1 +
 include/libcamera/ipa/soft.mojom              |  28 +
 meson_options.txt                             |   2 +-
 src/ipa/meson.build                           |   3 +
 src/ipa/simple/black_level.cpp                |  88 ++
 src/ipa/simple/black_level.h                  |  28 +
 src/ipa/simple/data/meson.build               |  10 +
 src/ipa/simple/data/uncalibrated.yaml         |   5 +
 src/ipa/simple/meson.build                    |  30 +
 src/ipa/simple/soft_simple.cpp                | 403 +++++++++
 src/libcamera/dma_heaps.cpp                   | 165 ++++
 src/libcamera/meson.build                     |   3 +
 .../pipeline/rpi/common/shared_mem_object.h   | 128 ---
 src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp  |  90 --
 src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
 src/libcamera/pipeline/simple/simple.cpp      | 241 ++++--
 src/libcamera/shared_mem_object.cpp           | 236 +++++
 src/libcamera/software_isp/TODO               | 279 ++++++
 src/libcamera/software_isp/debayer.cpp        | 132 +++
 src/libcamera/software_isp/debayer.h          |  54 ++
 src/libcamera/software_isp/debayer_cpu.cpp    | 807 ++++++++++++++++++
 src/libcamera/software_isp/debayer_cpu.h      | 158 ++++
 src/libcamera/software_isp/meson.build        |  15 +
 src/libcamera/software_isp/software_isp.cpp   | 357 ++++++++
 src/libcamera/software_isp/swstats_cpu.cpp    | 432 ++++++++++
 src/libcamera/software_isp/swstats_cpu.h      |  97 +++
 38 files changed, 3922 insertions(+), 284 deletions(-)
 create mode 100644 Documentation/software-isp-benchmarking.rst
 rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
 create mode 100644 include/libcamera/internal/shared_mem_object.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/software_isp.h
 create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
 create mode 100644 include/libcamera/ipa/soft.mojom
 create mode 100644 src/ipa/simple/black_level.cpp
 create mode 100644 src/ipa/simple/black_level.h
 create mode 100644 src/ipa/simple/data/meson.build
 create mode 100644 src/ipa/simple/data/uncalibrated.yaml
 create mode 100644 src/ipa/simple/meson.build
 create mode 100644 src/ipa/simple/soft_simple.cpp
 create mode 100644 src/libcamera/dma_heaps.cpp
 delete mode 100644 src/libcamera/pipeline/rpi/common/shared_mem_object.h
 delete mode 100644 src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
 create mode 100644 src/libcamera/shared_mem_object.cpp
 create mode 100644 src/libcamera/software_isp/TODO
 create mode 100644 src/libcamera/software_isp/debayer.cpp
 create mode 100644 src/libcamera/software_isp/debayer.h
 create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
 create mode 100644 src/libcamera/software_isp/debayer_cpu.h
 create mode 100644 src/libcamera/software_isp/meson.build
 create mode 100644 src/libcamera/software_isp/software_isp.cpp
 create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
 create mode 100644 src/libcamera/software_isp/swstats_cpu.h

Comments

Kieran Bingham April 16, 2024, 12:55 p.m. UTC | #1
Hi Milan,

Quoting Milan Zamazal (2024-04-16 10:13:36)
> Here is v8 of the patch-set to add Software ISP support
> to libcamera / to the simple pipeline-handler.

Thank you for all this work updating here.

I've triggered the tests to run at
https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1155543.

There is one failure (don't panic) on debian:10. But I beleive that's
because Debian:10 apt repositories have moved to archive, and so we'll
need to update our container builds.

To compensate for this, I've also run through my old integration build
matrix which still passed.

As such I believe we're ready to merge this series \o/

And as this is what I was holding off for to tag libcamera-0.3 - I think
that means a release is imminent, except that I'm very overloaded and
don't have much time this week ;-)

--
Kieran


> 
> Changes in v8 vs v7:
> - Doc fixes in dma_heaps.cpp.
> - SharedMemObject::size -> SharedMemObject::kSize
>   (+ commit message expanded as suggested by Kieran).
> - My commit messages limited to the line length of 72.
> - All commit messages capitalized
>   (for consistency both within the branch and with other libcamera
>   commits).
> - Using Span directly rather than defining SharedMem::span.
> - Minor code changes in shared_mem_object.cpp as suggested by Laurent.
> - Forgotten use of a type alias added in black_level.h
>   (+ the type name capitalized).
> - Rebased on current master.
> - The dummy parameter dropped from setIspParams.
> 
> git branch for this v8 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v11
> CI pipeline:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1154685
> (The debian10 failure looks unrelated.)
> 
> Changes in v7 vs v6:
> Laurent's comments addressed, most notably:
> * Formatting changes.
> * Docstrings moved from *.h to *.cpp files.
> * Improvements of docstrings.
> * Miscellaneous refactorings without changing the code itself.
> * Added src/libcamera/software_isp/TODO.
> * Variables renamed:
>   - DmaHeapFlag flag -> type
>   - DmaHeapInfo.name -> deviceNodeName
>   - generally renames to snakeCase where applicable
> * DmaHeap no longer officially prefers CMA heap.
> * SharedMem::mem_ is a Span now and private; size_ dropped.
> * SharedMemObject template is limited to standard layout objects now.
> * A more specific #include used in soft_simple.cpp.
> * DebayerCpu::setDebayerFunctions no longer uses goto.
> * The benchmarking document reformated to 80 columns.
> * CameraSensorHelper patch integrated into preceding patches.
> 
> git branch for this v7 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v10
> CI pipeline:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1145462
> 
> Changes in v6 vs v5:
> - [06/18] libcamera: software_isp: Add SwStatsCpu class
>   Resolved a rebase conflict in meson.build.
> - [16/18] libcamera: Add "Software ISP benchmarking"
>   Fixed a trivial typo (Stefan).
> - [17/18] libcamera: software_isp: Apply black level compensation
>   A comment style adjusted (Kieran).
>   A common value extracted into a variable.
> 
> git branch for this v6 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v09
> 
> Changes in v5 vs v4:
> - CI fully passes now!
> - Fix some small typos / using unnamed const values pointed
>   out in Milan's reviews
> 
> Since there have been no significant review comments on v4
> and since this fully passes CI now I believe that this version
> is ready for merging.
> 
> git branch for this v5 patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v08-for-ci
> 
> Older changelogs and links to git branches:
> 
> Changes in v4 vs v3:
> - Andrey:
>   [05/18] "libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class"
>   - documentation fixes
>   [09/18] "libcamera: ipa: add Soft IPA"
>   - Use int32_t for again*_ and exposure*_ as this matches
>     the value the corresponding ControlValue::get() returns
>   - Check for mmap() returning MAP_FAILED on error
>   - Drop std::move() called on const SharedFD & argument
>   - Replace #defines (EXPOSURE_OPTIMAL_VALUE etc) with const expressions
>   - Use std::clamp() to keep exposure_ and again_ between the *_min_ and
>     the *_max_
>   - add comment on non-linear gain value vs gain code
>   [10/18] "libcamera: introduce SoftwareIsp"
>   - Fix the check of if sharedParams_ were created OK
>   new [18/18] "libcamera: Soft IPA: use CameraSensorHelper for analogue gain"
>   - One can check if his camera sensor has the CameraSensorHelper implemented with something
>     like 'grep ^REGISTER_CAMERA_SENSOR_HELPER src/ipa/libipa/camera_sensor_helper.cpp'. If
>     the camera sensor used has no CameraSensorHelper, the Soft IPA works as before this patch.
>     This fallback option doesn't make the code nicer, and makes it harder to change the AE/AGC
>     algorithm to work faster, but I decided to keep it for now.
> - Milan:
>   [17/18] "libcamera: software_isp: Apply black level compensation"
>   - New patch adding black-level compensation autmatically detecting the
>     sensor blacklevel
> - Hans:
>   - Small typo changes in various comments/docs
>   - Replace typedef-s with using
> 
> Changes in v3 vs v2:
> - The Software ISP now is always build when building the simple pipeline
>   handler, no more -Dpipelines=simple/simple. Instead whether the SoftISP
>   is used or not is based on the media-controller driver. For now it is
>   only enabled for the "qcom-camss" driver (Andrey)
> - SoftISP factory has been removed, there is a just a single SoftwareISP class now
> - Fix the multi-threading issues (Andrey)
> - Fully document the SharedMemObject and DmaHeap classes (Andrey)
> - Drop SwStats base (integrate into SwStatsCpu class)
> - Move headers for classes only used by the SoftISP into src/libcamera/software_isp
> - Move SwStats / SwDebayer docs into .cpp and extend it
> - Rename many foo_bar symbols to fooBar
> - Add constexpr kFooBar values for varies hardcoded sizes like
>   the yHistoGram having 16 bins and the gammalookuptable having 1024 entries
> - Make startFrame() and finishFrame() normal methods instead of
>   using function pointers for these
> - Document how/why an array of src pointers is passed to
>   the debayer functions
> - 12bpp unpacked raw bayer input support (Kieran)
> - Add "Software ISP benchmarking" doc
> 
> 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 v4 of the patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v07
> 
> git branch for v3 of the patch set:
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v06
> 
> git branch for v2 of the 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
> 
> Andrei Konovalov (1):
>   libcamera: shared_mem_object: Reorganize the code and document the
>     SharedMemObject class
> 
> Andrey Konovalov (8):
>   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: ipa: Add Soft IPA
>   libcamera: Introduce SoftwareIsp
>   libcamera: pipeline: simple: Rename converterBuffers_ and related vars
>   libcamera: pipeline: simple: Enable use of Soft ISP and Soft IPA
> 
> Hans de Goede (7):
>   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, 10 and 12 bpp unpacked
>     bayer input
>   libcamera: debayer_cpu: Add support for 8, 10 and 12 bpp unpacked
>     bayer input
>   libcamera: debayer_cpu: Add BGR888 output support
>   libcamera: Add "Software ISP benchmarking" documentation
> 
> Milan Zamazal (2):
>   libcamera: shared_mem_object: Rename SIZE constant to `size'
>   libcamera: software_isp: Apply black level compensation
> 
>  Documentation/Doxyfile.in                     |   1 +
>  Documentation/index.rst                       |   1 +
>  Documentation/meson.build                     |   1 +
>  Documentation/software-isp-benchmarking.rst   |  77 ++
>  .../libcamera/internal}/dma_heaps.h           |  14 +-
>  include/libcamera/internal/meson.build        |   3 +
>  .../libcamera/internal/shared_mem_object.h    | 127 +++
>  .../internal/software_isp/debayer_params.h    |  29 +
>  .../internal/software_isp/meson.build         |   7 +
>  .../internal/software_isp/software_isp.h      |  99 +++
>  .../internal/software_isp/swisp_stats.h       |  49 ++
>  include/libcamera/ipa/meson.build             |   1 +
>  include/libcamera/ipa/soft.mojom              |  28 +
>  meson_options.txt                             |   2 +-
>  src/ipa/meson.build                           |   3 +
>  src/ipa/simple/black_level.cpp                |  88 ++
>  src/ipa/simple/black_level.h                  |  28 +
>  src/ipa/simple/data/meson.build               |  10 +
>  src/ipa/simple/data/uncalibrated.yaml         |   5 +
>  src/ipa/simple/meson.build                    |  30 +
>  src/ipa/simple/soft_simple.cpp                | 403 +++++++++
>  src/libcamera/dma_heaps.cpp                   | 165 ++++
>  src/libcamera/meson.build                     |   3 +
>  .../pipeline/rpi/common/shared_mem_object.h   | 128 ---
>  src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp  |  90 --
>  src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
>  src/libcamera/pipeline/simple/simple.cpp      | 241 ++++--
>  src/libcamera/shared_mem_object.cpp           | 236 +++++
>  src/libcamera/software_isp/TODO               | 279 ++++++
>  src/libcamera/software_isp/debayer.cpp        | 132 +++
>  src/libcamera/software_isp/debayer.h          |  54 ++
>  src/libcamera/software_isp/debayer_cpu.cpp    | 807 ++++++++++++++++++
>  src/libcamera/software_isp/debayer_cpu.h      | 158 ++++
>  src/libcamera/software_isp/meson.build        |  15 +
>  src/libcamera/software_isp/software_isp.cpp   | 357 ++++++++
>  src/libcamera/software_isp/swstats_cpu.cpp    | 432 ++++++++++
>  src/libcamera/software_isp/swstats_cpu.h      |  97 +++
>  38 files changed, 3922 insertions(+), 284 deletions(-)
>  create mode 100644 Documentation/software-isp-benchmarking.rst
>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
>  create mode 100644 include/libcamera/internal/shared_mem_object.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/software_isp.h
>  create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
>  create mode 100644 include/libcamera/ipa/soft.mojom
>  create mode 100644 src/ipa/simple/black_level.cpp
>  create mode 100644 src/ipa/simple/black_level.h
>  create mode 100644 src/ipa/simple/data/meson.build
>  create mode 100644 src/ipa/simple/data/uncalibrated.yaml
>  create mode 100644 src/ipa/simple/meson.build
>  create mode 100644 src/ipa/simple/soft_simple.cpp
>  create mode 100644 src/libcamera/dma_heaps.cpp
>  delete mode 100644 src/libcamera/pipeline/rpi/common/shared_mem_object.h
>  delete mode 100644 src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
>  create mode 100644 src/libcamera/shared_mem_object.cpp
>  create mode 100644 src/libcamera/software_isp/TODO
>  create mode 100644 src/libcamera/software_isp/debayer.cpp
>  create mode 100644 src/libcamera/software_isp/debayer.h
>  create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
>  create mode 100644 src/libcamera/software_isp/debayer_cpu.h
>  create mode 100644 src/libcamera/software_isp/meson.build
>  create mode 100644 src/libcamera/software_isp/software_isp.cpp
>  create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
>  create mode 100644 src/libcamera/software_isp/swstats_cpu.h
> 
> -- 
> 2.42.0
>
Milan Zamazal April 16, 2024, 3:37 p.m. UTC | #2
Kieran Bingham <kieran.bingham@ideasonboard.com> writes:

> Hi Milan,
>
> Quoting Milan Zamazal (2024-04-16 10:13:36)
>> Here is v8 of the patch-set to add Software ISP support
>> to libcamera / to the simple pipeline-handler.
>
> Thank you for all this work updating here.
>
> I've triggered the tests to run at
> https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1155543.
>
> There is one failure (don't panic) on debian:10. But I beleive that's
> because Debian:10 apt repositories have moved to archive, and so we'll
> need to update our container builds.
>
> To compensate for this, I've also run through my old integration build
> matrix which still passed.

Hi Kieran,

thank you for testing.  I noticed the debian10 failure too (it has been
mentioned, not very prominently, a bit more below).

> As such I believe we're ready to merge this series \o/
>
> And as this is what I was holding off for to tag libcamera-0.3 - I think
> that means a release is imminent, except that I'm very overloaded and
> don't have much time this week ;-)

Cool, looking forward both. :-)

Regards,
Milan

> --
> Kieran
>
>
>> 
>> Changes in v8 vs v7:
>> - Doc fixes in dma_heaps.cpp.
>> - SharedMemObject::size -> SharedMemObject::kSize
>>   (+ commit message expanded as suggested by Kieran).
>> - My commit messages limited to the line length of 72.
>> - All commit messages capitalized
>>   (for consistency both within the branch and with other libcamera
>>   commits).
>> - Using Span directly rather than defining SharedMem::span.
>> - Minor code changes in shared_mem_object.cpp as suggested by Laurent.
>> - Forgotten use of a type alias added in black_level.h
>>   (+ the type name capitalized).
>> - Rebased on current master.
>> - The dummy parameter dropped from setIspParams.
>> 
>> git branch for this v8 patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v11
>> CI pipeline:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1154685
>> (The debian10 failure looks unrelated.)
>> 
>> Changes in v7 vs v6:
>> Laurent's comments addressed, most notably:
>> * Formatting changes.
>> * Docstrings moved from *.h to *.cpp files.
>> * Improvements of docstrings.
>> * Miscellaneous refactorings without changing the code itself.
>> * Added src/libcamera/software_isp/TODO.
>> * Variables renamed:
>>   - DmaHeapFlag flag -> type
>>   - DmaHeapInfo.name -> deviceNodeName
>>   - generally renames to snakeCase where applicable
>> * DmaHeap no longer officially prefers CMA heap.
>> * SharedMem::mem_ is a Span now and private; size_ dropped.
>> * SharedMemObject template is limited to standard layout objects now.
>> * A more specific #include used in soft_simple.cpp.
>> * DebayerCpu::setDebayerFunctions no longer uses goto.
>> * The benchmarking document reformated to 80 columns.
>> * CameraSensorHelper patch integrated into preceding patches.
>> 
>> git branch for this v7 patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v10
>> CI pipeline:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/pipelines/1145462
>> 
>> Changes in v6 vs v5:
>> - [06/18] libcamera: software_isp: Add SwStatsCpu class
>>   Resolved a rebase conflict in meson.build.
>> - [16/18] libcamera: Add "Software ISP benchmarking"
>>   Fixed a trivial typo (Stefan).
>> - [17/18] libcamera: software_isp: Apply black level compensation
>>   A comment style adjusted (Kieran).
>>   A common value extracted into a variable.
>> 
>> git branch for this v6 patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v09
>> 
>> Changes in v5 vs v4:
>> - CI fully passes now!
>> - Fix some small typos / using unnamed const values pointed
>>   out in Milan's reviews
>> 
>> Since there have been no significant review comments on v4
>> and since this fully passes CI now I believe that this version
>> is ready for merging.
>> 
>> git branch for this v5 patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v08-for-ci
>> 
>> Older changelogs and links to git branches:
>> 
>> Changes in v4 vs v3:
>> - Andrey:
>>   [05/18] "libcamera: shared_mem_object: reorganize the code and document the SharedMemObject class"
>>   - documentation fixes
>>   [09/18] "libcamera: ipa: add Soft IPA"
>>   - Use int32_t for again*_ and exposure*_ as this matches
>>     the value the corresponding ControlValue::get() returns
>>   - Check for mmap() returning MAP_FAILED on error
>>   - Drop std::move() called on const SharedFD & argument
>>   - Replace #defines (EXPOSURE_OPTIMAL_VALUE etc) with const expressions
>>   - Use std::clamp() to keep exposure_ and again_ between the *_min_ and
>>     the *_max_
>>   - add comment on non-linear gain value vs gain code
>>   [10/18] "libcamera: introduce SoftwareIsp"
>>   - Fix the check of if sharedParams_ were created OK
>>   new [18/18] "libcamera: Soft IPA: use CameraSensorHelper for analogue gain"
>>   - One can check if his camera sensor has the CameraSensorHelper implemented with something
>>     like 'grep ^REGISTER_CAMERA_SENSOR_HELPER src/ipa/libipa/camera_sensor_helper.cpp'. If
>>     the camera sensor used has no CameraSensorHelper, the Soft IPA works as before this patch.
>>     This fallback option doesn't make the code nicer, and makes it harder to change the AE/AGC
>>     algorithm to work faster, but I decided to keep it for now.
>> - Milan:
>>   [17/18] "libcamera: software_isp: Apply black level compensation"
>>   - New patch adding black-level compensation autmatically detecting the
>>     sensor blacklevel
>> - Hans:
>>   - Small typo changes in various comments/docs
>>   - Replace typedef-s with using
>> 
>> Changes in v3 vs v2:
>> - The Software ISP now is always build when building the simple pipeline
>>   handler, no more -Dpipelines=simple/simple. Instead whether the SoftISP
>>   is used or not is based on the media-controller driver. For now it is
>>   only enabled for the "qcom-camss" driver (Andrey)
>> - SoftISP factory has been removed, there is a just a single SoftwareISP class now
>> - Fix the multi-threading issues (Andrey)
>> - Fully document the SharedMemObject and DmaHeap classes (Andrey)
>> - Drop SwStats base (integrate into SwStatsCpu class)
>> - Move headers for classes only used by the SoftISP into src/libcamera/software_isp
>> - Move SwStats / SwDebayer docs into .cpp and extend it
>> - Rename many foo_bar symbols to fooBar
>> - Add constexpr kFooBar values for varies hardcoded sizes like
>>   the yHistoGram having 16 bins and the gammalookuptable having 1024 entries
>> - Make startFrame() and finishFrame() normal methods instead of
>>   using function pointers for these
>> - Document how/why an array of src pointers is passed to
>>   the debayer functions
>> - 12bpp unpacked raw bayer input support (Kieran)
>> - Add "Software ISP benchmarking" doc
>> 
>> 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 v4 of the patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v07
>> 
>> git branch for v3 of the patch set:
>> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v06
>> 
>> git branch for v2 of the 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
>> 
>> Andrei Konovalov (1):
>>   libcamera: shared_mem_object: Reorganize the code and document the
>>     SharedMemObject class
>> 
>> Andrey Konovalov (8):
>>   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: ipa: Add Soft IPA
>>   libcamera: Introduce SoftwareIsp
>>   libcamera: pipeline: simple: Rename converterBuffers_ and related vars
>>   libcamera: pipeline: simple: Enable use of Soft ISP and Soft IPA
>> 
>> Hans de Goede (7):
>>   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, 10 and 12 bpp unpacked
>>     bayer input
>>   libcamera: debayer_cpu: Add support for 8, 10 and 12 bpp unpacked
>>     bayer input
>>   libcamera: debayer_cpu: Add BGR888 output support
>>   libcamera: Add "Software ISP benchmarking" documentation
>> 
>> Milan Zamazal (2):
>>   libcamera: shared_mem_object: Rename SIZE constant to `size'
>>   libcamera: software_isp: Apply black level compensation
>> 
>>  Documentation/Doxyfile.in                     |   1 +
>>  Documentation/index.rst                       |   1 +
>>  Documentation/meson.build                     |   1 +
>>  Documentation/software-isp-benchmarking.rst   |  77 ++
>>  .../libcamera/internal}/dma_heaps.h           |  14 +-
>>  include/libcamera/internal/meson.build        |   3 +
>>  .../libcamera/internal/shared_mem_object.h    | 127 +++
>>  .../internal/software_isp/debayer_params.h    |  29 +
>>  .../internal/software_isp/meson.build         |   7 +
>>  .../internal/software_isp/software_isp.h      |  99 +++
>>  .../internal/software_isp/swisp_stats.h       |  49 ++
>>  include/libcamera/ipa/meson.build             |   1 +
>>  include/libcamera/ipa/soft.mojom              |  28 +
>>  meson_options.txt                             |   2 +-
>>  src/ipa/meson.build                           |   3 +
>>  src/ipa/simple/black_level.cpp                |  88 ++
>>  src/ipa/simple/black_level.h                  |  28 +
>>  src/ipa/simple/data/meson.build               |  10 +
>>  src/ipa/simple/data/uncalibrated.yaml         |   5 +
>>  src/ipa/simple/meson.build                    |  30 +
>>  src/ipa/simple/soft_simple.cpp                | 403 +++++++++
>>  src/libcamera/dma_heaps.cpp                   | 165 ++++
>>  src/libcamera/meson.build                     |   3 +
>>  .../pipeline/rpi/common/shared_mem_object.h   | 128 ---
>>  src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp  |  90 --
>>  src/libcamera/pipeline/rpi/vc4/meson.build    |   1 -
>>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |   5 +-
>>  src/libcamera/pipeline/simple/simple.cpp      | 241 ++++--
>>  src/libcamera/shared_mem_object.cpp           | 236 +++++
>>  src/libcamera/software_isp/TODO               | 279 ++++++
>>  src/libcamera/software_isp/debayer.cpp        | 132 +++
>>  src/libcamera/software_isp/debayer.h          |  54 ++
>>  src/libcamera/software_isp/debayer_cpu.cpp    | 807 ++++++++++++++++++
>>  src/libcamera/software_isp/debayer_cpu.h      | 158 ++++
>>  src/libcamera/software_isp/meson.build        |  15 +
>>  src/libcamera/software_isp/software_isp.cpp   | 357 ++++++++
>>  src/libcamera/software_isp/swstats_cpu.cpp    | 432 ++++++++++
>>  src/libcamera/software_isp/swstats_cpu.h      |  97 +++
>>  38 files changed, 3922 insertions(+), 284 deletions(-)
>>  create mode 100644 Documentation/software-isp-benchmarking.rst
>>  rename {src/libcamera/pipeline/rpi/vc4 => include/libcamera/internal}/dma_heaps.h (65%)
>>  create mode 100644 include/libcamera/internal/shared_mem_object.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/software_isp.h
>>  create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
>>  create mode 100644 include/libcamera/ipa/soft.mojom
>>  create mode 100644 src/ipa/simple/black_level.cpp
>>  create mode 100644 src/ipa/simple/black_level.h
>>  create mode 100644 src/ipa/simple/data/meson.build
>>  create mode 100644 src/ipa/simple/data/uncalibrated.yaml
>>  create mode 100644 src/ipa/simple/meson.build
>>  create mode 100644 src/ipa/simple/soft_simple.cpp
>>  create mode 100644 src/libcamera/dma_heaps.cpp
>>  delete mode 100644 src/libcamera/pipeline/rpi/common/shared_mem_object.h
>>  delete mode 100644 src/libcamera/pipeline/rpi/vc4/dma_heaps.cpp
>>  create mode 100644 src/libcamera/shared_mem_object.cpp
>>  create mode 100644 src/libcamera/software_isp/TODO
>>  create mode 100644 src/libcamera/software_isp/debayer.cpp
>>  create mode 100644 src/libcamera/software_isp/debayer.h
>>  create mode 100644 src/libcamera/software_isp/debayer_cpu.cpp
>>  create mode 100644 src/libcamera/software_isp/debayer_cpu.h
>>  create mode 100644 src/libcamera/software_isp/meson.build
>>  create mode 100644 src/libcamera/software_isp/software_isp.cpp
>>  create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
>>  create mode 100644 src/libcamera/software_isp/swstats_cpu.h
>> 
>> -- 
>> 2.42.0
>>