[libcamera-devel] [PATCH v8 04/12] libcamera: IPAInterface: Replace C API with the new C++-only API
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Feb 15 04:24:14 CET 2021
Hi Paul,
Thank you for the patch.
On Sat, Feb 13, 2021 at 01:22:17PM +0900, Paul Elder wrote:
> Remove everything related to the C API, including ipa_context,
> ipa_context_wrapper, and IPAInterfaceWrapper. Also remove relevant
> documentation.
>
> ipaCreate() provided by IPA implementations, and createInterface()
> provided by IPAModule (wrapper around IPA implementation) both now
> return a C++ object IPAInterface instead of struct ipa_context.
>
> Although IPAInterfaceWrapper is the only component of libipa, the
> skeleton and build files for libipa are retained.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> No change in v8
>
> No change in v7
>
> Changes in v6:
> - remove definitions and documentation for IPASettings, IPAStream,
> IPABuffer, and IPAOperationData, as the first three are moved to
> mojom, and the last is simply superseded
>
> Squashed in v5
> - change some words, remove some outdated points
> - don't remove libipa, only remove IPAInterfaceWrapper
> - as a result, libipa is now empty
> - update IPAInterface documentation
> - add todo on how to generate documentation for the new IPAInterface
> implementations
>
> No change in v4
>
> No change in v3
>
> Changes in v2:
> - add documentation for IPAModule::createInterface()
>
> ---
>
> This is a combination of 6 commits:
>
> ---
>
> libcamera: IPAModule: Replace ipa_context with IPAInterface
>
> With the new IPC infrastructure, we no longer need the C interface as
> provided by struct ipa_context. Make ipaCreate_() and createInterface()
> return IPAInterface.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
>
> libcamera: ipa_context_wrapper: Remove ipa_context_wrapper
>
> Since ipa_context has been replaced with custom IPAInterfaces, it is not
> longer needed. Remove it.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
>
> libcamera: IPAInterface: remove ipa_context and functions from documentation
>
> Remove all the documentation related to ipa_context and the C IPA API,
> as well as the documentation about the functions in the IPAInterface.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> ---
>
> libcamera: IPAInterface: Remove all functions from IPAInterface
>
> Now that all the functions in the IPA interface are defined in the data
> definition file and a specialized IPAInterface is generated per pipeline
> handler, remove all the functions from the base IPAInterface.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
>
> libcamera: IPAInterface: make ipaCreate return IPAInterface
>
> With the new IPC infrastructure, we no longer need the C interface as
> provided by struct ipa_context. Make ipaCreate return IPAinterface.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> ---
>
> ipa: remove IPAInterfaceWrapper
>
> As every pipeline has its own proxy, IPAInterfaceWrapper is no
> longer necessary. Remove it.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Acked-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Used to be "ipa: remove libipa"
> ---
> .../libcamera/internal/ipa_context_wrapper.h | 53 --
> include/libcamera/internal/ipa_module.h | 4 +-
> include/libcamera/internal/meson.build | 1 -
> include/libcamera/ipa/ipa_interface.h | 147 +---
> src/ipa/ipu3/ipu3.cpp | 2 -
> src/ipa/libipa/ipa_interface_wrapper.cpp | 287 --------
> src/ipa/libipa/ipa_interface_wrapper.h | 61 --
> src/ipa/libipa/meson.build | 2 -
> src/ipa/raspberrypi/raspberrypi.cpp | 2 -
> src/ipa/rkisp1/rkisp1.cpp | 2 -
> src/ipa/vimc/vimc.cpp | 2 -
> src/libcamera/ipa_context_wrapper.cpp | 298 --------
> src/libcamera/ipa_interface.cpp | 643 ++----------------
> src/libcamera/ipa_module.cpp | 18 +-
> src/libcamera/meson.build | 1 -
> 15 files changed, 57 insertions(+), 1466 deletions(-)
> delete mode 100644 include/libcamera/internal/ipa_context_wrapper.h
> delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp
> delete mode 100644 src/ipa/libipa/ipa_interface_wrapper.h
> delete mode 100644 src/libcamera/ipa_context_wrapper.cpp
[snip]
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index 5be6f787..ae049831 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
[snip]
> @@ -516,147 +90,16 @@ namespace libcamera {
> * mirror core libcamera structures when the latter are not suitable, such as
> * IPAStream to carry StreamConfiguration data.
> *
> - * As for the functions defined in struct ipa_context_ops, the methods defined
> - * by this class shall not return data from the IPA.
> + * Custom data structures may also be defined in the mojom file, in which case
> + * the (de)serialization will automatically be generated. If any other libcamera
> + * structures are to be used as parameters, then a (de)serializer for them must
> + * be implemented in IPADataSerializer.
> *
> - * The pipeline handler shall use the IPAManager to locate a compatible
> + * The pipeline handlers shall use the IPAManager to locate a compatible
> * IPAInterface. The interface may then be used to interact with the IPA module.
> - */
> -
> -/**
> - * \fn IPAInterface::init()
> - * \brief Initialise the IPAInterface
> - * \param[in] settings The IPA initialization settings
> - *
> - * This function initializes the IPA interface. It shall be called before any
> - * other function of the IPAInterface. The \a settings carry initialization
> - * parameters that are valid for the whole life time of the IPA interface.
> - */
> -
> -/**
> - * \fn IPAInterface::start()
> - * \brief Start the IPA
> - * \param[in] data Protocol-specific data for the start operation
> - * \param[out] result Result of the start operation
> - *
> - * This method informs the IPA module that the camera is about to be started.
> - * The IPA module shall prepare any resources it needs to operate.
> - *
> - * The \a data and \a result parameters carry custom data passed by the
> - * pipeline handler to the IPA and back. The pipeline handler may set the \a
> - * result parameter to null if the IPA protocol doesn't need to pass a result
> - * back through the start() function.
> - *
> - * \return 0 on success or a negative error code otherwise
> - */
> -
> -/**
> - * \fn IPAInterface::stop()
> - * \brief Stop the IPA
> - *
> - * This method informs the IPA module that the camera is stopped. The IPA module
> - * shall release resources prepared in start().
> - */
> -
> -/**
> - * \fn IPAInterface::configure()
> - * \brief Configure the IPA stream and sensor settings
> - * \param[in] sensorInfo Camera sensor information
> - * \param[in] streamConfig Configuration of all active streams
> - * \param[in] entityControls Controls provided by the pipeline entities
> - * \param[in] ipaConfig Pipeline-handler-specific configuration data
> - * \param[out] result Pipeline-handler-specific configuration result
> *
> - * This method shall be called when the camera is started to inform the IPA of
> - * the camera's streams and the sensor settings. The meaning of the numerical
> - * keys in the \a streamConfig and \a entityControls maps is defined by the IPA
> - * protocol.
> - *
> - * The \a sensorInfo conveys information about the camera sensor settings that
> - * the pipeline handler has selected for the configuration. The IPA may use
> - * that information to tune its algorithms.
> - *
> - * The \a ipaConfig and \a result parameters carry custom data passed by the
> - * pipeline handler to the IPA and back. The pipeline handler may set the \a
> - * result parameter to null if the IPA protocol doesn't need to pass a result
> - * back through the configure() function.
> - */
> -
> -/**
> - * \fn IPAInterface::mapBuffers()
> - * \brief Map buffers shared between the pipeline handler and the IPA
> - * \param[in] buffers List of buffers to map
> - *
> - * This method informs the IPA module of memory buffers set up by the pipeline
> - * handler that the IPA needs to access. It provides dmabuf file handles for
> - * each buffer, and associates the buffers with unique numerical IDs.
> - *
> - * IPAs shall map the dmabuf file handles to their address space and keep a
> - * cache of the mappings, indexed by the buffer numerical IDs. The IDs are used
> - * in all other IPA interface methods to refer to buffers, including the
> - * unmapBuffers() method.
> - *
> - * All buffers that the pipeline handler wishes to share with an IPA shall be
> - * mapped with this method. Buffers may be mapped all at once with a single
> - * call, or mapped and unmapped dynamically at runtime, depending on the IPA
> - * protocol. Regardless of the protocol, all buffers mapped at a given time
> - * shall have unique numerical IDs.
> - *
> - * The numerical IDs have no meaning defined by the IPA interface, and IPA
> - * protocols shall not give them any specific meaning either. They should be
> - * treated as opaque handles by IPAs, with the only exception that ID zero is
> - * invalid.
> - *
> - * \sa unmapBuffers()
> - *
> - * \todo Provide a generic implementation of mapBuffers and unmapBuffers for
> - * IPAs
> - */
> -
> -/**
> - * \fn IPAInterface::unmapBuffers()
> - * \brief Unmap buffers shared by the pipeline to the IPA
> - * \param[in] ids List of buffer IDs to unmap
> - *
> - * This method removes mappings set up with mapBuffers(). Buffers may be
> - * unmapped all at once with a single call, or selectively at runtime, depending
> - * on the IPA protocol. Numerical IDs of unmapped buffers may be reused when
> - * mapping new buffers.
> - *
> - * \sa mapBuffers()
> + * \todo Figure out how to generate IPAInterface documentation.
> */
>
> -/**
> - * \fn IPAInterface::processEvent()
> - * \brief Process an event from the pipeline handler
> - * \param[in] data IPA operation data
> - *
> - * This operation is used by pipeline handlers to inform the IPA module of
> - * events that occurred during the on-going capture operation.
> - *
> - * The event notified by the pipeline handler with this method is handled by the
> - * IPA, which interprets the operation parameters according to the separately
> - * documented IPA protocol.
> - */
> -
> -/**
> - * \var IPAInterface::queueFrameAction
> - * \brief Queue an action associated with a frame to the pipeline handler
> - * \param[in] frame The frame number for the action
> - * \param[in] data IPA operation data
> - *
> - * This signal is emitted when the IPA wishes to queue a FrameAction on the
> - * pipeline. The pipeline is still responsible for the scheduling of the action
> - * on its timeline.
> - *
> - * This signal is emitted by the IPA to queue an action to be executed by the
> - * pipeline handler on a frame. The type of action is identified by the
> - * \a data.operation field, as defined by the IPA protocol, and the rest of the
> - * \a data is interpreted accordingly. The pipeline handler shall queue the
> - * action and execute it as appropriate.
> - *
> - * The signal is only emitted when the IPA is running, that is after start() and
> - * before stop() have been called.
> - */
>
Extra blank line.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list