| Message ID | 20241220162724.756494-5-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan, Thank you for the patch. The subject line is incorrect, this patch isn't about the pipeline handler but the IPA module. On Fri, Dec 20, 2024 at 05:26:50PM +0100, Stefan Klug wrote: > IPARkISP1::setControls() is called when new sensor controls shall be > queued in 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() and by moving > the setSensorControls.emit() out of the function. > > This patch contains no functional changes. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/rkisp1/rkisp1.cpp | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 2ffdd99b158a..45a539a61fb1 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -76,7 +76,7 @@ private: > void updateControls(const IPACameraSensorInfo &sensorInfo, > const ControlInfoMap &sensorControls, > ControlInfoMap *ipaControls); > - void setControls(unsigned int frame); > + ControlList getSensorControls(unsigned int frame); > > std::map<unsigned int, FrameBuffer> buffers_; > std::map<unsigned int, MappedFrameBuffer> mappedBuffers_; > @@ -211,7 +211,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, > > int IPARkISP1::start() > { > - setControls(0); > + ControlList ctrls = getSensorControls(0); Will this work fine before patch 5/7 ? Or at least with no regression (such as crashes) ? > + setSensorControls.emit(0, ctrls); > > return 0; > } > @@ -378,7 +379,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 The doxygen tag is '\todo', not '\todo:'. > + * into account. > + */ I'm not sure how accurate that is, but I suppose it's not worse than the existing comment. I'd keep the mention of frame number though. > + ControlList ctrls = getSensorControls(frame); > + setSensorControls.emit(frame, ctrls); > > context_.debugMetadata.moveEntries(metadata); > metadataReady.emit(frame, metadata); > @@ -443,13 +449,8 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, > *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); > } > > -void IPARkISP1::setControls(unsigned int frame) > +ControlList IPARkISP1::getSensorControls(unsigned int frame) > { > - /* > - * \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); > @@ -457,8 +458,7 @@ void IPARkISP1::setControls(unsigned int frame) > ControlList ctrls(sensorControls_); > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure)); > ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain)); > - > - setSensorControls.emit(frame, ctrls); > + return ctrls; > } > > } /* namespace ipa::rkisp1 */
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 2ffdd99b158a..45a539a61fb1 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -76,7 +76,7 @@ private: void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); - void setControls(unsigned int frame); + ControlList getSensorControls(unsigned int frame); std::map<unsigned int, FrameBuffer> buffers_; std::map<unsigned int, MappedFrameBuffer> mappedBuffers_; @@ -211,7 +211,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, int IPARkISP1::start() { - setControls(0); + ControlList ctrls = getSensorControls(0); + setSensorControls.emit(0, ctrls); return 0; } @@ -378,7 +379,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(frame); + setSensorControls.emit(frame, ctrls); context_.debugMetadata.moveEntries(metadata); metadataReady.emit(frame, metadata); @@ -443,13 +449,8 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); } -void IPARkISP1::setControls(unsigned int frame) +ControlList IPARkISP1::getSensorControls(unsigned int frame) { - /* - * \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); @@ -457,8 +458,7 @@ void IPARkISP1::setControls(unsigned int frame) ControlList ctrls(sensorControls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain)); - - setSensorControls.emit(frame, ctrls); + return ctrls; } } /* namespace ipa::rkisp1 */
IPARkISP1::setControls() is called when new sensor controls shall be queued in 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() and by moving the setSensorControls.emit() out of the function. This patch contains no functional changes. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/rkisp1/rkisp1.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)