[{"id":20667,"web_url":"https://patchwork.libcamera.org/comment/20667/","msgid":"<163597818115.1378115.7023055491040883953@Monstersaurus>","date":"2021-11-03T22:23:01","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Han-Lin Chen (2021-10-29 12:59:59)\n> Apply auto focus and send lens controls to pipeline handler.\n> \n> Signed-off-by: Han-Lin Chen \n> ---\n> aiq/aiq.cpp | 5 ++++-\n> aiq/aiq.h | 2 +-\n> aiq/aiq_input_parameters.cpp | 2 +-\n> ipu3.cpp | 27 ++++++++++++++++++++++++++-\n> 4 files changed, 32 insertions(+), 4 deletions(-)\n> \n> diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp\n> index 24c61cb..9c7f9d6 100644\n> --- a/aiq/aiq.cpp\n> +++ b/aiq/aiq.cpp\n> @@ -139,7 +139,7 @@ int AIQ::setStatistics(unsigned int frame,\n> * of the parameter buffer being the only part handled when called for.\n> */\n> int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> - AiqResults &results)\n> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime)\n\nIndentation changed for the worse there I think.\n\nDo we really need to pass in individual parameters through the run\nfunction though? I don't think that's sustainable - as we'll likely have\nto pass in more parameters.\n\nIs the lens position / start time something that would already be\nexpected to be passed in therou the AiqInputParametners? Or would we\nneed to define a new parameter structure to pass into the run function?\n\n\n\n> {\n> (void)frame;\n> \n> @@ -157,6 +157,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> params.saParams.awb_results = results.awb();\n> shadingAdapterRun(params.saParams, results);\n> \n> + params.afParams.lens_position = lensPosition;\n> + params.afParams.lens_movement_start_timestamp = lensMovementStartTime;\n> +\n> afRun(params.afParams, results);\n> \n> return 0;\n> diff --git a/aiq/aiq.h b/aiq/aiq.h\n> index fcd02d2..4f5f868 100644\n> --- a/aiq/aiq.h\n> +++ b/aiq/aiq.h\n> @@ -41,7 +41,7 @@ public:\n> const ipu3_uapi_stats_3a *stats);\n> \n> int run2a(unsigned int frame, AiqInputParameters ¶ms,\n> - AiqResults &results);\n> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime);\n> \n> private:\n> std::string decodeError(ia_err err);\n> diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp\n> index 46553a6..5dd2f6c 100644\n> --- a/aiq/aiq_input_parameters.cpp\n> +++ b/aiq/aiq_input_parameters.cpp\n> @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults()\n> ia_aiq_af_range_normal,\n> ia_aiq_af_metering_mode_auto,\n> ia_aiq_flash_mode_off,\n> - NULL, NULL, false\n> + &focusRect, &manualFocusParams, false\n> };\n> \n> /* GBCE Params */\n> diff --git a/ipu3.cpp b/ipu3.cpp\n> index f463805..c2dc754 100644\n> --- a/ipu3.cpp\n> +++ b/ipu3.cpp\n> @@ -77,6 +77,10 @@ private:\n> uint32_t gain_;\n> uint32_t minGain_;\n> uint32_t maxGain_;\n> + int32_t lensPosition_;\n> +\n> + /* Intel AF library relies on timestamp to wait for lens movement */\n> + uint64_t lensMovementStartTime_;\n> \n> /* Intel Library Instances. */\n> aiq::AIQ aiq_;\n> @@ -258,6 +262,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> maxGain_ = itGain->second.max().get();\n> gain_ = maxGain_;\n> \n> + lensMovementStartTime_ = 0;\n> + lensPosition_ = 0;\n> +\n> int ret;\n> \n> ret = aiq_.configure();\n> @@ -381,7 +388,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> */\n> \n> /* Run algorithms into/using this context structure */\n> - aiq_.run2a(frame, aiqInputParams_, results_);\n> + aiq_.run2a(frame, aiqInputParams_, results_, lensPosition_, lensMovementStartTime_);\n> \n> aic_.updateRuntimeParams(results_);\n> aic_.run(params);\n> @@ -389,6 +396,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;\n> gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;\n> \n> + /*\n> + * Af algorithm compares the timestamp of start of lens movement and the\n> + * that of the statistics generated to estimate whether next lens\n> + * positon should be produced.\n> + * Todo: Use the lens movement start time reported by the pipeline handler.\n> + */\n> + if (lensPosition_ != results_.af()->next_lens_position) {\n> + utils::time_point time = utils::clock::now();\n> + uint64_t msecs = std::chrono::duration_cast(time.time_since_epoch()).count();\n> + lensMovementStartTime_ = msecs;\n> + }\n> + lensPosition_ = results_.af()->next_lens_position;\n> +\n> resultsHistory_.Push(results_);\n> \n> setControls(frame);\n> @@ -472,6 +492,11 @@ void IPAIPU3::setControls(unsigned int frame)\n> \n> op.sensorControls = sensorCtrls;\n> \n> + ControlList lensCtrls;\n> + lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> +\n> + op.lensControls = lensCtrls;\n\nDoes this need to create a ControlList just to copy/assign it?\nCan we call\n\n\t op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n\ndirectly?\n\n\n> +\n> queueFrameAction.emit(frame, op);\n> }\n> \n> -- \n> 2.33.1.1089.g2158813163f-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9FF53BF415\n\tfor ;\n\tWed, 3 Nov 2021 22:23:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF7E56033A;\n\tWed, 3 Nov 2021 23:23:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1D4B600B5\n\tfor ;\n\tWed, 3 Nov 2021 23:23:03 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7775F501;\n\tWed, 3 Nov 2021 23:23:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RellEfZ9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635978183;\n\tbh=8uHOOxVvR7pFxSUCszQWYIT71PZSP4mEkQA+cGcv4Gs=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=RellEfZ9onZVw2+zWEeUx7hGUsvSdW0knRUHmHnFQnJX5XS3WdWkMHDkIhTflIMLy\n\txeX1hJcLq8ezCITugCbtDJmtWR2OjMBje3e8XNtq4koDVwTk8CNFvWqc0LsQogoG9m\n\tI+UGRPUAccHi3JKfhQHwt5Q76vpSaTEhxP4o2J9w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211029120001.2469018-4-hanlinchen@chromium.org>","References":"<20211029120001.2469018-1-hanlinchen@chromium.org>\n\t<20211029120001.2469018-4-hanlinchen@chromium.org>","From":"Kieran Bingham ","To":"Han-Lin Chen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 03 Nov 2021 22:23:01 +0000","Message-ID":"<163597818115.1378115.7023055491040883953@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20808,"web_url":"https://patchwork.libcamera.org/comment/20808/","msgid":"","date":"2021-11-10T13:35:34","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Kieran,\n\nOn Thu, Nov 4, 2021 at 6:23 AM Kieran Bingham\n wrote:\n>\n> Quoting Han-Lin Chen (2021-10-29 12:59:59)\n> > Apply auto focus and send lens controls to pipeline handler.\n> >\n> > Signed-off-by: Han-Lin Chen \n> > ---\n> > aiq/aiq.cpp | 5 ++++-\n> > aiq/aiq.h | 2 +-\n> > aiq/aiq_input_parameters.cpp | 2 +-\n> > ipu3.cpp | 27 ++++++++++++++++++++++++++-\n> > 4 files changed, 32 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp\n> > index 24c61cb..9c7f9d6 100644\n> > --- a/aiq/aiq.cpp\n> > +++ b/aiq/aiq.cpp\n> > @@ -139,7 +139,7 @@ int AIQ::setStatistics(unsigned int frame,\n> > * of the parameter buffer being the only part handled when called for.\n> > */\n> > int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> > - AiqResults &results)\n> > + AiqResults &results, int lensPosition, int64_t lensMovementStartTime)\n>\n> Indentation changed for the worse there I think.\n>\n> Do we really need to pass in individual parameters through the run\n> function though? I don't think that's sustainable - as we'll likely have\n> to pass in more parameters.\n>\n> Is the lens position / start time something that would already be\n> expected to be passed in therou the AiqInputParametners? Or would we\n> need to define a new parameter structure to pass into the run function?\nAgreed. I should set it in AiqInputParametners directly.\n>\n>\n>\n> > {\n> > (void)frame;\n> >\n> > @@ -157,6 +157,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> > params.saParams.awb_results = results.awb();\n> > shadingAdapterRun(params.saParams, results);\n> >\n> > + params.afParams.lens_position = lensPosition;\n> > + params.afParams.lens_movement_start_timestamp = lensMovementStartTime;\n> > +\n> > afRun(params.afParams, results);\n> >\n> > return 0;\n> > diff --git a/aiq/aiq.h b/aiq/aiq.h\n> > index fcd02d2..4f5f868 100644\n> > --- a/aiq/aiq.h\n> > +++ b/aiq/aiq.h\n> > @@ -41,7 +41,7 @@ public:\n> > const ipu3_uapi_stats_3a *stats);\n> >\n> > int run2a(unsigned int frame, AiqInputParameters ¶ms,\n> > - AiqResults &results);\n> > + AiqResults &results, int lensPosition, int64_t lensMovementStartTime);\n> >\n> > private:\n> > std::string decodeError(ia_err err);\n> > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp\n> > index 46553a6..5dd2f6c 100644\n> > --- a/aiq/aiq_input_parameters.cpp\n> > +++ b/aiq/aiq_input_parameters.cpp\n> > @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults()\n> > ia_aiq_af_range_normal,\n> > ia_aiq_af_metering_mode_auto,\n> > ia_aiq_flash_mode_off,\n> > - NULL, NULL, false\n> > + &focusRect, &manualFocusParams, false\n> > };\n> >\n> > /* GBCE Params */\n> > diff --git a/ipu3.cpp b/ipu3.cpp\n> > index f463805..c2dc754 100644\n> > --- a/ipu3.cpp\n> > +++ b/ipu3.cpp\n> > @@ -77,6 +77,10 @@ private:\n> > uint32_t gain_;\n> > uint32_t minGain_;\n> > uint32_t maxGain_;\n> > + int32_t lensPosition_;\n> > +\n> > + /* Intel AF library relies on timestamp to wait for lens movement */\n> > + uint64_t lensMovementStartTime_;\n> >\n> > /* Intel Library Instances. */\n> > aiq::AIQ aiq_;\n> > @@ -258,6 +262,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> > maxGain_ = itGain->second.max().get();\n> > gain_ = maxGain_;\n> >\n> > + lensMovementStartTime_ = 0;\n> > + lensPosition_ = 0;\n> > +\n> > int ret;\n> >\n> > ret = aiq_.configure();\n> > @@ -381,7 +388,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> > */\n> >\n> > /* Run algorithms into/using this context structure */\n> > - aiq_.run2a(frame, aiqInputParams_, results_);\n> > + aiq_.run2a(frame, aiqInputParams_, results_, lensPosition_, lensMovementStartTime_);\n> >\n> > aic_.updateRuntimeParams(results_);\n> > aic_.run(params);\n> > @@ -389,6 +396,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> > exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;\n> > gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;\n> >\n> > + /*\n> > + * Af algorithm compares the timestamp of start of lens movement and the\n> > + * that of the statistics generated to estimate whether next lens\n> > + * positon should be produced.\n> > + * Todo: Use the lens movement start time reported by the pipeline handler.\n> > + */\n> > + if (lensPosition_ != results_.af()->next_lens_position) {\n> > + utils::time_point time = utils::clock::now();\n> > + uint64_t msecs = std::chrono::duration_cast(time.time_since_epoch()).count();\n> > + lensMovementStartTime_ = msecs;\n> > + }\n> > + lensPosition_ = results_.af()->next_lens_position;\n> > +\n> > resultsHistory_.Push(results_);\n> >\n> > setControls(frame);\n> > @@ -472,6 +492,11 @@ void IPAIPU3::setControls(unsigned int frame)\n> >\n> > op.sensorControls = sensorCtrls;\n> >\n> > + ControlList lensCtrls;\n> > + lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> > +\n> > + op.lensControls = lensCtrls;\n>\n> Does this need to create a ControlList just to copy/assign it?\n> Can we call\n>\n> op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n>\n> directly?\nIndeed.\n>\n>\n> > +\n> > queueFrameAction.emit(frame, op);\n> > }\n> >\n> > --\n> > 2.33.1.1089.g2158813163f-goog\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E490DBF415\n\tfor ;\n\tWed, 10 Nov 2021 13:35:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4878C6035D;\n\tWed, 10 Nov 2021 14:35:47 +0100 (CET)","from mail-ot1-x32a.google.com (mail-ot1-x32a.google.com\n\t[IPv6:2607:f8b0:4864:20::32a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2CCD16033C\n\tfor ;\n\tWed, 10 Nov 2021 14:35:46 +0100 (CET)","by mail-ot1-x32a.google.com with SMTP id\n\tr10-20020a056830080a00b0055c8fd2cebdso3904689ots.6\n\tfor ;\n\tWed, 10 Nov 2021 05:35:46 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"I56c6YHq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Jx3FE6GCuYFQHIvNOgcCWOXrBb+m1znVRUjiDhFaF24=;\n\tb=I56c6YHqJiU8s01T+jeAVmWBvlWrultwe3b33LZtcf3wpui7ZwaJVSya9ztxv9LxEm\n\tgYbIMQ8dR/U6SLCZwpc9E2+uGC0O6TJQidy7cYbS5z1T5BN/IVGV6bWORA8H0O7RmlPk\n\t7nm2ubeGn6QpAJd8Qq5RxXinA2zY++pPBWTZI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Jx3FE6GCuYFQHIvNOgcCWOXrBb+m1znVRUjiDhFaF24=;\n\tb=mS5QwNlZwGLy2gbWWZKOoq+wVFmBBF2AvwsO6TOvaIkjeU2atPdwQuce6z1Rm/c1Mt\n\tS+9MmCN0W0gyQHRUQpr5g50ouPuA3nkpKchXu+e/dJdzZkhryvrJ99L6+Oa9GI+Jptsc\n\t0uQdKDuGJvMpVPn8v4JckJvia51lE+2+NsYV9D3p9TmKdL8ioENcSusmLJnKH1d7mMtr\n\tE5ZxUhN6E2s/F7LHHGOOErSJAXlhuM9eN/SEmUgmwVifQNq4EoL4wUKqZ/Rt+LmuIru5\n\tizvTXNPseM5rvYh5Jkd8KX/8W//IXIDQDQnHOlQAf9PYE0fxOWtsvNmWPNzZosdpr0Po\n\tXyTg==","X-Gm-Message-State":"AOAM530H52ck1waZ0Cm43N1fmgtaDgOUDCNWNIDRzPbMHrt504URZ8QJ\n\tO0zg8gFfj+Y+z7QrK899pLo3o8HIiqElhxtSRugdzfrEU8LG6Q==","X-Google-Smtp-Source":"ABdhPJxoRyjGcbsBVauX40SU5YZHvu65ZXPqIgNHD8uyP9g98c/DCUVEMsl6FqkXOxB9ifD8dQkwXFIKVLS7aS+Ke20=","X-Received":"by 2002:a9d:62ce:: with SMTP id z14mr8149206otk.56.1636551344946;\n\tWed, 10 Nov 2021 05:35:44 -0800 (PST)","MIME-Version":"1.0","References":"<20211029120001.2469018-1-hanlinchen@chromium.org>\n\t<20211029120001.2469018-4-hanlinchen@chromium.org>\n\t<163597818115.1378115.7023055491040883953@Monstersaurus>","In-Reply-To":"<163597818115.1378115.7023055491040883953@Monstersaurus>","From":"Hanlin Chen ","Date":"Wed, 10 Nov 2021 21:35:34 +0800","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20813,"web_url":"https://patchwork.libcamera.org/comment/20813/","msgid":"<4e726730-67fe-d204-7998-c873922f042b@ideasonboard.com>","date":"2021-11-10T14:07:10","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 11/10/21 7:05 PM, Hanlin Chen wrote:\n> Hi Kieran,\n>\n> On Thu, Nov 4, 2021 at 6:23 AM Kieran Bingham\n> wrote:\n>> Quoting Han-Lin Chen (2021-10-29 12:59:59)\n>>> Apply auto focus and send lens controls to pipeline handler.\n>>>\n>>> Signed-off-by: Han-Lin Chen \n>>> ---\n>>> aiq/aiq.cpp | 5 ++++-\n>>> aiq/aiq.h | 2 +-\n>>> aiq/aiq_input_parameters.cpp | 2 +-\n>>> ipu3.cpp | 27 ++++++++++++++++++++++++++-\n>>> 4 files changed, 32 insertions(+), 4 deletions(-)\n>>>\n>>> diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp\n>>> index 24c61cb..9c7f9d6 100644\n>>> --- a/aiq/aiq.cpp\n>>> +++ b/aiq/aiq.cpp\n>>> @@ -139,7 +139,7 @@ int AIQ::setStatistics(unsigned int frame,\n>>> * of the parameter buffer being the only part handled when called for.\n>>> */\n>>> int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n>>> - AiqResults &results)\n>>> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime)\n>> Indentation changed for the worse there I think.\n>>\n>> Do we really need to pass in individual parameters through the run\n>> function though? I don't think that's sustainable - as we'll likely have\n>> to pass in more parameters.\n>>\n>> Is the lens position / start time something that would already be\n>> expected to be passed in therou the AiqInputParametners? Or would we\n>> need to define a new parameter structure to pass into the run function?\n> Agreed. I should set it in AiqInputParametners directly.\n\n\nAs stated in the v1, I guess we are due for a refactoring to handle \nvarious input parameters for the algorithms. Probably also need to work \nout a design plan and schedule. If it doesn't happen in this series or \nbefore this, I recommend adding a \\todo here.\n\n\nv1 \nhttps://lists.libcamera.org/pipermail/libcamera-devel/2021-October/026452.html\n\n>>\n>>\n>>> {\n>>> (void)frame;\n>>>\n>>> @@ -157,6 +157,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n>>> params.saParams.awb_results = results.awb();\n>>> shadingAdapterRun(params.saParams, results);\n>>>\n>>> + params.afParams.lens_position = lensPosition;\n>>> + params.afParams.lens_movement_start_timestamp = lensMovementStartTime;\n>>> +\n>>> afRun(params.afParams, results);\n>>>\n>>> return 0;\n>>> diff --git a/aiq/aiq.h b/aiq/aiq.h\n>>> index fcd02d2..4f5f868 100644\n>>> --- a/aiq/aiq.h\n>>> +++ b/aiq/aiq.h\n>>> @@ -41,7 +41,7 @@ public:\n>>> const ipu3_uapi_stats_3a *stats);\n>>>\n>>> int run2a(unsigned int frame, AiqInputParameters ¶ms,\n>>> - AiqResults &results);\n>>> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime);\n>>>\n>>> private:\n>>> std::string decodeError(ia_err err);\n>>> diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp\n>>> index 46553a6..5dd2f6c 100644\n>>> --- a/aiq/aiq_input_parameters.cpp\n>>> +++ b/aiq/aiq_input_parameters.cpp\n>>> @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults()\n>>> ia_aiq_af_range_normal,\n>>> ia_aiq_af_metering_mode_auto,\n>>> ia_aiq_flash_mode_off,\n>>> - NULL, NULL, false\n>>> + &focusRect, &manualFocusParams, false\n>>> };\n>>>\n>>> /* GBCE Params */\n>>> diff --git a/ipu3.cpp b/ipu3.cpp\n>>> index f463805..c2dc754 100644\n>>> --- a/ipu3.cpp\n>>> +++ b/ipu3.cpp\n>>> @@ -77,6 +77,10 @@ private:\n>>> uint32_t gain_;\n>>> uint32_t minGain_;\n>>> uint32_t maxGain_;\n>>> + int32_t lensPosition_;\n>>> +\n>>> + /* Intel AF library relies on timestamp to wait for lens movement */\n>>> + uint64_t lensMovementStartTime_;\n>>>\n>>> /* Intel Library Instances. */\n>>> aiq::AIQ aiq_;\n>>> @@ -258,6 +262,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>>> maxGain_ = itGain->second.max().get();\n>>> gain_ = maxGain_;\n>>>\n>>> + lensMovementStartTime_ = 0;\n>>> + lensPosition_ = 0;\n>>> +\n>>> int ret;\n>>>\n>>> ret = aiq_.configure();\n>>> @@ -381,7 +388,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n>>> */\n>>>\n>>> /* Run algorithms into/using this context structure */\n>>> - aiq_.run2a(frame, aiqInputParams_, results_);\n>>> + aiq_.run2a(frame, aiqInputParams_, results_, lensPosition_, lensMovementStartTime_);\n>>>\n>>> aic_.updateRuntimeParams(results_);\n>>> aic_.run(params);\n>>> @@ -389,6 +396,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n>>> exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;\n>>> gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;\n>>>\n>>> + /*\n>>> + * Af algorithm compares the timestamp of start of lens movement and the\n>>> + * that of the statistics generated to estimate whether next lens\n>>> + * positon should be produced.\n>>> + * Todo: Use the lens movement start time reported by the pipeline handler.\n>>> + */\n>>> + if (lensPosition_ != results_.af()->next_lens_position) {\n>>> + utils::time_point time = utils::clock::now();\n>>> + uint64_t msecs = std::chrono::duration_cast(time.time_since_epoch()).count();\n>>> + lensMovementStartTime_ = msecs;\n>>> + }\n>>> + lensPosition_ = results_.af()->next_lens_position;\n>>> +\n>>> resultsHistory_.Push(results_);\n>>>\n>>> setControls(frame);\n>>> @@ -472,6 +492,11 @@ void IPAIPU3::setControls(unsigned int frame)\n>>>\n>>> op.sensorControls = sensorCtrls;\n>>>\n>>> + ControlList lensCtrls;\n>>> + lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n>>> +\n>>> + op.lensControls = lensCtrls;\n>> Does this need to create a ControlList just to copy/assign it?\n>> Can we call\n>>\n>> op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n>>\n>> directly?\n> Indeed.\n>>\n>>> +\n>>> queueFrameAction.emit(frame, op);\n>>> }\n>>>\n>>> --\n>>> 2.33.1.1089.g2158813163f-goog\n>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A2D9DBF415\n\tfor ;\n\tWed, 10 Nov 2021 14:07:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 50FA86035D;\n\tWed, 10 Nov 2021 15:07:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1F786033C\n\tfor ;\n\tWed, 10 Nov 2021 15:07:16 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.5])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BF406501;\n\tWed, 10 Nov 2021 15:07:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"enUkjmFO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636553236;\n\tbh=wVZhSHMw+5jGa5fg1VPLZtotdG5sC6DTacXtFD+ekeo=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=enUkjmFO1q101khA22Nxjw7kggLkEwVWPTzqvSQCz6bUIjrVSHMM/s1xATQ5KzASm\n\tQbjH/n+2Z7OBXOlGRkLvOZ6txvsYWfl+0lbnJeGvsBolkTyfMO8KUP+Gt4vNLX5+Xp\n\tPMF5lneR4Gpa7c8i3XpOUEYHp3VBIEoTxHLtQfyQ=","To":"Hanlin Chen ,\n\tKieran Bingham ","References":"<20211029120001.2469018-1-hanlinchen@chromium.org>\n\t<20211029120001.2469018-4-hanlinchen@chromium.org>\n\t<163597818115.1378115.7023055491040883953@Monstersaurus>\n\t","From":"Umang Jain ","Message-ID":"<4e726730-67fe-d204-7998-c873922f042b@ideasonboard.com>","Date":"Wed, 10 Nov 2021 19:37:10 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20815,"web_url":"https://patchwork.libcamera.org/comment/20815/","msgid":"","date":"2021-11-10T14:14:56","subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi Umang,\n\nOn Wed, Nov 10, 2021 at 10:07 PM Umang Jain wrote:\n>\n> Hi,\n>\n> On 11/10/21 7:05 PM, Hanlin Chen wrote:\n> > Hi Kieran,\n> >\n> > On Thu, Nov 4, 2021 at 6:23 AM Kieran Bingham\n> > wrote:\n> >> Quoting Han-Lin Chen (2021-10-29 12:59:59)\n> >>> Apply auto focus and send lens controls to pipeline handler.\n> >>>\n> >>> Signed-off-by: Han-Lin Chen \n> >>> ---\n> >>> aiq/aiq.cpp | 5 ++++-\n> >>> aiq/aiq.h | 2 +-\n> >>> aiq/aiq_input_parameters.cpp | 2 +-\n> >>> ipu3.cpp | 27 ++++++++++++++++++++++++++-\n> >>> 4 files changed, 32 insertions(+), 4 deletions(-)\n> >>>\n> >>> diff --git a/aiq/aiq.cpp b/aiq/aiq.cpp\n> >>> index 24c61cb..9c7f9d6 100644\n> >>> --- a/aiq/aiq.cpp\n> >>> +++ b/aiq/aiq.cpp\n> >>> @@ -139,7 +139,7 @@ int AIQ::setStatistics(unsigned int frame,\n> >>> * of the parameter buffer being the only part handled when called for.\n> >>> */\n> >>> int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> >>> - AiqResults &results)\n> >>> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime)\n> >> Indentation changed for the worse there I think.\n> >>\n> >> Do we really need to pass in individual parameters through the run\n> >> function though? I don't think that's sustainable - as we'll likely have\n> >> to pass in more parameters.\n> >>\n> >> Is the lens position / start time something that would already be\n> >> expected to be passed in therou the AiqInputParametners? Or would we\n> >> need to define a new parameter structure to pass into the run function?\n> > Agreed. I should set it in AiqInputParametners directly.\n>\n>\n> As stated in the v1, I guess we are due for a refactoring to handle\n> various input parameters for the algorithms. Probably also need to work\n> out a design plan and schedule. If it doesn't happen in this series or\n> before this, I recommend adding a \\todo here.\n>\n>\n> v1\n> https://lists.libcamera.org/pipermail/libcamera-devel/2021-October/026452.html\nSGTM.\n>\n> >>\n> >>\n> >>> {\n> >>> (void)frame;\n> >>>\n> >>> @@ -157,6 +157,9 @@ int AIQ::run2a(unsigned int frame, AiqInputParameters ¶ms,\n> >>> params.saParams.awb_results = results.awb();\n> >>> shadingAdapterRun(params.saParams, results);\n> >>>\n> >>> + params.afParams.lens_position = lensPosition;\n> >>> + params.afParams.lens_movement_start_timestamp = lensMovementStartTime;\n> >>> +\n> >>> afRun(params.afParams, results);\n> >>>\n> >>> return 0;\n> >>> diff --git a/aiq/aiq.h b/aiq/aiq.h\n> >>> index fcd02d2..4f5f868 100644\n> >>> --- a/aiq/aiq.h\n> >>> +++ b/aiq/aiq.h\n> >>> @@ -41,7 +41,7 @@ public:\n> >>> const ipu3_uapi_stats_3a *stats);\n> >>>\n> >>> int run2a(unsigned int frame, AiqInputParameters ¶ms,\n> >>> - AiqResults &results);\n> >>> + AiqResults &results, int lensPosition, int64_t lensMovementStartTime);\n> >>>\n> >>> private:\n> >>> std::string decodeError(ia_err err);\n> >>> diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp\n> >>> index 46553a6..5dd2f6c 100644\n> >>> --- a/aiq/aiq_input_parameters.cpp\n> >>> +++ b/aiq/aiq_input_parameters.cpp\n> >>> @@ -166,7 +166,7 @@ void AiqInputParameters::setAeAwbAfDefaults()\n> >>> ia_aiq_af_range_normal,\n> >>> ia_aiq_af_metering_mode_auto,\n> >>> ia_aiq_flash_mode_off,\n> >>> - NULL, NULL, false\n> >>> + &focusRect, &manualFocusParams, false\n> >>> };\n> >>>\n> >>> /* GBCE Params */\n> >>> diff --git a/ipu3.cpp b/ipu3.cpp\n> >>> index f463805..c2dc754 100644\n> >>> --- a/ipu3.cpp\n> >>> +++ b/ipu3.cpp\n> >>> @@ -77,6 +77,10 @@ private:\n> >>> uint32_t gain_;\n> >>> uint32_t minGain_;\n> >>> uint32_t maxGain_;\n> >>> + int32_t lensPosition_;\n> >>> +\n> >>> + /* Intel AF library relies on timestamp to wait for lens movement */\n> >>> + uint64_t lensMovementStartTime_;\n> >>>\n> >>> /* Intel Library Instances. */\n> >>> aiq::AIQ aiq_;\n> >>> @@ -258,6 +262,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> >>> maxGain_ = itGain->second.max().get();\n> >>> gain_ = maxGain_;\n> >>>\n> >>> + lensMovementStartTime_ = 0;\n> >>> + lensPosition_ = 0;\n> >>> +\n> >>> int ret;\n> >>>\n> >>> ret = aiq_.configure();\n> >>> @@ -381,7 +388,7 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> >>> */\n> >>>\n> >>> /* Run algorithms into/using this context structure */\n> >>> - aiq_.run2a(frame, aiqInputParams_, results_);\n> >>> + aiq_.run2a(frame, aiqInputParams_, results_, lensPosition_, lensMovementStartTime_);\n> >>>\n> >>> aic_.updateRuntimeParams(results_);\n> >>> aic_.run(params);\n> >>> @@ -389,6 +396,19 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)\n> >>> exposure_ = results_.ae()->exposures[0].sensor_exposure->coarse_integration_time;\n> >>> gain_ = results_.ae()->exposures[0].sensor_exposure->analog_gain_code_global;\n> >>>\n> >>> + /*\n> >>> + * Af algorithm compares the timestamp of start of lens movement and the\n> >>> + * that of the statistics generated to estimate whether next lens\n> >>> + * positon should be produced.\n> >>> + * Todo: Use the lens movement start time reported by the pipeline handler.\n> >>> + */\n> >>> + if (lensPosition_ != results_.af()->next_lens_position) {\n> >>> + utils::time_point time = utils::clock::now();\n> >>> + uint64_t msecs = std::chrono::duration_cast(time.time_since_epoch()).count();\n> >>> + lensMovementStartTime_ = msecs;\n> >>> + }\n> >>> + lensPosition_ = results_.af()->next_lens_position;\n> >>> +\n> >>> resultsHistory_.Push(results_);\n> >>>\n> >>> setControls(frame);\n> >>> @@ -472,6 +492,11 @@ void IPAIPU3::setControls(unsigned int frame)\n> >>>\n> >>> op.sensorControls = sensorCtrls;\n> >>>\n> >>> + ControlList lensCtrls;\n> >>> + lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> >>> +\n> >>> + op.lensControls = lensCtrls;\n> >> Does this need to create a ControlList just to copy/assign it?\n> >> Can we call\n> >>\n> >> op.lensControls.set(V4L2_CID_FOCUS_ABSOLUTE, lensPosition_);\n> >>\n> >> directly?\n> > Indeed.\n> >>\n> >>> +\n> >>> queueFrameAction.emit(frame, op);\n> >>> }\n> >>>\n> >>> --\n> >>> 2.33.1.1089.g2158813163f-goog\n> >>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DCF45BF415\n\tfor ;\n\tWed, 10 Nov 2021 14:15:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B3B66035D;\n\tWed, 10 Nov 2021 15:15:09 +0100 (CET)","from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com\n\t[IPv6:2607:f8b0:4864:20::22d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D71D16033C\n\tfor ;\n\tWed, 10 Nov 2021 15:15:08 +0100 (CET)","by mail-oi1-x22d.google.com with SMTP id bl27so5569876oib.0\n\tfor ;\n\tWed, 10 Nov 2021 06:15:08 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"eYw8VxN2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=sBZB8/OeR1s5448QCgfmdxthuaHiqHzliisj3h6IyK0=;\n\tb=eYw8VxN2ALO8yzu3R0U+5A7ie3LV+PoEkKixl4RIMHHYLmHb+mrvufro4EBRQzs6kN\n\tpDtPEkpbdqhhwjgRBW3wZgu65JQbTk1i+Lsd9JUmIG12B6ElGMNOOUht4pLSFcV0GH7R\n\t3DGqCFFANdUhlZcxqGuoYAVu2Sgl8hBuoA+JY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=sBZB8/OeR1s5448QCgfmdxthuaHiqHzliisj3h6IyK0=;\n\tb=Y5MFxRDEQsHEXoxVJe+oOz004CQSIpwUcBLvaKdWa51LREaT1VY4TJhV7NPWtmKYX9\n\tu4DTG6LWFNBrKcTyL70ReD1l/RaUO5YfktsxPdephlwYT+dg7tnBCyJUW6jXha3ohLIY\n\t6uDXHYWxksv88A1YrQ50BqdNicWCL7HLCPbSRgEEMGkZ6Q7/KspTOFdiJFqqV9YBOsJU\n\tjZpah1d1J1DUnnILdsmZ4TlQuSgqm3eoJPFUfcyPR8FpwdIBWSaG41Jb5KdY1FfjNmJX\n\tOnmU+lvy4kUcXkS4gvpOIACxjjpxFiad3On30xZxMNrvb8bN3amMus/C91hOrMmpDNsH\n\tj7MQ==","X-Gm-Message-State":"AOAM530WAxKSEfeWlYMfnUl4BMrDLTaAJrriol5Woyghb/WDRf4OyFQ3\n\tak+doMcDl3zLXq+Y3/qV3Lyp1UnUE9gmCl+EASIiDQ==","X-Google-Smtp-Source":"ABdhPJwSHet7ia/wtMG+bbnCzJejrcxNvvYd1kT9Cb4n6f4FJG2hW4S51OlHg7nv+6u72BKgiFEiWNXMYigefJVwCIw=","X-Received":"by 2002:a54:4401:: with SMTP id\n\tk1mr13017054oiw.143.1636553707637; \n\tWed, 10 Nov 2021 06:15:07 -0800 (PST)","MIME-Version":"1.0","References":"<20211029120001.2469018-1-hanlinchen@chromium.org>\n\t<20211029120001.2469018-4-hanlinchen@chromium.org>\n\t<163597818115.1378115.7023055491040883953@Monstersaurus>\n\t\n\t<4e726730-67fe-d204-7998-c873922f042b@ideasonboard.com>","In-Reply-To":"<4e726730-67fe-d204-7998-c873922f042b@ideasonboard.com>","From":"Hanlin Chen ","Date":"Wed, 10 Nov 2021 22:14:56 +0800","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] ipu3: Apply auto focus and\n\tsend lens controls to pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]