| Message ID | 20211111104908.295992-5-hanlinchen@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Han-Lin Chen (2021-11-11 10:49:06) > Apply auto focus and send lens controls to pipeline handler. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > --- > aiq/aiq.cpp | 3 +-- > aiq/aiq.h | 4 +--- > aiq/aiq_input_parameters.cpp | 2 +- > ipu3.cpp | 26 ++++++++++++++++++++++++++ > 4 files changed, 29 insertions(+), 6 deletions(-) > > diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp > index 24c61cb..52a9c07 100644 > --- a/aiq/aiq.cpp > +++ b/aiq/aiq.cpp > @@ -138,8 +138,7 @@ int AIQ::setStatistics(unsigned int frame, > * might run asycnronously, or after receipt of statistics, with the filling > * of the parameter buffer being the only part handled when called for. > */ > -int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, > - AiqResults &results) > +int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results) > { > (void)frame; > > diff --git a/aiq/aiq.h b/aiq/aiq.h > index fcd02d2..49eee2d 100644 > --- a/aiq/aiq.h > +++ b/aiq/aiq.h > @@ -39,9 +39,7 @@ public: > int setStatistics(unsigned int frame, > int64_t timestamp, AiqResults &results, > const ipu3_uapi_stats_3a *stats); > - > - int run2a(unsigned int frame, AiqInputParameters ¶ms, > - AiqResults &results); > + int run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results); > > private: > std::string decodeError(ia_err err); > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp > index 46553a6..5dd2f6c 100644 > --- a/aiq/aiq_input_parameters.cpp > +++ b/aiq/aiq_input_parameters.cpp > @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults() > ia_aiq_af_range_normal, > ia_aiq_af_metering_mode_auto, > ia_aiq_flash_mode_off, > - NULL, NULL, false > + &focusRect, &manualFocusParams, false > }; > > /* GBCE Params */ > diff --git a/ipu3.cpp b/ipu3.cpp > index 9d07268..f38440d 100644 > --- a/ipu3.cpp > +++ b/ipu3.cpp > @@ -77,6 +77,10 @@ private: > uint32_t gain_; > uint32_t minGain_; > uint32_t maxGain_; > + int32_t lensPosition_; > + > + /* Intel AF library relies on timestamp to wait for lens movement */ > + uint64_t lensMovementStartTime_; > > /* Intel Library Instances. */ > aiq::AIQ aiq_; > @@ -257,6 +261,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, > maxGain_ = itGain->second.max().get<int32_t>(); > gain_ = maxGain_; > > + lensMovementStartTime_ = 0; > + lensPosition_ = 0; > + > int ret; > > ret = aiq_.configure(); > @@ -383,6 +390,10 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > resultsHistory_.extendOne(); > aiq::AiqResults& latestResults = resultsHistory_.latest(); > > + /* Todo: Refactor AiqInputParameters interface to set following parameters. */ > + aiqInputParams_.afParams.lens_position = lensPosition_; > + aiqInputParams_.afParams.lens_movement_start_timestamp = lensMovementStartTime_; > + > aiq_.run2a(frame, aiqInputParams_, latestResults); > aic_.updateRuntimeParams(latestResults); > aic_.run(params); > @@ -390,6 +401,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > exposure_ = latestResults.ae()->exposures[0].sensor_exposure->coarse_integration_time; > gain_ = latestResults.ae()->exposures[0].sensor_exposure->analog_gain_code_global; > > + /* > + * Af algorithm compares the timestamp of start of lens movement and the > + * that of the statistics generated to estimate whether next lens > + * positon should be produced. /positon/position/ > + * Todo: Use the lens movement start time reported by the pipeline handler. Is this the time the control is set? or something that's expected to be identified from the VCM? > + */ > + if (lensPosition_ != latestResults.af()->next_lens_position) { > + utils::time_point time = utils::clock::now(); > + uint64_t msecs = std::chrono::duration_cast<std::chrono::microseconds>(time.time_since_epoch()).count(); > + lensMovementStartTime_ = msecs; > + } > + lensPosition_ = latestResults.af()->next_lens_position; > + > setControls(frame); > > IPU3Action op; > @@ -473,6 +497,8 @@ void IPAIPU3::setControls(unsigned int frame) > > op.sensorControls = sensorCtrls; > > + op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_); > + This one is going to need the corresponding update in libcamera of course. Perhaps merging the interface change is possible before/while we sort out the other parts, as that's distinct and could unblock this patch. But when this can compile: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > queueFrameAction.emit(frame, op); > } > > -- > 2.34.0.rc1.387.gb447b232ab-goog >
Quoting Kieran Bingham (2021-11-17 17:58:00) > Quoting Han-Lin Chen (2021-11-11 10:49:06) > > Apply auto focus and send lens controls to pipeline handler. > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > > --- > > aiq/aiq.cpp | 3 +-- > > aiq/aiq.h | 4 +--- > > aiq/aiq_input_parameters.cpp | 2 +- > > ipu3.cpp | 26 ++++++++++++++++++++++++++ > > 4 files changed, 29 insertions(+), 6 deletions(-) > > > > diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp > > index 24c61cb..52a9c07 100644 > > --- a/aiq/aiq.cpp > > +++ b/aiq/aiq.cpp > > @@ -138,8 +138,7 @@ int AIQ::setStatistics(unsigned int frame, > > * might run asycnronously, or after receipt of statistics, with the filling > > * of the parameter buffer being the only part handled when called for. > > */ > > -int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, > > - AiqResults &results) > > +int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results) > > { > > (void)frame; > > > > diff --git a/aiq/aiq.h b/aiq/aiq.h > > index fcd02d2..49eee2d 100644 > > --- a/aiq/aiq.h > > +++ b/aiq/aiq.h > > @@ -39,9 +39,7 @@ public: > > int setStatistics(unsigned int frame, > > int64_t timestamp, AiqResults &results, > > const ipu3_uapi_stats_3a *stats); > > - > > - int run2a(unsigned int frame, AiqInputParameters ¶ms, > > - AiqResults &results); > > + int run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results); > > > > private: > > std::string decodeError(ia_err err); > > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp > > index 46553a6..5dd2f6c 100644 > > --- a/aiq/aiq_input_parameters.cpp > > +++ b/aiq/aiq_input_parameters.cpp > > @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults() > > ia_aiq_af_range_normal, > > ia_aiq_af_metering_mode_auto, > > ia_aiq_flash_mode_off, > > - NULL, NULL, false > > + &focusRect, &manualFocusParams, false > > }; > > > > /* GBCE Params */ > > diff --git a/ipu3.cpp b/ipu3.cpp > > index 9d07268..f38440d 100644 > > --- a/ipu3.cpp > > +++ b/ipu3.cpp > > @@ -77,6 +77,10 @@ private: > > uint32_t gain_; > > uint32_t minGain_; > > uint32_t maxGain_; > > + int32_t lensPosition_; > > + > > + /* Intel AF library relies on timestamp to wait for lens movement */ > > + uint64_t lensMovementStartTime_; > > > > /* Intel Library Instances. */ > > aiq::AIQ aiq_; > > @@ -257,6 +261,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, > > maxGain_ = itGain->second.max().get<int32_t>(); > > gain_ = maxGain_; > > > > + lensMovementStartTime_ = 0; > > + lensPosition_ = 0; > > + > > int ret; > > > > ret = aiq_.configure(); > > @@ -383,6 +390,10 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > > resultsHistory_.extendOne(); > > aiq::AiqResults& latestResults = resultsHistory_.latest(); > > > > + /* Todo: Refactor AiqInputParameters interface to set following parameters. */ > > + aiqInputParams_.afParams.lens_position = lensPosition_; > > + aiqInputParams_.afParams.lens_movement_start_timestamp = lensMovementStartTime_; > > + > > aiq_.run2a(frame, aiqInputParams_, latestResults); > > aic_.updateRuntimeParams(latestResults); > > aic_.run(params); > > @@ -390,6 +401,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > > exposure_ = latestResults.ae()->exposures[0].sensor_exposure->coarse_integration_time; > > gain_ = latestResults.ae()->exposures[0].sensor_exposure->analog_gain_code_global; > > > > + /* > > + * Af algorithm compares the timestamp of start of lens movement and the > > + * that of the statistics generated to estimate whether next lens > > + * positon should be produced. > > /positon/position/ > > > + * Todo: Use the lens movement start time reported by the pipeline handler. > > Is this the time the control is set? or something that's expected to be > identified from the VCM? > > > + */ > > + if (lensPosition_ != latestResults.af()->next_lens_position) { > > + utils::time_point time = utils::clock::now(); > > + uint64_t msecs = std::chrono::duration_cast<std::chrono::microseconds>(time.time_since_epoch()).count(); > > + lensMovementStartTime_ = msecs; Small note here. microseconds is usually referred to as us, (usecs?) ... msecs to me would imply milli-seconds...? The defintiion and use are closely grouped here, so it's clear eitherway, but thought it was worth mentioning here. > > + } > > + lensPosition_ = latestResults.af()->next_lens_position; > > + > > setControls(frame); > > > > IPU3Action op; > > @@ -473,6 +497,8 @@ void IPAIPU3::setControls(unsigned int frame) > > > > op.sensorControls = sensorCtrls; > > > > + op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_); > > + > > This one is going to need the corresponding update in libcamera of > course. > > Perhaps merging the interface change is possible before/while we sort > out the other parts, as that's distinct and could unblock this patch. > > But when this can compile: I'll merge this series with the exception of these two commits: ipu3: Apply auto focus and send lens controls to pipeline handler ipu3: Run AIQ for the first frame to avoid blanking As they require the lens updates on the libcamera side. "Run AIQ..." doesn't, but it seems to need to be on top of the other patch, and I can't cherry-pick it cleanly, (which I was able to do with ipu3: Use gbce and tone mapping from the tuning files) so I'll leave it out for now. Your patch queue should be easier to handle with those down anyway. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > queueFrameAction.emit(frame, op); > > } > > > > -- > > 2.34.0.rc1.387.gb447b232ab-goog > >
Hi Han-Lin, Quoting Kieran Bingham (2021-11-19 16:36:11) > Quoting Kieran Bingham (2021-11-17 17:58:00) > > Quoting Han-Lin Chen (2021-11-11 10:49:06) > > > Apply auto focus and send lens controls to pipeline handler. > > > > > I'll merge this series with the exception of these two commits: > > ipu3: Apply auto focus and send lens controls to pipeline handler > ipu3: Run AIQ for the first frame to avoid blanking > > As they require the lens updates on the libcamera side. "Run AIQ..." > doesn't, but it seems to need to be on top of the other patch, and I > can't cherry-pick it cleanly, (which I was able to do with ipu3: Use > gbce and tone mapping from the tuning files) so I'll leave it out for > now. > > Your patch queue should be easier to handle with those down anyway. I've merged this series with the exception of: ipu3: Apply auto focus and send lens controls to pipeline handler That needs the corresponding updates in libcamera to be accepted first. The AIQ was easy to fix the conflicts when cherry-picking, and has been tested on the Soraka with the latest libcamera master branch. -- Kieran
Hi Kieran, Many thanks for the help. On Sat, Nov 20, 2021 at 12:36 AM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Kieran Bingham (2021-11-17 17:58:00) > > Quoting Han-Lin Chen (2021-11-11 10:49:06) > > > Apply auto focus and send lens controls to pipeline handler. > > > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > > > --- > > > aiq/aiq.cpp | 3 +-- > > > aiq/aiq.h | 4 +--- > > > aiq/aiq_input_parameters.cpp | 2 +- > > > ipu3.cpp | 26 ++++++++++++++++++++++++++ > > > 4 files changed, 29 insertions(+), 6 deletions(-) > > > > > > diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp > > > index 24c61cb..52a9c07 100644 > > > --- a/aiq/aiq.cpp > > > +++ b/aiq/aiq.cpp > > > @@ -138,8 +138,7 @@ int AIQ::setStatistics(unsigned int frame, > > > * might run asycnronously, or after receipt of statistics, with the filling > > > * of the parameter buffer being the only part handled when called for. > > > */ > > > -int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, > > > - AiqResults &results) > > > +int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results) > > > { > > > (void)frame; > > > > > > diff --git a/aiq/aiq.h b/aiq/aiq.h > > > index fcd02d2..49eee2d 100644 > > > --- a/aiq/aiq.h > > > +++ b/aiq/aiq.h > > > @@ -39,9 +39,7 @@ public: > > > int setStatistics(unsigned int frame, > > > int64_t timestamp, AiqResults &results, > > > const ipu3_uapi_stats_3a *stats); > > > - > > > - int run2a(unsigned int frame, AiqInputParameters ¶ms, > > > - AiqResults &results); > > > + int run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results); > > > > > > private: > > > std::string decodeError(ia_err err); > > > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp > > > index 46553a6..5dd2f6c 100644 > > > --- a/aiq/aiq_input_parameters.cpp > > > +++ b/aiq/aiq_input_parameters.cpp > > > @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults() > > > ia_aiq_af_range_normal, > > > ia_aiq_af_metering_mode_auto, > > > ia_aiq_flash_mode_off, > > > - NULL, NULL, false > > > + &focusRect, &manualFocusParams, false > > > }; > > > > > > /* GBCE Params */ > > > diff --git a/ipu3.cpp b/ipu3.cpp > > > index 9d07268..f38440d 100644 > > > --- a/ipu3.cpp > > > +++ b/ipu3.cpp > > > @@ -77,6 +77,10 @@ private: > > > uint32_t gain_; > > > uint32_t minGain_; > > > uint32_t maxGain_; > > > + int32_t lensPosition_; > > > + > > > + /* Intel AF library relies on timestamp to wait for lens movement */ > > > + uint64_t lensMovementStartTime_; > > > > > > /* Intel Library Instances. */ > > > aiq::AIQ aiq_; > > > @@ -257,6 +261,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, > > > maxGain_ = itGain->second.max().get<int32_t>(); > > > gain_ = maxGain_; > > > > > > + lensMovementStartTime_ = 0; > > > + lensPosition_ = 0; > > > + > > > int ret; > > > > > > ret = aiq_.configure(); > > > @@ -383,6 +390,10 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > > > resultsHistory_.extendOne(); > > > aiq::AiqResults& latestResults = resultsHistory_.latest(); > > > > > > + /* Todo: Refactor AiqInputParameters interface to set following parameters. */ > > > + aiqInputParams_.afParams.lens_position = lensPosition_; > > > + aiqInputParams_.afParams.lens_movement_start_timestamp = lensMovementStartTime_; > > > + > > > aiq_.run2a(frame, aiqInputParams_, latestResults); > > > aic_.updateRuntimeParams(latestResults); > > > aic_.run(params); > > > @@ -390,6 +401,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) > > > exposure_ = latestResults.ae()->exposures[0].sensor_exposure->coarse_integration_time; > > > gain_ = latestResults.ae()->exposures[0].sensor_exposure->analog_gain_code_global; > > > > > > + /* > > > + * Af algorithm compares the timestamp of start of lens movement and the > > > + * that of the statistics generated to estimate whether next lens > > > + * positon should be produced. > > > > /positon/position/ > > > > > + * Todo: Use the lens movement start time reported by the pipeline handler. > > > > Is this the time the control is set? or something that's expected to be > > identified from the VCM? It's the time the control is set into the device. I cannot find extension of the V4L2 which reports it from the VCM. > > > > > + */ > > > + if (lensPosition_ != latestResults.af()->next_lens_position) { > > > + utils::time_point time = utils::clock::now(); > > > + uint64_t msecs = std::chrono::duration_cast<std::chrono::microseconds>(time.time_since_epoch()).count(); > > > + lensMovementStartTime_ = msecs; > > Small note here. microseconds is usually referred to as us, (usecs?) ... > msecs to me would imply milli-seconds...? You're right. I was always confused with the time abbreviations. :-| > > The defintiion and use are closely grouped here, so it's clear > eitherway, but thought it was worth mentioning here. > > > > > > + } > > > + lensPosition_ = latestResults.af()->next_lens_position; > > > + > > > setControls(frame); > > > > > > IPU3Action op; > > > @@ -473,6 +497,8 @@ void IPAIPU3::setControls(unsigned int frame) > > > > > > op.sensorControls = sensorCtrls; > > > > > > + op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_); > > > + > > > > This one is going to need the corresponding update in libcamera of > > course. > > > > Perhaps merging the interface change is possible before/while we sort > > out the other parts, as that's distinct and could unblock this patch. > > > > But when this can compile: > > I'll merge this series with the exception of these two commits: > > ipu3: Apply auto focus and send lens controls to pipeline handler > ipu3: Run AIQ for the first frame to avoid blanking > > As they require the lens updates on the libcamera side. "Run AIQ..." > doesn't, but it seems to need to be on top of the other patch, and I > can't cherry-pick it cleanly, (which I was able to do with ipu3: Use > gbce and tone mapping from the tuning files) so I'll leave it out for > now. > > Your patch queue should be easier to handle with those down anyway. > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > queueFrameAction.emit(frame, op); > > > } > > > > > > -- > > > 2.34.0.rc1.387.gb447b232ab-goog > > >
diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp index 24c61cb..52a9c07 100644 --- a/aiq/aiq.cpp +++ b/aiq/aiq.cpp @@ -138,8 +138,7 @@ int AIQ::setStatistics(unsigned int frame, * might run asycnronously, or after receipt of statistics, with the filling * of the parameter buffer being the only part handled when called for. */ -int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, - AiqResults &results) +int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results) { (void)frame; diff --git a/aiq/aiq.h b/aiq/aiq.h index fcd02d2..49eee2d 100644 --- a/aiq/aiq.h +++ b/aiq/aiq.h @@ -39,9 +39,7 @@ public: int setStatistics(unsigned int frame, int64_t timestamp, AiqResults &results, const ipu3_uapi_stats_3a *stats); - - int run2a(unsigned int frame, AiqInputParameters ¶ms, - AiqResults &results); + int run2a(unsigned int frame, AiqInputParameters ¶ms, AiqResults &results); private: std::string decodeError(ia_err err); diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp index 46553a6..5dd2f6c 100644 --- a/aiq/aiq_input_parameters.cpp +++ b/aiq/aiq_input_parameters.cpp @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults() ia_aiq_af_range_normal, ia_aiq_af_metering_mode_auto, ia_aiq_flash_mode_off, - NULL, NULL, false + &focusRect, &manualFocusParams, false }; /* GBCE Params */ diff --git a/ipu3.cpp b/ipu3.cpp index 9d07268..f38440d 100644 --- a/ipu3.cpp +++ b/ipu3.cpp @@ -77,6 +77,10 @@ private: uint32_t gain_; uint32_t minGain_; uint32_t maxGain_; + int32_t lensPosition_; + + /* Intel AF library relies on timestamp to wait for lens movement */ + uint64_t lensMovementStartTime_; /* Intel Library Instances. */ aiq::AIQ aiq_; @@ -257,6 +261,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, maxGain_ = itGain->second.max().get<int32_t>(); gain_ = maxGain_; + lensMovementStartTime_ = 0; + lensPosition_ = 0; + int ret; ret = aiq_.configure(); @@ -383,6 +390,10 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) resultsHistory_.extendOne(); aiq::AiqResults& latestResults = resultsHistory_.latest(); + /* Todo: Refactor AiqInputParameters interface to set following parameters. */ + aiqInputParams_.afParams.lens_position = lensPosition_; + aiqInputParams_.afParams.lens_movement_start_timestamp = lensMovementStartTime_; + aiq_.run2a(frame, aiqInputParams_, latestResults); aic_.updateRuntimeParams(latestResults); aic_.run(params); @@ -390,6 +401,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params) exposure_ = latestResults.ae()->exposures[0].sensor_exposure->coarse_integration_time; gain_ = latestResults.ae()->exposures[0].sensor_exposure->analog_gain_code_global; + /* + * Af algorithm compares the timestamp of start of lens movement and the + * that of the statistics generated to estimate whether next lens + * positon should be produced. + * Todo: Use the lens movement start time reported by the pipeline handler. + */ + if (lensPosition_ != latestResults.af()->next_lens_position) { + utils::time_point time = utils::clock::now(); + uint64_t msecs = std::chrono::duration_cast<std::chrono::microseconds>(time.time_since_epoch()).count(); + lensMovementStartTime_ = msecs; + } + lensPosition_ = latestResults.af()->next_lens_position; + setControls(frame); IPU3Action op; @@ -473,6 +497,8 @@ void IPAIPU3::setControls(unsigned int frame) op.sensorControls = sensorCtrls; + op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_); + queueFrameAction.emit(frame, op); }
Apply auto focus and send lens controls to pipeline handler. Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> --- aiq/aiq.cpp | 3 +-- aiq/aiq.h | 4 +--- aiq/aiq_input_parameters.cpp | 2 +- ipu3.cpp | 26 ++++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-)