| Message ID | 20190927024417.725906-9-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On Fri, Sep 27, 2019 at 04:44:12AM +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. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/ipa/ipa_interface.h | 21 +++++ > src/ipa/ipa_dummy.cpp | 6 +- > src/libcamera/ipa_interface.cpp | 106 ++++++++++++++++++++++++ > src/libcamera/proxy/ipa_proxy_linux.cpp | 13 ++- > 4 files changed, 137 insertions(+), 9 deletions(-) > > diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h > index 2c5eb1fd524311cb..feeb00b1b1315ca5 100644 > --- a/include/ipa/ipa_interface.h > +++ b/include/ipa/ipa_interface.h > @@ -7,14 +7,35 @@ > #ifndef __LIBCAMERA_IPA_INTERFACE_H__ > #define __LIBCAMERA_IPA_INTERFACE_H__ > > +#include <libcamera/controls.h> > +#include <libcamera/signal.h> > + > +#include "v4l2_controls.h" > + > namespace libcamera { > > +class BufferMemory; > +class Request; > +struct IPAMetaData; > + > class IPAInterface > { > public: > virtual ~IPAInterface() {} > > virtual int init() = 0; > + > + virtual void initSensor(const V4L2ControlInfoMap &controls) = 0; Are we ok for now to assume all sensor related information could be expressed a V4L2Controls ? > + virtual void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) = 0; Why raw memory buffers and not pools ? When running through the C interface I assume we'll just need to get down to pass file descriptors in a binary buffer, which makes me wonder if we assume all memory buffers will have the same number of planes (each of them with an associated fd) or do we need to define a serialization format to express that. > + > + virtual void signalBuffer(unsigned int type, unsigned int id) = 0; > + > + virtual void queueRequest(unsigned int frame, const ControlList &controls) = 0; > + > + Signal<unsigned int, V4L2ControlList> updateSensor; > + Signal<unsigned int, unsigned int, unsigned int> queueBuffer; > + Signal<unsigned int, int, int> setDelay; > + Signal<unsigned int, IPAMetaData> metaDataReady; I'm a bit skeptical of IPAMetaData. As we refreined from using C structures to pass parameters from pipeline to IPA to avoid versioning and the need to keep in sync several pieces of the implementation, I'm for the same reason oriented to do the same here and return a ControlList. Would this be possible? > }; > > } /* namespace libcamera */ > diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp > index 9d0cbdc8b1ad5565..4d429f20d7aaa955 100644 > --- a/src/ipa/ipa_dummy.cpp > +++ b/src/ipa/ipa_dummy.cpp > @@ -15,7 +15,11 @@ namespace libcamera { > class IPADummy : public IPAInterface > { > public: > - int init(); > + int init() override; > + void initSensor(const V4L2ControlInfoMap &controls) override {} > + void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) override {} > + void signalBuffer(unsigned int type, unsigned int id) override {} > + void queueRequest(unsigned int frame, const ControlList &controls) override {} > }; > > int IPADummy::init() > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp > index d7d8ca8881efcf66..79853771c70f15d4 100644 > --- a/src/libcamera/ipa_interface.cpp > +++ b/src/libcamera/ipa_interface.cpp > @@ -24,4 +24,110 @@ namespace libcamera { > * \brief Initialise the IPAInterface > */ > > +/** > + * \fn IPAInterface::initSensor() > + * \brief Initialize the IPA sensor settings > + * \param[in] controls List of controls provided by the sensor mm, more than provided I would say supported. Same question as above, this is the list of supported controls, will they capture the sensor's characteristics too ? > + * > + * 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. > + */ > + > +/** > + * \fn IPAInterface::initBuffers() > + * \brief Initialize the buffers shared between pipeline and IPA > + * \param[in] type Type of buffers described in \a buffers > + * \param[in] buffers List of buffers of \a type > + * > + * This function is called when a pipeline handler wants to inform the IPA of > + * which buffers it has mapped for a specific purpose. All buffers shared > + * between these two object's needs to be shared using this function prior to > + * use. > + * > + * After the buffers have been initialized they are referenced using an > + * numerical \a id which represents the insertion order in the \a buffers list > + * given as an argument here. > + * > + * It is possible to call this function multiple times for different kinds of > + * buffer types, e.g. once for statistic buffers and once more for parameter > + * buffers. It's also possible to call this function multiple times for the > + * same buffer type if the pipeline handler wants to update the mappings inside > + * the IPA. > + * > + * The buffer type numerical ids and it's usage are not enforced by the IPA > + * interface and is left as pipeline handler specific. > + */ > + > +/** > + * \fn IPAInterface::signalBuffer() > + * \brief Signal that the pipeline handlers is done processing a buffer done processing or the buffer is ready for processing on the IPA side ? > + * \param[in] type Type of buffer As a general note on types, we could define an empty ActionType enum and let timeline subclasses extend it. This would gurantee a better (sometimes annoying) type safety when handling types in timelines implementations > + * \param[in] id Buffer id in \a type I wonder if the bookeeping of buffer id <-> buffers you have implemented in the RkISP IPA is robust. We had a similar translation to indexes to buffer for IPU3 and it was a bit of a pain. Couldn't we just make BufferPools and Buffers serializable and pass the buffer here? > + * > + * A pipeline handler can signal to the IPA that it is done with buffer. This > + * may have different meanings for different buffer types. For example signaling > + * a parameter buffer may be an indication that it's now free and the IPA can > + * update its content and schedule it to be written to the hardware again. While > + * signaling a parameters buffer may indicate that it's filled with data from > + * the hardware and are ready to be consumed by the IPA. > + * > + * As all pipeline designs are under the unique it must document what signaling > + * a buffer of a specific type means for that particular IPA interface. Is this a todo ? I didn't get "under the unique it" > + */ > + > +/** > + * \fn IPAInterface::queueRequest() > + * \brief Inform the IPA that a request have been queued to the pipeline > + * \param[in] frame The frame number for the request > + * \param[in] controls List of controls associated with the request > + * > + * This function is called by a pipeline handler when it has a request to > + * process. The pipeline informs the IPA that \a frame shall be processed > + * with the parameters in \a controls. > + * > + * The IPA may act on this by emitting different signals in the IPA interface > + * to configure the pipeline to act on \a frame according to the \a controls. > + */ > + > +/** > + * \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 frame number the settings should be > + * ready for and a list of controls to modify is passed as parameters. > + */ > + > +/** > + * \var IPAInterface::queueBuffer > + * \brief Signal emitted when the IPA wish to queue a buffer to the hardware > + * > + * This signal is emitted then the IPA wish to queue q buffer to the hardware. > + * The frame number and buffer type and id are passed as parameters. > + */ > + > +/** > + * \var IPAInterface::setDelay > + * \brief Signal emitted when the IPA wish to set a delay in the pipeline > + * > + * This signal is emitted when the IPA wish to change a delay inside the > + * pipeline timeline. The action type, frame and time offsets are passed as > + * parameters. > + * > + * The unit for the time offset is not fixed in the IPA interface, it is up to > + * the pipeline implementation to choose a suitable unit for its use-case. > + */ > + > +/** > + * \var IPAInterface::metaDataReady > + * \brief Signal emitted when the IPA is done processing statistics > + * > + * This signal is emitted when the IPA have finished processing the statistics > + * buffer and have created an IPAMetaData object which are ready to be consumed are/is > + * by the pipeline handler. The frame number and the meta data is passed as is/are > + * parameters. > + */ > + > } /* namespace libcamera */ > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp > index 62fcb529e1c7e4ff..17d09fb21582376d 100644 > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp > @@ -26,7 +26,11 @@ public: > IPAProxyLinux(IPAModule *ipam); > ~IPAProxyLinux(); > > - int init(); > + int init() override { return 0; } > + void initSensor(const V4L2ControlInfoMap &controls) override {} > + void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) override {} > + void signalBuffer(unsigned int type, unsigned int id) override {} > + void queueRequest(unsigned int frame, const ControlList &controls) override {} > > private: > void readyRead(IPCUnixSocket *ipc); > @@ -36,13 +40,6 @@ private: > IPCUnixSocket *socket_; > }; > > -int IPAProxyLinux::init() > -{ > - LOG(IPAProxy, Debug) << "initializing IPA via dummy proxy!"; > - > - return 0; > -} > - > IPAProxyLinux::IPAProxyLinux(IPAModule *ipam) > : proc_(nullptr), socket_(nullptr) > { > -- > 2.23.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 2c5eb1fd524311cb..feeb00b1b1315ca5 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -7,14 +7,35 @@ #ifndef __LIBCAMERA_IPA_INTERFACE_H__ #define __LIBCAMERA_IPA_INTERFACE_H__ +#include <libcamera/controls.h> +#include <libcamera/signal.h> + +#include "v4l2_controls.h" + namespace libcamera { +class BufferMemory; +class Request; +struct IPAMetaData; + class IPAInterface { public: virtual ~IPAInterface() {} virtual int init() = 0; + + virtual void initSensor(const V4L2ControlInfoMap &controls) = 0; + virtual void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) = 0; + + virtual void signalBuffer(unsigned int type, unsigned int id) = 0; + + virtual void queueRequest(unsigned int frame, const ControlList &controls) = 0; + + Signal<unsigned int, V4L2ControlList> updateSensor; + Signal<unsigned int, unsigned int, unsigned int> queueBuffer; + Signal<unsigned int, int, int> setDelay; + Signal<unsigned int, IPAMetaData> metaDataReady; }; } /* namespace libcamera */ diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp index 9d0cbdc8b1ad5565..4d429f20d7aaa955 100644 --- a/src/ipa/ipa_dummy.cpp +++ b/src/ipa/ipa_dummy.cpp @@ -15,7 +15,11 @@ namespace libcamera { class IPADummy : public IPAInterface { public: - int init(); + int init() override; + void initSensor(const V4L2ControlInfoMap &controls) override {} + void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) override {} + void signalBuffer(unsigned int type, unsigned int id) override {} + void queueRequest(unsigned int frame, const ControlList &controls) override {} }; int IPADummy::init() diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index d7d8ca8881efcf66..79853771c70f15d4 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -24,4 +24,110 @@ namespace libcamera { * \brief Initialise the IPAInterface */ +/** + * \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. + */ + +/** + * \fn IPAInterface::initBuffers() + * \brief Initialize the buffers shared between pipeline and IPA + * \param[in] type Type of buffers described in \a buffers + * \param[in] buffers List of buffers of \a type + * + * This function is called when a pipeline handler wants to inform the IPA of + * which buffers it has mapped for a specific purpose. All buffers shared + * between these two object's needs to be shared using this function prior to + * use. + * + * After the buffers have been initialized they are referenced using an + * numerical \a id which represents the insertion order in the \a buffers list + * given as an argument here. + * + * It is possible to call this function multiple times for different kinds of + * buffer types, e.g. once for statistic buffers and once more for parameter + * buffers. It's also possible to call this function multiple times for the + * same buffer type if the pipeline handler wants to update the mappings inside + * the IPA. + * + * The buffer type numerical ids and it's usage are not enforced by the IPA + * interface and is left as pipeline handler specific. + */ + +/** + * \fn IPAInterface::signalBuffer() + * \brief Signal that the pipeline handlers is done processing a buffer + * \param[in] type Type of buffer + * \param[in] id Buffer id in \a type + * + * A pipeline handler can signal to the IPA that it is done with buffer. This + * may have different meanings for different buffer types. For example signaling + * a parameter buffer may be an indication that it's now free and the IPA can + * update its content and schedule it to be written to the hardware again. While + * signaling a parameters buffer may indicate that it's filled with data from + * the hardware and are ready to be consumed by the IPA. + * + * As all pipeline designs are under the unique it must document what signaling + * a buffer of a specific type means for that particular IPA interface. + */ + +/** + * \fn IPAInterface::queueRequest() + * \brief Inform the IPA that a request have been queued to the pipeline + * \param[in] frame The frame number for the request + * \param[in] controls List of controls associated with the request + * + * This function is called by a pipeline handler when it has a request to + * process. The pipeline informs the IPA that \a frame shall be processed + * with the parameters in \a controls. + * + * The IPA may act on this by emitting different signals in the IPA interface + * to configure the pipeline to act on \a frame according to the \a controls. + */ + +/** + * \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 frame number the settings should be + * ready for and a list of controls to modify is passed as parameters. + */ + +/** + * \var IPAInterface::queueBuffer + * \brief Signal emitted when the IPA wish to queue a buffer to the hardware + * + * This signal is emitted then the IPA wish to queue q buffer to the hardware. + * The frame number and buffer type and id are passed as parameters. + */ + +/** + * \var IPAInterface::setDelay + * \brief Signal emitted when the IPA wish to set a delay in the pipeline + * + * This signal is emitted when the IPA wish to change a delay inside the + * pipeline timeline. The action type, frame and time offsets are passed as + * parameters. + * + * The unit for the time offset is not fixed in the IPA interface, it is up to + * the pipeline implementation to choose a suitable unit for its use-case. + */ + +/** + * \var IPAInterface::metaDataReady + * \brief Signal emitted when the IPA is done processing statistics + * + * This signal is emitted when the IPA have finished processing the statistics + * buffer and have created an IPAMetaData object which are ready to be consumed + * by the pipeline handler. The frame number and the meta data is passed as + * parameters. + */ + } /* namespace libcamera */ diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp index 62fcb529e1c7e4ff..17d09fb21582376d 100644 --- a/src/libcamera/proxy/ipa_proxy_linux.cpp +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp @@ -26,7 +26,11 @@ public: IPAProxyLinux(IPAModule *ipam); ~IPAProxyLinux(); - int init(); + int init() override { return 0; } + void initSensor(const V4L2ControlInfoMap &controls) override {} + void initBuffers(unsigned int type, const std::vector<BufferMemory> &buffers) override {} + void signalBuffer(unsigned int type, unsigned int id) override {} + void queueRequest(unsigned int frame, const ControlList &controls) override {} private: void readyRead(IPCUnixSocket *ipc); @@ -36,13 +40,6 @@ private: IPCUnixSocket *socket_; }; -int IPAProxyLinux::init() -{ - LOG(IPAProxy, Debug) << "initializing IPA via dummy proxy!"; - - return 0; -} - IPAProxyLinux::IPAProxyLinux(IPAModule *ipam) : proc_(nullptr), socket_(nullptr) {
The IPA interface needs to support interactions with the pipeline, add interfaces to control the sensor and handling of request ISP parameters and statistics. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- include/ipa/ipa_interface.h | 21 +++++ src/ipa/ipa_dummy.cpp | 6 +- src/libcamera/ipa_interface.cpp | 106 ++++++++++++++++++++++++ src/libcamera/proxy/ipa_proxy_linux.cpp | 13 ++- 4 files changed, 137 insertions(+), 9 deletions(-)