Message ID | 20191108205409.18845-1-laurent.pinchart@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
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 >