[libcamera-devel] [PATCH v2 00/24] Control serialization and IPA C API
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Nov 19 18:06:02 CET 2019
Hi Laurent,
As far as I can see, there are only a few minor comments across this
series, and has been reviewed extensively enough to say lets get this
in, with the minors resolved.
A topic such as this, which is so intrinsic to the rest of the system
will only block further development, so I think it's important that we
get this in soon so that development and testing can continue on top.
Thanks for tackling such a large series.
--
Kieran
On 08/11/2019 20:53, Laurent Pinchart wrote:
> Hello,
>
> This patch series turns the IPA API towards IPA modules to a plain C
> API. The rationale is explained in patch 19/24:
>
> The C++ objects that are expected to convey data through the IPA API
> will have associated methods that would require IPAs to link to
> libcamera. Even though the libcamera license allows this, suppliers of
> closed-source IPAs may have a different interpretation. To ease their
> mind and clearly separate vendor code and libcamera code, define a plain
> C IPA API. The corresponding C objects are stored in plain C structures
> or have their binary format documented, removing the need for linking to
> libcamera code on the IPA side.
>
> Patches 01/24 to 12/24 are preparatory changes. Patches 01/24 to 03/24
> add a test case for and fix a bug in the ControlInfoMap. Patches 04/24
> to 12/24 then rework and expand the BufferMemory and control classes to
> make translation between the C++ and C API possible.
>
> A plain C API requires storing all data passed to IPA context operations
> in plain C data structures. For most of the data used in the
> IPAInterface methods, this only requires defining corresponding C
> structures and replacing std::vector and std::map with arrays. However,
> for ControlInfoMap and ControlList, a different approach is needed due
> to their complexity.
>
> Patch 13/24 defines a serialization format for those two classes, and
> patches 14/24 to 17/24 implement serialization and deserialization
> (including a test case).
>
> Patches 18/24 to 20/24 then define a plain C IPA API and switch to it.
> Patch 21/24 is a small improvement to compile time test coverage related
> to IPA modules, and patch 22/24 allows short-circuiting the C API for
> IPA modules that are loaded without isolation. Patch 23/24 adds a test
> case, and patch 24/24 finally fixes a typo.
>
> I would like to thank both Kieran and Jacopo for all their preparatory
> work on this. We all spend lots of time burning brain cells, and
> hopefully the result will make everybody happy.
>
> Jacopo Mondi (6):
> libcamera: controls: Make ControlId constructor public
> libcamera: controls: Store reference to the InfoMap
> ipa: Define serialized controls
> libcamera: Add ByteStreamBuffer
> test: Add control serialization test
> ipa: Switch to the plain C API
>
> Laurent Pinchart (18):
> test: Extract CameraTest class out of camera tests to libtest
> test: controls: Add ControlInfoMap test
> libcamera: controls: Avoid exception in ControlList count() and find()
> libcamera: controls: Add operator== and operator!= to ControlRange
> libcamera: controls: Index ControlList by unsigned int
> libcamera: controls: Add move constructor to ControlInfoMap
> libcamera: controls: Make ControList constructor public
> libcamera: controls: Catch type mismatch in ControlInfoMap
> libcamera: v4l2_controls: Fix control range construction for bool
> libcamera: buffer: Add const accessor to Buffer planes
> test: Add ByteStreamBuffer test
> libcamera: Add controls serializer
> ipa: Pass ControlInfoMap references to IPAInterface::configure()
> ipa: Define a plain C API
> ipa: Declare the ipaCreate() function prototype
> ipa: Allow short-circuiting the ipa_context_ops
> test: ipa: Add IPA wrappers test
> libcamera: Fix typo related to serialization
>
> Documentation/Doxyfile.in | 1 +
> Documentation/meson.build | 2 +
> include/ipa/ipa_controls.h | 54 ++
> include/ipa/ipa_interface.h | 82 ++-
> include/ipa/meson.build | 1 +
> include/libcamera/buffer.h | 1 +
> include/libcamera/controls.h | 50 +-
> src/ipa/ipa_vimc.cpp | 9 +-
> src/ipa/libipa/ipa_interface_wrapper.cpp | 248 +++++++++
> src/ipa/libipa/ipa_interface_wrapper.h | 57 ++
> src/ipa/libipa/meson.build | 13 +
> src/ipa/meson.build | 3 +
> src/ipa/rkisp1/meson.build | 3 +-
> src/ipa/rkisp1/rkisp1.cpp | 9 +-
> src/libcamera/buffer.cpp | 6 +
> src/libcamera/byte_stream_buffer.cpp | 286 ++++++++++
> src/libcamera/camera_controls.cpp | 4 +-
> src/libcamera/control_serializer.cpp | 501 ++++++++++++++++++
> src/libcamera/controls.cpp | 139 +++--
> src/libcamera/include/byte_stream_buffer.h | 63 +++
> src/libcamera/include/camera_controls.h | 2 +-
> src/libcamera/include/control_serializer.h | 52 ++
> src/libcamera/include/control_validator.h | 2 +-
> src/libcamera/include/ipa_context_wrapper.h | 47 ++
> src/libcamera/include/ipa_module.h | 5 +-
> src/libcamera/include/meson.build | 3 +
> src/libcamera/ipa_context_wrapper.cpp | 249 +++++++++
> src/libcamera/ipa_controls.cpp | 187 +++++++
> src/libcamera/ipa_interface.cpp | 325 ++++++++++--
> src/libcamera/ipa_manager.cpp | 67 ++-
> src/libcamera/ipa_module.cpp | 23 +-
> src/libcamera/meson.build | 4 +
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/uvcvideo.cpp | 4 +-
> src/libcamera/pipeline/vimc.cpp | 4 +-
> src/libcamera/proxy/ipa_proxy_linux.cpp | 2 +-
> .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +-
> src/libcamera/v4l2_controls.cpp | 14 +-
> src/libcamera/v4l2_device.cpp | 23 +-
> test/byte-stream-buffer.cpp | 172 ++++++
> test/camera/buffer_import.cpp | 14 +-
> test/camera/capture.cpp | 15 +-
> test/camera/configuration_default.cpp | 16 +-
> test/camera/configuration_set.cpp | 15 +-
> test/camera/meson.build | 2 +-
> test/camera/statemachine.cpp | 15 +-
> test/controls/control_info.cpp | 77 +++
> test/controls/control_list.cpp | 39 +-
> test/controls/meson.build | 1 +
> test/ipa/ipa_wrappers_test.cpp | 390 ++++++++++++++
> test/ipa/meson.build | 5 +-
> test/{camera => libtest}/camera_test.cpp | 24 +-
> test/{camera => libtest}/camera_test.h | 12 +-
> test/libtest/meson.build | 1 +
> test/meson.build | 2 +
> test/serialization/control_serialization.cpp | 161 ++++++
> test/serialization/meson.build | 11 +
> test/serialization/serialization_test.cpp | 89 ++++
> test/serialization/serialization_test.h | 33 ++
> 59 files changed, 3432 insertions(+), 217 deletions(-)
> create mode 100644 include/ipa/ipa_controls.h
> create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp
> create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h
> create mode 100644 src/ipa/libipa/meson.build
> create mode 100644 src/libcamera/byte_stream_buffer.cpp
> create mode 100644 src/libcamera/control_serializer.cpp
> create mode 100644 src/libcamera/include/byte_stream_buffer.h
> create mode 100644 src/libcamera/include/control_serializer.h
> create mode 100644 src/libcamera/include/ipa_context_wrapper.h
> create mode 100644 src/libcamera/ipa_context_wrapper.cpp
> create mode 100644 src/libcamera/ipa_controls.cpp
> create mode 100644 test/byte-stream-buffer.cpp
> create mode 100644 test/controls/control_info.cpp
> create mode 100644 test/ipa/ipa_wrappers_test.cpp
> rename test/{camera => libtest}/camera_test.cpp (55%)
> rename test/{camera => libtest}/camera_test.h (77%)
> create mode 100644 test/serialization/control_serialization.cpp
> create mode 100644 test/serialization/meson.build
> create mode 100644 test/serialization/serialization_test.cpp
> create mode 100644 test/serialization/serialization_test.h
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list