[libcamera-devel] [PATCH v3 03/19] ipa: ipu3: Document the IPAIPU3 class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 26 11:13:08 CEST 2021
Hi Jean-Michel,
On Tue, Oct 26, 2021 at 07:49:19AM +0200, Jean-Michel Hautbois wrote:
> On 25/10/2021 22:05, Laurent Pinchart wrote:
> > On Fri, Oct 22, 2021 at 05:12:02PM +0200, Jean-Michel Hautbois wrote:
> >> Clarify the roles and interactions between the pipeline handler events
> >> and the algorithm calls by documenting all the remaining functions of
> >> the class.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >> ---
> >> src/ipa/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++++++++++++------
> >> 1 file changed, 73 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index 6e28a5aa..c5461d8e 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -301,9 +301,9 @@ private:
> >> struct IPAContext context_;
> >> };
> >>
> >> -/*
> >> - * Compute IPASessionConfiguration using the sensor information and the sensor
> >> - * v4l2 controls.
> >> +/**
> >> + * \brief Compute IPASessionConfiguration using the sensor information and the
> >> + * sensor v4l2 controls
> >
> > s/v4l2/V4L2/
> >
> >> */
> >> void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,
> >> const ControlInfoMap &sensorControls)
> >> @@ -332,9 +332,9 @@ void IPAIPU3::updateSessionConfiguration(const IPACameraSensorInfo &sensorInfo,
> >> context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
> >> }
> >>
> >> -/*
> >> - * Compute camera controls using the sensor information and the sensor
> >> - * v4l2 controls.
> >> +/**
> >> + * \brief Compute camera controls using the sensor information and the sensor
> >> + * v4l2 controls
> >
> > Ditto.
> >
> >> *
> >> * Some of the camera controls are computed by the pipeline handler, some others
> >> * by the IPA module which is in charge of handling, for example, the exposure
> >> @@ -395,7 +395,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> >> }
> >>
> >> /**
> >> - * Initialize the IPA module and its controls.
> >> + * \brief Initialize the IPA module and its controls
> >> *
> >> * This function receives the camera sensor information from the pipeline
> >> * handler, computes the limits of the controls it handles and returns
> >> @@ -426,8 +426,15 @@ int IPAIPU3::init(const IPASettings &settings,
> >> return 0;
> >> }
> >>
> >> +/**
> >> + * \brief Perform any processing required before the first frame
> >> + */
> >> int IPAIPU3::start()
> >> {
> >> + /*
> >> + * Set the sensors V4L2 controls before the first frame to ensure that
> >> + * we have an expected and known configuration from the start.
> >> + */
> >> setControls(0);
> >>
> >> return 0;
> >> @@ -581,6 +588,10 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
> >> return 0;
> >> }
> >>
> >> +/**
> >> + * \brief Map the parameters and stats buffers allocated in the Pipeline Handler
> >> + * \param[in] buffers The buffers to map
> >> + */
> >> void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
> >> {
> >> for (const IPABuffer &buffer : buffers) {
> >> @@ -590,6 +601,10 @@ void IPAIPU3::mapBuffers(const std::vector<IPABuffer> &buffers)
> >> }
> >> }
> >>
> >> +/**
> >> + * \brief Unmap the parameters and stats buffers
> >> + * \param[in] ids The IDs of the buffers to unmap
> >> + */
> >> void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
> >> {
> >> for (unsigned int id : ids) {
> >> @@ -601,6 +616,10 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
> >> }
> >> }
> >>
> >> +/**
> >> + * \brief Process an event generated by the Pipeline Handler
> >> + * \param[in] event The event sent from Pipeline Handler
> >> + */
> >> void IPAIPU3::processEvent(const IPU3Event &event)
> >> {
> >> switch (event.op) {
> >> @@ -642,12 +661,26 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> >> }
> >> }
> >>
> >> +/**
> >> + * \brief Process a control list for a request from the application
> >> + *
> >> + * Parse the request to handle any IPA managed controls that were set from the
> >
> > My comments on v1 for the next 4 functions seem to have been ignored.
> > I'll review v4.
>
> quoting you on v1 (sorry to have missed it):
> "Is it the current frame number or the next frame number ? We'll
> probably have to document "current" and "next" at some point, when we'll
> match per-frame controls with the corresponding frame."
>
> It is the next frame that will be received on cio2 side, and fillParams
> and parseStatistics will have the current 'output' frame number.
>
> The logs look like:
>
> [11818] DEBUG IPAIPU3 ipu3.cpp:560 processControls: frame 0
> [11818] DEBUG IPAIPU3 ipu3.cpp:560 processControls: frame 1
> [11818] DEBUG IPAIPU3 ipu3.cpp:560 processControls: frame 2
> [11818] DEBUG IPAIPU3 ipu3.cpp:560 processControls: frame 3
> [11818] DEBUG IPAIPU3 ipu3.cpp:573 fillParams: frame 0
> [11818] DEBUG IPAIPU3 ipu3.cpp:608 parseStatistics: frame 0
> [11818] DEBUG IPAIPU3 ipu3.cpp:560 processControls: frame 4
> [11818] DEBUG IPAIPU3 ipu3.cpp:573 fillParams: frame 1
> [11818] DEBUG IPAIPU3 ipu3.cpp:608 parseStatistics: frame 1
> etc.
>
> This is where the IPAFrameContext could be useful probably if it was a
> queue and not the latest frame only, as we could have the
> delayedControls received for frame 3 when we are processing frame 3.
> This is where I am right now with per-frame controls btw, I can't see
> another way to do it correctly :-).
Moving the frame context to a queue sounds good to me.
> Which means that for this function, frame is the next frame to be
> processed, for parseStatistics it is the latest frame processed, and for
> fillParams... Well technically it is also the latest frame processed but
> it applies to the next frame... ?
OK, thanks for the clarification.
BTW, it seems you're missing the parameters documentation for this
function (I can't recall if they're added in a patch later in the
series).
> >> + * application such as manual sensor settings.
> >> + */
> >> void IPAIPU3::processControls([[maybe_unused]] unsigned int frame,
> >> [[maybe_unused]] const ControlList &controls)
> >> {
> >> /* \todo Start processing for 'frame' based on 'controls'. */
> >> }
> >>
> >> +/**
> >> + * \brief Fill the ImgU parameter buffer for the next frame
> >> + * \param[in] frame The current frame number
> >> + * \param[inout] params the parameter buffer to update
> >> + *
> >> + * Algorithms are expected to fill the IPU3 parameter buffer for the next
> >> + * frame given their most recent processing of the ImgU statistics.
> >> + */
> >> void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> >> {
> >> /*
> >> @@ -670,6 +703,16 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> >> queueFrameAction.emit(frame, op);
> >> }
> >>
> >> +/**
> >> + * \brief Process the statistics generated by the ImgU
> >> + * \param[in] frame The current frame number
> >> + * \param[in] frameTimestamp The current frame timestamp
> >> + * \param[in] stats The IPU3 statistics and ISP results
> >> + *
> >> + * Parse the most recently processed image statistics from the ImgU. The
> >> + * statistics are passed to each algorithm module to run their calculations and
> >> + * update their state accordingly.
> >> + */
> >> void IPAIPU3::parseStatistics(unsigned int frame,
> >> [[maybe_unused]] int64_t frameTimestamp,
> >> [[maybe_unused]] const ipu3_uapi_stats_3a *stats)
> >> @@ -693,6 +736,13 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> >> queueFrameAction.emit(frame, op);
> >> }
> >>
> >> +/**
> >> + * \brief Handle V4L2 controls for a given \a frame number
> >> + * \param[in] frame The frame on which the V4L2 controls should be set
> >> + *
> >> + * Send the desired sensor control values to the Pipeline handler to request
> >> + * that they are applied on the Camera sensor.
> >> + */
> >> void IPAIPU3::setControls(unsigned int frame)
> >> {
> >> IPU3Action op;
> >> @@ -711,10 +761,15 @@ void IPAIPU3::setControls(unsigned int frame)
> >>
> >> } /* namespace ipa::ipu3 */
> >>
> >> -/*
> >> - * External IPA module interface
> >> +/**
> >> + * \brief External IPA module interface
> >> + *
> >> + * The IPAModuleInfo is required to match an IPA module construction against the
> >> + * intented pipeline handler with the module. The API and Pipeline handler
> >> + * versions must match the corresponding IPA interface and pipeline handler.
> >> + *
> >> + * \sa struct IPAModuleInfo
> >> */
> >> -
> >> extern "C" {
> >> const struct IPAModuleInfo ipaModuleInfo = {
> >> IPA_MODULE_API_VERSION,
> >> @@ -723,6 +778,14 @@ const struct IPAModuleInfo ipaModuleInfo = {
> >> "ipu3",
> >> };
> >>
> >> +/**
> >> + * \brief Create an instance of the IPA interface
> >> + *
> >> + * This function is the entry point of the IPA module. It is called by the IPA
> >> + * manager to create an instance of the IPA interface for each camera. When
> >> + * matched against with a pipeline handler, the IPAManager will construct an IPA
> >> + * instance for each associated Camera.
> >> + */
> >> IPAInterface *ipaCreate()
> >> {
> >> return new ipa::ipu3::IPAIPU3();
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list