[v1,09/35] ipa: rkisp1: Refactor setControls()

24775 diff mbox series

IPARkISP1::setControls() is called when new sensor controls shall be
queued to the pipeline handler. It constructs a list of sensor controls
and then emits the setSensorControls signal.

To be able to return initial sensor controls from the IPARkISP1::start()
function, similar functionality will be needed. Prepare for that by
changing the setControls() function to a getSensorControls() that is
passed a frame context and by moving the setSensorControls.emit() out of
the function.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/libipa/fc_queue.cpp |  6 ++++++
 src/ipa/libipa/fc_queue.h   |  2 ++
 src/ipa/rkisp1/rkisp1.cpp   | 26 +++++++++++++-------------
 3 files changed, 21 insertions(+), 13 deletions(-)

Quoting Stefan Klug (2025-10-24 17:50:33)
> IPARkISP1::setControls() is called when new sensor controls shall be
> queued to the pipeline handler. It constructs a list of sensor controls
> and then emits the setSensorControls signal.
> 
> To be able to return initial sensor controls from the IPARkISP1::start()
> function, similar functionality will be needed. Prepare for that by
> changing the setControls() function to a getSensorControls() that is
> passed a frame context and by moving the setSensorControls.emit() out of
> the function.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Looks good to me.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/libipa/fc_queue.cpp |  6 ++++++
>  src/ipa/libipa/fc_queue.h   |  2 ++
>  src/ipa/rkisp1/rkisp1.cpp   | 26 +++++++++++++-------------
>  3 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp
> index 39222c2ed204..7ba28ed21611 100644
> --- a/src/ipa/libipa/fc_queue.cpp
> +++ b/src/ipa/libipa/fc_queue.cpp
> @@ -38,6 +38,12 @@ namespace ipa {
>   * \brief The frame number
>   */
>  
> +/**
> + * \fn FrameContext::frame()
> + * \brief Get the frame of that frame context
> + * \return THe frame number
> + */
> +
>  /**
>   * \class FCQueue
>   * \brief A support class for managing FrameContext instances in IPA modules
> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h
> index 1128e42f8ca6..812022c496ed 100644
> --- a/src/ipa/libipa/fc_queue.h
> +++ b/src/ipa/libipa/fc_queue.h
> @@ -22,6 +22,8 @@ template<typename FrameContext>
>  class FCQueue;
>  
>  struct FrameContext {
> +       uint32_t frame() const { return frame_; }
> +
>  private:
>         template<typename T> friend class FCQueue;
>         uint32_t frame_;
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index fa22bfc34904..d64c72ecaf4f 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -77,7 +77,7 @@ private:
>         void updateControls(const IPACameraSensorInfo &sensorInfo,
>                             const ControlInfoMap &sensorControls,
>                             ControlInfoMap *ipaControls);
> -       void setControls(unsigned int frame);
> +       ControlList getSensorControls(const IPAFrameContext &context);
>  
>         std::map<unsigned int, FrameBuffer> buffers_;
>         std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
> @@ -380,7 +380,12 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
>                 algo->process(context_, frame, frameContext, stats, metadata);
>         }
>  
> -       setControls(frame);
> +       /*
> +        * \todo: Here we should do a lookahead that takes the sensor delays
> +        * into account.
> +        */
> +       ControlList ctrls = getSensorControls(frameContext);
> +       setSensorControls.emit(frame, ctrls);
>  
>         context_.debugMetadata.moveEntries(metadata);
>         metadataReady.emit(frame, metadata);
> @@ -445,28 +450,23 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,
>         *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);
>  }
>  
> -void IPARkISP1::setControls(unsigned int frame)
> +ControlList IPARkISP1::getSensorControls(const IPAFrameContext &frameContext)
>  {
> -       /*
> -        * \todo The frame number is most likely wrong here, we need to take
> -        * internal sensor delays and other timing parameters into account.
> -        */
> -
> -       IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>         uint32_t exposure = frameContext.agc.exposure;
>         uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain);
>         uint32_t vblank = frameContext.agc.vblank;
>  
>         LOG(IPARkISP1, Debug)
> -               << "Set controls for frame " << frame << ": exposure " << exposure
> -               << ", gain " << frameContext.agc.gain << ", vblank " << vblank;
> +               << "Set controls for frame " << frameContext.frame()
> +               << ": exposure " << exposure
> +               << ", gain " << frameContext.agc.gain
> +               << ", vblank " << vblank;
>  
>         ControlList ctrls(sensorControls_);
>         ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));
>         ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain));
>         ctrls.set(V4L2_CID_VBLANK, static_cast<int32_t>(vblank));
> -
> -       setSensorControls.emit(frame, ctrls);
> +       return ctrls;
>  }
>  
>  } /* namespace ipa::rkisp1 */
> -- 
> 2.48.1
>