[libcamera-devel] [PATCH v2 11/14] libcamera: ipa: Extend to support IPA interactions
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Sep 5 00:32:52 CEST 2019
Hi Niklas,
Thank you for the patch.
Just a few comments already, I'll review the API itself after reading
the rest of the series.
On Fri, Aug 30, 2019 at 01:26:50AM +0200, Niklas Söderlund wrote:
> The IPA interface needs to support interactions with the pipeline, add
> interfaces to control the sensor and handling of request ISP parameters
> and statistics.
>
> The interface needs to be extended further to support attaching the
> result of the statistic analyses to the request before the request
> completes to the user. It also needs to modified to allow proper per
> frame control of capture parameters.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/ipa/ipa_interface.h | 17 ++++++
> src/ipa/ipa_dummy.cpp | 5 ++
> src/ipa/ipa_dummy_isolate.cpp | 5 ++
> src/libcamera/ipa_interface.cpp | 73 +++++++++++++++++++++++++
> src/libcamera/proxy/ipa_proxy_linux.cpp | 12 ++--
> 5 files changed, 104 insertions(+), 8 deletions(-)
>
> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
> index 9bbc4cf58ec66420..38e16ff37214f7b9 100644
> --- a/include/libcamera/ipa/ipa_interface.h
> +++ b/include/libcamera/ipa/ipa_interface.h
> @@ -7,12 +7,29 @@
> #ifndef __LIBCAMERA_IPA_INTERFACE_H__
> #define __LIBCAMERA_IPA_INTERFACE_H__
>
> +#include <libcamera/controls.h>
> +#include <libcamera/signal.h>
> +
> +#include "v4l2_controls.h"
I really don't like including internal headers here, as the IPA API
needs to preserve backward compatibility. Any libcamera API used by
out-of-tree components should be part of the public API.
> +
> namespace libcamera {
>
> +class Buffer;
> +class Request;
> +
> class IPAInterface
> {
> public:
> virtual ~IPAInterface() {}
> +
> + virtual int initSensor(const V4L2ControlInfoMap &controls) = 0;
> + virtual void processRequest(const void *cookie,
> + const ControlList &controls,
> + Buffer ¶meters) = 0;
> + virtual void updateStatistics(const void *cookie, Buffer &statistics) = 0;
> +
> + Signal<V4L2ControlList> updateSensor;
> + Signal<const void *> queueRequest;
> };
>
> } /* namespace libcamera */
> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
> index 09f1b96a8f3d5c36..11dcc54ce4ebc6f4 100644
> --- a/src/ipa/ipa_dummy.cpp
> +++ b/src/ipa/ipa_dummy.cpp
> @@ -14,6 +14,11 @@ namespace libcamera {
>
> class IPADummy : public IPAInterface
> {
> +public:
> + int initSensor(const V4L2ControlInfoMap &controls) { return 0; }
> + void processRequest(const void *cookie, const ControlList &controls,
> + Buffer ¶meters) {}
> + void updateStatistics(const void *cookie, Buffer &statistics) {}
Please mark all these method as override, here and for IPADummyIsolate
and IPAProxyLinux.
> };
>
> /*
> diff --git a/src/ipa/ipa_dummy_isolate.cpp b/src/ipa/ipa_dummy_isolate.cpp
> index fa50be5309eba3c4..4d49c0e372466af6 100644
> --- a/src/ipa/ipa_dummy_isolate.cpp
> +++ b/src/ipa/ipa_dummy_isolate.cpp
> @@ -15,6 +15,11 @@ namespace libcamera {
>
> class IPADummyIsolate : public IPAInterface
> {
> +public:
> + int initSensor(const V4L2ControlInfoMap &controls) { return 0; }
> + void processRequest(const void *cookie, const ControlList &controls,
> + Buffer ¶meters) {}
> + void updateStatistics(const void *cookie, Buffer &statistics) {}
> };
>
> /*
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index 273477a5272677b7..be9eab3cda32379d 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -19,4 +19,77 @@ namespace libcamera {
> * \brief Interface for IPA implementation
> */
>
> +/**
> + * \fn IPAInterface::initSensor()
> + * \brief Initialize the IPA sensor settings
> + * \param[in] controls List of controls provided by the sensor
> + *
> + * This function is called when a pipeline attaches to an IPA to inform the IPA
> + * of the controls and limits the sensor in the video pipeline supports. The IPA
> + * have the option to set controls of the sensor by emitting the updateSensor
> + * signal.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
> +/**
> + * \fn IPAInterface::processRequest()
> + * \brief Provide IPA with a parameter buffer to fill
> + * \param[in] cookie Cookie for the request
> + * \param[in] controls List of controls associated with the request
> + * \param[in,out] parameters Buffer containing an empty parameters buffer
> + *
> + * This function is called by a pipeline handler when it has a request to
> + * process. The IPA shall fill in the \a parameters buffer to achieve the
> + * capture result described in \a controls. The \a cookie value identifies
> + * the request the controls and parameters buffer corresponds to and can be
> + * matched in updateStatistics() once the request have been processed by the
> + * hardware. The cookie value is only valid from that processRequest() is
> + * called and until updateStatistics() return.
> + *
> + * When the \a parameters buffer is filled in and ready to be queued to hardware
> + * the IPA shall signal pipeline handler using the queueRequest signal with the
> + * cookie value corresponding to the request the parameters belong to.
> + */
> +
> +/**
> + * \fn IPAInterface::updateStatistics()
> + * \brief Provide IPA with statistic buffer to examine
> + * \param[in] cookie Cookie for the request
> + * \param[in] statistics Buffer containing a filled in statistic buffer
> + *
> + * This function is called once a statistic buffer is completed and have been
> + * dequeue. The IPA may inspect the buffer and update its internal view of the
> + * capture conditions. The \a cookie value can be used to associate the dequeued
> + * statistics buffer with the parameters buffer filled in by the IPA in
> + * processRequest().
> + *
> + * \todo Extend this function to return or signal the result of the statistic
> + * examination by the IPA.
> + */
> +
> +/**
> + * \var IPAInterface::updateSensor
> + * \brief Signal emitted when the IPA wish to update sensor V4L2 controls
> + *
> + * This signal is emitted when the IPA wish to update one or more V4L2 control
> + * of the sensor in the video pipeline. The list of controls to modify is passed
> + * as a parameter.
> + *
> + * \todo Extend this function to work with per-frame control setting of
> + * controls.
> + */
> +
> +/**
> + * \var IPAInterface::queueRequest
> + * \brief Signal emitted when the IPA is done preparing a request
> + *
> + * This signal is emitted then the IPA have finished filling in the parameters
> + * buffer and is ready for the request to be committed to hardware for capture.
> + * The request cookie is passed as a parameter.
> + *
> + * \todo Extend this function to work with per-frame control setting of
> + * controls.
> + */
> +
> } /* namespace libcamera */
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index f881aab5b2e1178a..bb7aeedf12779c66 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -26,7 +26,10 @@ public:
> IPAProxyLinux(IPAModule *ipam);
> ~IPAProxyLinux();
>
> - int init();
This belongs to the patch that removed the init() method from
IPAInterface.
> + int initSensor(const V4L2ControlInfoMap &controls) { return 0; }
> + void processRequest(const void *cookie, const ControlList &controls,
> + Buffer ¶meters) {}
> + void updateStatistics(const void *cookie, Buffer &statistics) {}
>
> private:
> void readyRead(IPCUnixSocket *ipc);
> @@ -36,13 +39,6 @@ private:
> IPCUnixSocket *socket_;
> };
>
> -int IPAProxyLinux::init()
> -{
> - LOG(IPAProxy, Debug) << "initializing IPA via dummy proxy!";
> -
> - return 0;
> -}
> -
> IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)
> {
> LOG(IPAProxy, Debug)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list