[PATCH v8 00/18] libcamera: introduce Software ISP and Software IPA

Milan Zamazal mzamazal at redhat.com
Tue Apr 16 17:37:48 CEST 2024


Kieran Bingham <kieran.bingham at 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
>>



More information about the libcamera-devel mailing list