[{"id":23875,"web_url":"https://patchwork.libcamera.org/comment/23875/","msgid":"<20220714145307.xrish3ofmps5yhfx@uno.localdomain>","date":"2022-07-14T14:53:07","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Daniel,\n\nOn Tue, Jun 28, 2022 at 11:06:54AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> Control lens focus from rkisp1 pipeline, using CameraLens controller\n> and expose lens controls to the IPA during configure().\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++++++++++++\n> 1 file changed, 20 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 7cf36524..363273b2 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -28,6 +28,7 @@\n> #include \n>\n> #include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/camera_lens.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> @@ -107,6 +108,7 @@ private:\n> \tvoid paramFilled(unsigned int frame);\n> \tvoid setSensorControls(unsigned int frame,\n> \t\t\t const ControlList &sensorControls);\n> +\tvoid setLensControls(const ControlList &lensControls);\n>\n> \tvoid metadataReady(unsigned int frame, const ControlList &metadata);\n> };\n> @@ -353,6 +355,20 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n> \tdelayedCtrls_->push(sensorControls);\n> }\n>\n> +void RkISP1CameraData::setLensControls(const ControlList &lensControls)\n> +{\n> +\tCameraLens *focusLens = sensor_->focusLens();\n> +\tif (!focusLens)\n> +\t\treturn;\n> +\n> +\tif (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))\n> +\t\treturn;\n\nI would rather do so in the CameraLens class.\n\nThe class validates that V4L2_CID_FOCUS_ABSOLUTE is available in\nCameraLens::init(). I would store a flag there and return immediately\nfrom setFocusPosition() if not available.\n\n> +\n> +\tconst ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);\n> +\n> +\tfocusLens->setFocusPosition(focusValue.get());\n> +}\n> +\n> void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n> {\n> \tRkISP1FrameInfo *info = frameInfo_.find(frame);\n> @@ -654,6 +670,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n>\n> +\tCameraLens *lens = data->sensor_->focusLens();\n> +\tif (lens)\n> +\t\tentityControls.emplace(1, lens->controls());\n> +\n\nI would have made 1 patch for the PH/IPA initialization, and one patch\nfor the IPA/PH control set phase instead of making one patch for PH\nand one for IPA. Hope this makes sense :P\n\nThe patch direction looks good!\n\nThanks\n j\n> \tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> \tif (ret) {\n> \t\tLOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> --\n> 2.34.1\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 0C5B0BE173\n\tfor ;\n\tThu, 14 Jul 2022 14:53:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6234B63312;\n\tThu, 14 Jul 2022 16:53:11 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::223])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 86FA36330D\n\tfor ;\n\tThu, 14 Jul 2022 16:53:10 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id B9E8960002;\n\tThu, 14 Jul 2022 14:53:09 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657810391;\n\tbh=AGxJrd/O4X0U3++brdD2pP35JpG5QCokmnaOuzvG0Xk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=0MB3+BixtqkD8uQB+U9Vq1Le/EgVxWfWOQXayhmQUXBvM9t54Fx0YFnDtH8T6+RfK\n\tWqVceVXoNe+S+TdDCgTFrcHOipgFImiz/eNsL02ubCvrY15h92dASCMfk4EQ/jl+tl\n\tlcALfWbDugay2KbwERCb047gNJBa10t9ISeRv0TFiEbDKroXWJZ3dlrFzqGZK3hkWg\n\tWr2X+4q7M/c/kh1/p9Js6w3GKJDYhpobYgSefyB3k56Nkac8lqcOWJE6EUKB2O7ddK\n\to+PJ4NfWrLBhZ/QEsswZGnirTKnoWqUsnnLAgwq/k/9t27uPbxJrwrI/5zK/FTo4QJ\n\tCkKqZorEBij1A==","Date":"Thu, 14 Jul 2022 16:53:07 +0200","To":"Daniel Semkowicz ","Message-ID":"<20220714145307.xrish3ofmps5yhfx@uno.localdomain>","References":"<20220628090656.19572-1-dse@thaumatec.com>\n\t<20220628090656.19572-2-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220628090656.19572-2-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23950,"web_url":"https://patchwork.libcamera.org/comment/23950/","msgid":"","date":"2022-07-18T13:56:07","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nOn Thu, Jul 14, 2022 at 4:53 PM Jacopo Mondi wrote:\n>\n> Hi Daniel,\n>\n> On Tue, Jun 28, 2022 at 11:06:54AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > Control lens focus from rkisp1 pipeline, using CameraLens controller\n> > and expose lens controls to the IPA during configure().\n> >\n> > Signed-off-by: Daniel Semkowicz \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++++++++++++\n> > 1 file changed, 20 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 7cf36524..363273b2 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -28,6 +28,7 @@\n> > #include \n> >\n> > #include \"libcamera/internal/camera.h\"\n> > +#include \"libcamera/internal/camera_lens.h\"\n> > #include \"libcamera/internal/camera_sensor.h\"\n> > #include \"libcamera/internal/delayed_controls.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > @@ -107,6 +108,7 @@ private:\n> > void paramFilled(unsigned int frame);\n> > void setSensorControls(unsigned int frame,\n> > const ControlList &sensorControls);\n> > + void setLensControls(const ControlList &lensControls);\n> >\n> > void metadataReady(unsigned int frame, const ControlList &metadata);\n> > };\n> > @@ -353,6 +355,20 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n> > delayedCtrls_->push(sensorControls);\n> > }\n> >\n> > +void RkISP1CameraData::setLensControls(const ControlList &lensControls)\n> > +{\n> > + CameraLens *focusLens = sensor_->focusLens();\n> > + if (!focusLens)\n> > + return;\n> > +\n> > + if (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))\n> > + return;\n>\n> I would rather do so in the CameraLens class.\n>\n> The class validates that V4L2_CID_FOCUS_ABSOLUTE is available in\n> CameraLens::init(). I would store a flag there and return immediately\n> from setFocusPosition() if not available.\n\nGood point! Now I wonder if the CameraSensor::focusLens() should return\nthe valid pointer at all if the CameraLens::init() failed. If it is\nspecified that V4L2_CID_FOCUS_ABSOLUTE is mandatory and init fails, then\nI would expect to not receive the CameraLens object if it is not valid.\n\nMaybe We should return nullptr in such case? Then We can forget about\nadditional checks in PH and IPA. If the CameraLens does not meet the\nmandatory requirements then I would treat this situation as if the lens\nare not available at all.\n\n>\n> > +\n> > + const ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);\n> > +\n> > + focusLens->setFocusPosition(focusValue.get());\n> > +}\n> > +\n> > void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n> > {\n> > RkISP1FrameInfo *info = frameInfo_.find(frame);\n> > @@ -654,6 +670,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > std::map entityControls;\n> > entityControls.emplace(0, data->sensor_->controls());\n> >\n> > + CameraLens *lens = data->sensor_->focusLens();\n> > + if (lens)\n> > + entityControls.emplace(1, lens->controls());\n> > +\n>\n> I would have made 1 patch for the PH/IPA initialization, and one patch\n> for the IPA/PH control set phase instead of making one patch for PH\n> and one for IPA. Hope this makes sense :P\n\nYes, this makes sense. I will try to arrange the next patchset this way :)\n\n>\n> The patch direction looks good!\n>\n> Thanks\n> j\n> > ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > if (ret) {\n> > LOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> > --\n> > 2.34.1\n> >\n\nBest regards\nDaniel","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 672CBBD1F1\n\tfor ;\n\tMon, 18 Jul 2022 13:56:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC1C063312;\n\tMon, 18 Jul 2022 15:56:21 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D21516048B\n\tfor ;\n\tMon, 18 Jul 2022 15:56:19 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id u15so13656703lji.10\n\tfor ;\n\tMon, 18 Jul 2022 06:56:19 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658152581;\n\tbh=i7TB76T5Qo3fbEvYlRrY1q6xO6T5fonEuuBmkREPwv8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=qPN5+SvLejBh27O0hYYzthVVDgGMjFkq3HNPYw59jTm9+jERf1c3B2lpu7NgMUnrz\n\txhz/a7e+FuKXzdEq9QXZgzugR8ZxwQSGmJMi9jk2Vms5fRs3/vHZY/dNFCGCJ/WI+3\n\tUSwS6aLjlcDfqOuc2HUs8+GTtmeh9zo0l2ACW5mqR9ym0yPXf7fqLEah/pkjlpwXBh\n\t0rm8gkiTuvUI9Qm4LAJsPpyunNjXNBIDT5vIpQXpNX2M8pftYEpUkj186ai3MDVTrF\n\tI5bZrvt48GA0SngvKMpvv93NFT7wsuSi8kNEXeCfa7ptnEr+GNaVAEUu27JK/DxbyE\n\tduZt2ImLTPAKQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=J8hKKu0qb+dDyZaTbmFiQtUolcIP0jFjQqhBL7REicg=;\n\tb=F6tIxpKeF+KADroET3ptQgE8xhWuadsc4BZ0SVyI7HlKkhRSWwVDCRPDoxswlON22B\n\tV1/Le+wwC/hFbZJG/FaobOzvK4QtKqp+qJrYNn807xrOpq4z6iTkbjffHE79+FZgF+U8\n\tfz87a2ue2HmdRbzage4R44d5Yfd5ilIYJKM5DaRnRBsmE5MTBW0zZimvRX5iFqZFZ8cA\n\tEXrm7eqC4B+Xj2qN2ZOnm5J1iPeR/HyKk0HYGK0xAKlvWSZOEbm99/zWr6MulK9jBchG\n\t2mEktHNwk2+g3XMlwAcm++mRdo/Whac76DMjGmBYWu6+1uCVOiQQLyP6SnRbYT4bo7G0\n\t6kBA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"F6tIxpKe\"; \n\tdkim-atps=neutral","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=J8hKKu0qb+dDyZaTbmFiQtUolcIP0jFjQqhBL7REicg=;\n\tb=MBJwrjZlorQiKNnSdzMRH+71CM6NPHJ0iEVIekuWWIuAG9jChwGaaZwJGiqKG+oInE\n\tMZSNouB7LKS1l86u6/jYJBfF8Jpc9Mq4kR78jGjGhknjbrrDyaq89sotTr8bhuY3IGbx\n\thKzMyKP02n61l0+G5L5JZkVuMlo/NlsFu4MjyJBOXUSgG38Wor1MltqCJhydFnUuzG8s\n\tMiWdMdZqruxXNJMQpVWgCAshMhkq9o3w6rgq4OpIbxfyyI/4ryTztfeEZTDD+s0JfjXF\n\t2T9j5HSlg3yk2zFjHdPkK1I5jGl98cgcy7An3e/QRGoUvOrEWJW8JU8Hfwqq0Uq7dUw1\n\tdlOg==","X-Gm-Message-State":"AJIora+AlyYEvfpIu3yewrXsYO0vRUHDSt432AQkYgIy4MeGuQXDlxxA\n\tQ3UevGA7ZoE8ZllAiLSYgrw3KCLuIXQEi2uS/Wtq6bi4i4nJlQ==","X-Google-Smtp-Source":"AGRyM1vBq6BBYRFAF3SShUlr1qThrbzlNOcWFVE+W/wLPmwORVGxtX2kcWZE4haR8iOvCWndsP/OHZR6wp/opS/i3zk=","X-Received":"by 2002:a2e:8707:0:b0:25d:bbd5:c2f with SMTP id\n\tm7-20020a2e8707000000b0025dbbd50c2fmr1947112lji.464.1658152578980;\n\tMon, 18 Jul 2022 06:56:18 -0700 (PDT)","MIME-Version":"1.0","References":"<20220628090656.19572-1-dse@thaumatec.com>\n\t<20220628090656.19572-2-dse@thaumatec.com>\n\t<20220714145307.xrish3ofmps5yhfx@uno.localdomain>","In-Reply-To":"<20220714145307.xrish3ofmps5yhfx@uno.localdomain>","Date":"Mon, 18 Jul 2022 15:56:07 +0200","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","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","From":"Daniel Semkowicz via libcamera-devel\n\t","Reply-To":"Daniel Semkowicz ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24143,"web_url":"https://patchwork.libcamera.org/comment/24143/","msgid":"<20220726102954.h5i6zmh6njykarvb@uno.localdomain>","date":"2022-07-26T10:29:54","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Daniel\n\nOn Mon, Jul 18, 2022 at 03:56:07PM +0200, Daniel Semkowicz wrote:\n> Hi Jacopo,\n>\n> On Thu, Jul 14, 2022 at 4:53 PM Jacopo Mondi wrote:\n> >\n> > Hi Daniel,\n> >\n> > On Tue, Jun 28, 2022 at 11:06:54AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > Control lens focus from rkisp1 pipeline, using CameraLens controller\n> > > and expose lens controls to the IPA during configure().\n> > >\n> > > Signed-off-by: Daniel Semkowicz \n> > > ---\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++++++++++++++++++++\n> > > 1 file changed, 20 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 7cf36524..363273b2 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -28,6 +28,7 @@\n> > > #include \n> > >\n> > > #include \"libcamera/internal/camera.h\"\n> > > +#include \"libcamera/internal/camera_lens.h\"\n> > > #include \"libcamera/internal/camera_sensor.h\"\n> > > #include \"libcamera/internal/delayed_controls.h\"\n> > > #include \"libcamera/internal/device_enumerator.h\"\n> > > @@ -107,6 +108,7 @@ private:\n> > > void paramFilled(unsigned int frame);\n> > > void setSensorControls(unsigned int frame,\n> > > const ControlList &sensorControls);\n> > > + void setLensControls(const ControlList &lensControls);\n> > >\n> > > void metadataReady(unsigned int frame, const ControlList &metadata);\n> > > };\n> > > @@ -353,6 +355,20 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n> > > delayedCtrls_->push(sensorControls);\n> > > }\n> > >\n> > > +void RkISP1CameraData::setLensControls(const ControlList &lensControls)\n> > > +{\n> > > + CameraLens *focusLens = sensor_->focusLens();\n> > > + if (!focusLens)\n> > > + return;\n> > > +\n> > > + if (!lensControls.contains(V4L2_CID_FOCUS_ABSOLUTE))\n> > > + return;\n> >\n> > I would rather do so in the CameraLens class.\n> >\n> > The class validates that V4L2_CID_FOCUS_ABSOLUTE is available in\n> > CameraLens::init(). I would store a flag there and return immediately\n> > from setFocusPosition() if not available.\n>\n> Good point! Now I wonder if the CameraSensor::focusLens() should return\n> the valid pointer at all if the CameraLens::init() failed. If it is\n> specified that V4L2_CID_FOCUS_ABSOLUTE is mandatory and init fails, then\n> I would expect to not receive the CameraLens object if it is not valid.\n>\n> Maybe We should return nullptr in such case? Then We can forget about\n> additional checks in PH and IPA. If the CameraLens does not meet the\n> mandatory requirements then I would treat this situation as if the lens\n> are not available at all.\n>\n\nIt feels rather dangerous to return a nullptr because a feature is\nmissing, it's a recipe for lazy callers to get into a segfault.\n\nImagine you connect a different camera to your platform and the lens\ndriver does not support the mandatory control while the one you were\nusing did. You would have a segfault. True, there's plenty of messages\nin the log that could suggest you what went wrong, but segfaults are\nnever nice.\n\nIt's true that if the lens is not registered in DTS right now you get a\nnullptr, so the pipeline handler should already be prepared to handle\nthat situation...\n\nPlease note that if the CameraLens class validation fails then\nCameraSensor::init() fails as well, so if the pipeline handler is\neducated enough it could bail out soon enough or simply ignore the\nlens.\n\nAlso, if we call CameraLens::setFocusPosition() and the lens does not\nsupport V4L2_CID_FOCUS_ABSOLUTE the V4L2Subdevice::setControl() call\nwould complain loudly but no crashes are expected.\n\nAll in all, I would not perform any check here (IPU3 should probably\nbe updated too) but rather let the CameraLens::setFocusPosition() fail\nat setControls() time, or catch it before even getting there (which\nmight be better as we can express a more precise error message instead\nof relying on the:\n\n LOG(V4L2, Error)\n << \"Control \" << utils::hex(id) << \" not found\";\n\nError message in V4L2Device::setControls() which reports the numerical\ncontrol id, which we know it's not nice to decipher.\n\nDoes this make sense to you ?\n\nThanks\n j\n\n> >\n> > > +\n> > > + const ControlValue &focusValue = lensControls.get(V4L2_CID_FOCUS_ABSOLUTE);\n> > > +\n> > > + focusLens->setFocusPosition(focusValue.get());\n> > > +}\n> > > +\n> > > void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n> > > {\n> > > RkISP1FrameInfo *info = frameInfo_.find(frame);\n> > > @@ -654,6 +670,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > > std::map entityControls;\n> > > entityControls.emplace(0, data->sensor_->controls());\n> > >\n> > > + CameraLens *lens = data->sensor_->focusLens();\n> > > + if (lens)\n> > > + entityControls.emplace(1, lens->controls());\n> > > +\n> >\n> > I would have made 1 patch for the PH/IPA initialization, and one patch\n> > for the IPA/PH control set phase instead of making one patch for PH\n> > and one for IPA. Hope this makes sense :P\n>\n> Yes, this makes sense. I will try to arrange the next patchset this way :)\n>\n> >\n> > The patch direction looks good!\n> >\n> > Thanks\n> > j\n> > > ret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > if (ret) {\n> > > LOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> > > --\n> > > 2.34.1\n> > >\n>\n> Best regards\n> Daniel","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 56C03C3275\n\tfor ;\n\tTue, 26 Jul 2022 10:29:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1AD063312;\n\tTue, 26 Jul 2022 12:29:58 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11BD360487\n\tfor ;\n\tTue, 26 Jul 2022 12:29:57 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 8145B20011;\n\tTue, 26 Jul 2022 10:29:56 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658831398;\n\tbh=y3BRmkNuU9hYEVgZC+VHStNA3go/Q+SfLJw7kmpMFPk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=eaYrybpyszwThu3i8ztvlXdOHJpq310/RFp0OuSALrnDqKTMxP80q38RJdQhNTmPn\n\thc8TPnREzjf6yrSFDXXvHCGbCQYJVHEA5MlGFq9qV+rteYcaYOup8juIakX5hGVRa8\n\tDX8T6QTY4tcEPGGcgazWU8540ZlGQKbZkYb8YnQ3P/s7UXRVLs23cHZIRioFy4X3l1\n\tN/uwkvqegTSaS2dVMmXkY3po+LdxPzHxx7sQ3MXuSSTPPrf3zqM74xVAau3I/jlCbf\n\tgcupZGaTc/1ZmySEVRkqaBRxpOWKz+arq7r3TF39PIjjAmlackdYnaTsKGDyFAazAO\n\t0IPmd2CRTxG2A==","Date":"Tue, 26 Jul 2022 12:29:54 +0200","To":"Daniel Semkowicz ","Message-ID":"<20220726102954.h5i6zmh6njykarvb@uno.localdomain>","References":"<20220628090656.19572-1-dse@thaumatec.com>\n\t<20220628090656.19572-2-dse@thaumatec.com>\n\t<20220714145307.xrish3ofmps5yhfx@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: rkisp1: Control the\n\tlens from pipeline","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]