[{"id":21594,"web_url":"https://patchwork.libcamera.org/comment/21594/","msgid":"","date":"2021-12-04T00:36:48","subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Daniel,\n\nThank you for the patch.\n\nOn Fri, Dec 03, 2021 at 10:42:27PM +0000, Daniel Scally wrote:\n> Add a function to the CameraLens class to fetch the V4L2 controls\n> for its V4L2 subdev\n> \n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v2:\n> \n> \t- New patch\n> \n> include/libcamera/internal/camera_lens.h | 4 ++++\n> src/libcamera/camera_lens.cpp | 11 +++++++++++\n> 2 files changed, 15 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> index 6f2ea1bc..64794294 100644\n> --- a/include/libcamera/internal/camera_lens.h\n> +++ b/include/libcamera/internal/camera_lens.h\n> @@ -12,6 +12,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> namespace libcamera {\n> \n> class MediaEntity;\n> @@ -28,6 +30,8 @@ public:\n> \n> \tconst std::string &model() const { return model_; }\n> \n> +\tconst ControlInfoMap &controls() const;\n> +\n> protected:\n> \tstd::string logPrefix() const override;\n> \n> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> index 189cb025..7b3b9c24 100644\n> --- a/src/libcamera/camera_lens.cpp\n> +++ b/src/libcamera/camera_lens.cpp\n> @@ -139,4 +139,15 @@ std::string CameraLens::logPrefix() const\n> \treturn \"'\" + entity_->name() + \"'\";\n> }\n> \n> +/**\n> + * \\fn CameraLens::controls()\n> + * \\brief Retrieve the V4L2 controls of the lens' subdev\n> + *\n> + * \\return A map of the V4L2 controls supported by the sensor\n\nNot a sensor anymore. With that fixed,\n\nReviewed-by: Laurent Pinchart \n\nI however would like to take one more step, and was wondering if you'd\nlike to volunteer for it.\n\nI think the CameraSensor and CameraLens classes shouldn't expose V4L2\ncontrols, but a custom set of controls specific to libcamera. This way\nwe could remove the dependencies on V4L2 in the IPA modules. The\ntranslation between those controls and the V4L2 controls would be local\nto the CameraSensor and CameraLens classes.\n\nWe would need three controls at the moment, for the analogue gain,\nexposure time, and focus position. It could be a good idea to decouple\nthose controls from the libcamera public controls (exposed by cameras to\napplications), but given that we have libcamera controls for those three\nalready, we could start by using them and see if we need something else\nlater.\n\nI'm also wondering if we could, this way, combine the sensor controls\nand lens controls in a single ControlInfoMap passed to the IPA, and a\nsingle ControlList passed back from the IPA.\n\n> + */\n> +const ControlInfoMap &CameraLens::controls() const\n> +{\n> +\treturn subdev_->controls();\n> +}\n> +\n> } /* namespace libcamera */","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 EA98BBDB13\n\tfor ;\n\tSat, 4 Dec 2021 00:37:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 574496011A;\n\tSat, 4 Dec 2021 01:37:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31A9860117\n\tfor ;\n\tSat, 4 Dec 2021 01:37:16 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8716F30C;\n\tSat, 4 Dec 2021 01:37: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=\"eBzAD4nT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638578235;\n\tbh=be4I6YtbCPveLMa20n0eJMaqzijO19V0eHr8obsvlYk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eBzAD4nTwEcrflbv6LAJXtUaY+1+TVw4SV46W1AttU4H/VSbMgF5MlNYB9vFEyo4F\n\tvNkHEitEhmYKNbsvgAbKcFXxEFkfckclUmCaalTehsvjKsg3Z0CqO6IBFuaLwjPYcI\n\tmfuJGFw5asrWDEZJE7b1QNpARxYuxWjXFg7TmsQI=","Date":"Sat, 4 Dec 2021 02:36:48 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Message-ID":"","References":"<20211203224230.38700-1-djrscally@gmail.com>\n\t<20211203224230.38700-5-djrscally@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211203224230.38700-5-djrscally@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","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":21599,"web_url":"https://patchwork.libcamera.org/comment/21599/","msgid":"<4d3efdd2-0fbf-a7d0-c889-4f1aedbef20d@gmail.com>","date":"2021-12-04T21:55:38","subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Hi Laurent\n\nOn 04/12/2021 00:36, Laurent Pinchart wrote:\n> Hi Daniel,\n> \n> Thank you for the patch.\n> \n> On Fri, Dec 03, 2021 at 10:42:27PM +0000, Daniel Scally wrote:\n>> Add a function to the CameraLens class to fetch the V4L2 controls\n>> for its V4L2 subdev\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> Changes in v2:\n>>\n>> \t- New patch\n>>\n>> include/libcamera/internal/camera_lens.h | 4 ++++\n>> src/libcamera/camera_lens.cpp | 11 +++++++++++\n>> 2 files changed, 15 insertions(+)\n>>\n>> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n>> index 6f2ea1bc..64794294 100644\n>> --- a/include/libcamera/internal/camera_lens.h\n>> +++ b/include/libcamera/internal/camera_lens.h\n>> @@ -12,6 +12,8 @@\n>> #include \n>> #include \n>> \n>> +#include \n>> +\n>> namespace libcamera {\n>> \n>> class MediaEntity;\n>> @@ -28,6 +30,8 @@ public:\n>> \n>> \tconst std::string &model() const { return model_; }\n>> \n>> +\tconst ControlInfoMap &controls() const;\n>> +\n>> protected:\n>> \tstd::string logPrefix() const override;\n>> \n>> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n>> index 189cb025..7b3b9c24 100644\n>> --- a/src/libcamera/camera_lens.cpp\n>> +++ b/src/libcamera/camera_lens.cpp\n>> @@ -139,4 +139,15 @@ std::string CameraLens::logPrefix() const\n>> \treturn \"'\" + entity_->name() + \"'\";\n>> }\n>> \n>> +/**\n>> + * \\fn CameraLens::controls()\n>> + * \\brief Retrieve the V4L2 controls of the lens' subdev\n>> + *\n>> + * \\return A map of the V4L2 controls supported by the sensor\n> \n> Not a sensor anymore. With that fixed,\n> \n> Reviewed-by: Laurent Pinchart \n\nThanks, and for the others too.\n\n> \n> I however would like to take one more step, and was wondering if you'd\n> like to volunteer for it.\n\nSure, I'd be happy to.\n\n> I think the CameraSensor and CameraLens classes shouldn't expose V4L2\n> controls, but a custom set of controls specific to libcamera. This way\n> we could remove the dependencies on V4L2 in the IPA modules. The\n> translation between those controls and the V4L2 controls would be local\n> to the CameraSensor and CameraLens classes.\n\nMakes sense; but what's the advantage to removing the dependency? Is\nthere another means of controlling sensor drivers beyond the V4L2 API\nthat could then be supported?\n\n> \n> We would need three controls at the moment, for the analogue gain,\n> exposure time, and focus position. It could be a good idea to decouple\n> those controls from the libcamera public controls (exposed by cameras to\n> applications), but given that we have libcamera controls for those three\n> already, we could start by using them and see if we need something else\n> later.\n> \n> I'm also wondering if we could, this way, combine the sensor controls\n> and lens controls in a single ControlInfoMap passed to the IPA, and a\n> single ControlList passed back from the IPA.\n\nYeah I thought this could do with combining somehow too; I'll take a look.\n\n> \n>> + */\n>> +const ControlInfoMap &CameraLens::controls() const\n>> +{\n>> +\treturn subdev_->controls();\n>> +}\n>> +\n>> } /* namespace libcamera */\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 8DACABDB13\n\tfor ;\n\tSat, 4 Dec 2021 21:55:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E08E605C4;\n\tSat, 4 Dec 2021 22:55:42 +0100 (CET)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 652FD605C4\n\tfor ;\n\tSat, 4 Dec 2021 22:55:40 +0100 (CET)","by mail-wm1-x329.google.com with SMTP id\n\ti8-20020a7bc948000000b0030db7b70b6bso7560026wml.1\n\tfor ;\n\tSat, 04 Dec 2021 13:55:40 -0800 (PST)","from [192.168.0.16]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161])\n\tby smtp.gmail.com with ESMTPSA id\n\td188sm9065759wmd.3.2021.12.04.13.55.39\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tSat, 04 Dec 2021 13:55:39 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Td4CyTRS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=bXzRYXkxS44bv5WH+Bd8nabj3EidKvqD3G/mSifeBSg=;\n\tb=Td4CyTRS9C8HZvg5HAVDgcBmmqDNDzcvBHsIchhcNth0Vpt5nfRvhpoRGMyBt1vLD+\n\t6uXMCqBs4cvamS4sv7dsRbyI8Em1pQR3kpEmFQr5+wuXczYoiYJLpX1aOu8U2Yd788Eb\n\tFIzElnbR1sFGe0bG9lo5EDTp/0UDY4WK3uz13JoWlxJcEDBMG5+b3Cz8xXmrX56brfGr\n\tSCDU+rHKlujCZysO2298ldvNBGLdrDb5bKAIJp7oPLorpWVJ7hNapbvM/5Zjw24Kbf3W\n\tToKI743oCJjSP7kXt210Y6FqLs4sjW8N6W6GR1XmrO/aVTy1Quz7sSa1jDCy0Rez2557\n\tUHoQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=bXzRYXkxS44bv5WH+Bd8nabj3EidKvqD3G/mSifeBSg=;\n\tb=7SnGzswJXMuVd6GKiWyUJKXvnOQ7EUfQZmTyEcJIT5yE+I/4t2suNX0t7ScCN3FPRm\n\t8MuPYEUL5eDiKlF+akzlsgromCnCcFRk2w/bhALcUIu/sWAlDmxMmYVAqUWMBHNkFYgv\n\tDZ9onq5th0vI5BLpwFT7j21Fqqc74WiRNGWU8lqTB/f1DaFD2BGR2Tb71YOIPNuUpw+9\n\tkJqwLoSBENrXPozFQHb4qwft9sUHRQkpC2DgT5RNf0ezZ/ld8QWaX2q8wxrqrs11+Ru9\n\tQV0Bm6sBbu3nU+ze2+Bk5Qx08uhiiM7AHS+JrJ/cXf4ErGven+HCYQ4cG5BlHm3PP5dc\n\tzEsw==","X-Gm-Message-State":"AOAM531Mk8Zwlh4UuyR60Vy+ix7QVTe058VGGlEClGWYl2Jh8qOI0Db6\n\tb7XbTA3zoc3yKGYrAjCSXoXIG5nYLcs=","X-Google-Smtp-Source":"ABdhPJyJ4Mk+IoO3POmbsJK9XxGQqJQwv+CNaRK41RePGCzw21NDCfyAbkqcBDMsjVYAyTpognJHWw==","X-Received":"by 2002:a1c:104:: with SMTP id 4mr25825541wmb.31.1638654940065; \n\tSat, 04 Dec 2021 13:55:40 -0800 (PST)","To":"Laurent Pinchart ","References":"<20211203224230.38700-1-djrscally@gmail.com>\n\t<20211203224230.38700-5-djrscally@gmail.com>\n\t","From":"Daniel Scally ","Message-ID":"<4d3efdd2-0fbf-a7d0-c889-4f1aedbef20d@gmail.com>","Date":"Sat, 4 Dec 2021 21:55:38 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.14.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","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":21652,"web_url":"https://patchwork.libcamera.org/comment/21652/","msgid":"","date":"2021-12-07T18:53:23","subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Daniel,\n\nOn Sat, Dec 04, 2021 at 09:55:38PM +0000, Daniel Scally wrote:\n> On 04/12/2021 00:36, Laurent Pinchart wrote:\n> > On Fri, Dec 03, 2021 at 10:42:27PM +0000, Daniel Scally wrote:\n> >> Add a function to the CameraLens class to fetch the V4L2 controls\n> >> for its V4L2 subdev\n> >>\n> >> Signed-off-by: Daniel Scally \n> >> ---\n> >> Changes in v2:\n> >>\n> >> \t- New patch\n> >>\n> >> include/libcamera/internal/camera_lens.h | 4 ++++\n> >> src/libcamera/camera_lens.cpp | 11 +++++++++++\n> >> 2 files changed, 15 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n> >> index 6f2ea1bc..64794294 100644\n> >> --- a/include/libcamera/internal/camera_lens.h\n> >> +++ b/include/libcamera/internal/camera_lens.h\n> >> @@ -12,6 +12,8 @@\n> >> #include \n> >> #include \n> >> \n> >> +#include \n> >> +\n> >> namespace libcamera {\n> >> \n> >> class MediaEntity;\n> >> @@ -28,6 +30,8 @@ public:\n> >> \n> >> \tconst std::string &model() const { return model_; }\n> >> \n> >> +\tconst ControlInfoMap &controls() const;\n> >> +\n> >> protected:\n> >> \tstd::string logPrefix() const override;\n> >> \n> >> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n> >> index 189cb025..7b3b9c24 100644\n> >> --- a/src/libcamera/camera_lens.cpp\n> >> +++ b/src/libcamera/camera_lens.cpp\n> >> @@ -139,4 +139,15 @@ std::string CameraLens::logPrefix() const\n> >> \treturn \"'\" + entity_->name() + \"'\";\n> >> }\n> >> \n> >> +/**\n> >> + * \\fn CameraLens::controls()\n> >> + * \\brief Retrieve the V4L2 controls of the lens' subdev\n> >> + *\n> >> + * \\return A map of the V4L2 controls supported by the sensor\n> > \n> > Not a sensor anymore. With that fixed,\n> > \n> > Reviewed-by: Laurent Pinchart \n> \n> Thanks, and for the others too.\n> \n> > I however would like to take one more step, and was wondering if you'd\n> > like to volunteer for it.\n> \n> Sure, I'd be happy to.\n> \n> > I think the CameraSensor and CameraLens classes shouldn't expose V4L2\n> > controls, but a custom set of controls specific to libcamera. This way\n> > we could remove the dependencies on V4L2 in the IPA modules. The\n> > translation between those controls and the V4L2 controls would be local\n> > to the CameraSensor and CameraLens classes.\n> \n> Makes sense; but what's the advantage to removing the dependency? Is\n> there another means of controlling sensor drivers beyond the V4L2 API\n> that could then be supported?\n\nAt the moment there isn't, but that shouldn't prevent us from getting\nready for the future :-) Even with V4L2, I could very well imagine\nimproving the controls dealing with sensor timings (VBLANK is really\npainful to deal with), with API differences handled in the CameraSensor\nclass.\n\nFuture predictions aside, another advantage would be to isolate all the\ncode dealing with V4L2 on the pipeline handler side (as opposed to the\nIPA side). IPA code would have less dependencies on the sensor, and\nwould thus be more generic and reusable. It could also possibly allow us\n(us noted below) to group sensor and lens controls in a single control\nlist.\n\nThis is a topic for research, as there may be drawbacks in trying to\nabstract the sensor interface. I'm not entirely sure what the right\nbalance would be.\n\n> > We would need three controls at the moment, for the analogue gain,\n> > exposure time, and focus position. It could be a good idea to decouple\n> > those controls from the libcamera public controls (exposed by cameras to\n> > applications), but given that we have libcamera controls for those three\n> > already, we could start by using them and see if we need something else\n> > later.\n> > \n> > I'm also wondering if we could, this way, combine the sensor controls\n> > and lens controls in a single ControlInfoMap passed to the IPA, and a\n> > single ControlList passed back from the IPA.\n> \n> Yeah I thought this could do with combining somehow too; I'll take a look.\n> \n> >> + */\n> >> +const ControlInfoMap &CameraLens::controls() const\n> >> +{\n> >> +\treturn subdev_->controls();\n> >> +}\n> >> +\n> >> } /* namespace libcamera */","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 3BE5EBDB13\n\tfor ;\n\tTue, 7 Dec 2021 18:53:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F68F6086A;\n\tTue, 7 Dec 2021 19:53:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4193360592\n\tfor ;\n\tTue, 7 Dec 2021 19:53:53 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B00B19CA;\n\tTue, 7 Dec 2021 19:53:52 +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=\"HRaq9PHL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638903232;\n\tbh=zLCDYqI/WDhJzMvdA4YcfqZCcLpXEzqoWT+BysMoCU4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HRaq9PHL7PPVoLu2W/H4cIT6EBuHqiw6VpOtdq2zeuK2w2nchVPsOva7cFaJaHw9h\n\tADdxW+5SVb8XljSinIZOwNNrf44wpOhHXa4ee/DwMA1YWdjXrWezJLela9rqVCA+qf\n\t7Ocl1BaqQGA6YzEKvIL/KdSt6JfAWy5CwrNV1NM8=","Date":"Tue, 7 Dec 2021 20:53:23 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Message-ID":"","References":"<20211203224230.38700-1-djrscally@gmail.com>\n\t<20211203224230.38700-5-djrscally@gmail.com>\n\t\n\t<4d3efdd2-0fbf-a7d0-c889-4f1aedbef20d@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<4d3efdd2-0fbf-a7d0-c889-4f1aedbef20d@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","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":21653,"web_url":"https://patchwork.libcamera.org/comment/21653/","msgid":"<7caa890f-b3a4-cabf-ce3f-ddd70eb72333@gmail.com>","date":"2021-12-07T22:03:39","subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","submitter":{"id":90,"url":"https://patchwork.libcamera.org/api/people/90/","name":"Daniel Scally","email":"djrscally@gmail.com"},"content":"Hi Laurent\n\nOn 07/12/2021 18:53, Laurent Pinchart wrote:\n> Hi Daniel,\n> \n> On Sat, Dec 04, 2021 at 09:55:38PM +0000, Daniel Scally wrote:\n>> On 04/12/2021 00:36, Laurent Pinchart wrote:\n>>> On Fri, Dec 03, 2021 at 10:42:27PM +0000, Daniel Scally wrote:\n>>>> Add a function to the CameraLens class to fetch the V4L2 controls\n>>>> for its V4L2 subdev\n>>>>\n>>>> Signed-off-by: Daniel Scally \n>>>> ---\n>>>> Changes in v2:\n>>>>\n>>>> \t- New patch\n>>>>\n>>>> include/libcamera/internal/camera_lens.h | 4 ++++\n>>>> src/libcamera/camera_lens.cpp | 11 +++++++++++\n>>>> 2 files changed, 15 insertions(+)\n>>>>\n>>>> diff --git a/include/libcamera/internal/camera_lens.h b/include/libcamera/internal/camera_lens.h\n>>>> index 6f2ea1bc..64794294 100644\n>>>> --- a/include/libcamera/internal/camera_lens.h\n>>>> +++ b/include/libcamera/internal/camera_lens.h\n>>>> @@ -12,6 +12,8 @@\n>>>> #include \n>>>> #include \n>>>> \n>>>> +#include \n>>>> +\n>>>> namespace libcamera {\n>>>> \n>>>> class MediaEntity;\n>>>> @@ -28,6 +30,8 @@ public:\n>>>> \n>>>> \tconst std::string &model() const { return model_; }\n>>>> \n>>>> +\tconst ControlInfoMap &controls() const;\n>>>> +\n>>>> protected:\n>>>> \tstd::string logPrefix() const override;\n>>>> \n>>>> diff --git a/src/libcamera/camera_lens.cpp b/src/libcamera/camera_lens.cpp\n>>>> index 189cb025..7b3b9c24 100644\n>>>> --- a/src/libcamera/camera_lens.cpp\n>>>> +++ b/src/libcamera/camera_lens.cpp\n>>>> @@ -139,4 +139,15 @@ std::string CameraLens::logPrefix() const\n>>>> \treturn \"'\" + entity_->name() + \"'\";\n>>>> }\n>>>> \n>>>> +/**\n>>>> + * \\fn CameraLens::controls()\n>>>> + * \\brief Retrieve the V4L2 controls of the lens' subdev\n>>>> + *\n>>>> + * \\return A map of the V4L2 controls supported by the sensor\n>>>\n>>> Not a sensor anymore. With that fixed,\n>>>\n>>> Reviewed-by: Laurent Pinchart \n>>\n>> Thanks, and for the others too.\n>>\n>>> I however would like to take one more step, and was wondering if you'd\n>>> like to volunteer for it.\n>>\n>> Sure, I'd be happy to.\n>>\n>>> I think the CameraSensor and CameraLens classes shouldn't expose V4L2\n>>> controls, but a custom set of controls specific to libcamera. This way\n>>> we could remove the dependencies on V4L2 in the IPA modules. The\n>>> translation between those controls and the V4L2 controls would be local\n>>> to the CameraSensor and CameraLens classes.\n>>\n>> Makes sense; but what's the advantage to removing the dependency? Is\n>> there another means of controlling sensor drivers beyond the V4L2 API\n>> that could then be supported?\n> \n> At the moment there isn't, but that shouldn't prevent us from getting\n> ready for the future :-) Even with V4L2, I could very well imagine\n> improving the controls dealing with sensor timings (VBLANK is really\n> painful to deal with), with API differences handled in the CameraSensor\n> class.\n> \n> Future predictions aside, another advantage would be to isolate all the\n> code dealing with V4L2 on the pipeline handler side (as opposed to the\n> IPA side). IPA code would have less dependencies on the sensor, and\n> would thus be more generic and reusable. It could also possibly allow us\n> (us noted below) to group sensor and lens controls in a single control\n> list.\n> \n> This is a topic for research, as there may be drawbacks in trying to\n> abstract the sensor interface. I'm not entirely sure what the right\n> balance would be.\n\nThanks for the explanation; makes sense to me. I'll start taking a look\nat that amongst the other stuff I'm working on.\n>>> We would need three controls at the moment, for the analogue gain,\n>>> exposure time, and focus position. It could be a good idea to decouple\n>>> those controls from the libcamera public controls (exposed by cameras to\n>>> applications), but given that we have libcamera controls for those three\n>>> already, we could start by using them and see if we need something else\n>>> later.\n>>>\n>>> I'm also wondering if we could, this way, combine the sensor controls\n>>> and lens controls in a single ControlInfoMap passed to the IPA, and a\n>>> single ControlList passed back from the IPA.\n>>\n>> Yeah I thought this could do with combining somehow too; I'll take a look.\n>>\n>>>> + */\n>>>> +const ControlInfoMap &CameraLens::controls() const\n>>>> +{\n>>>> +\treturn subdev_->controls();\n>>>> +}\n>>>> +\n>>>> } /* namespace libcamera */\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 BB2A1BF415\n\tfor ;\n\tTue, 7 Dec 2021 22:03:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC02D60872;\n\tTue, 7 Dec 2021 23:03:42 +0100 (CET)","from mail-wr1-x436.google.com (mail-wr1-x436.google.com\n\t[IPv6:2a00:1450:4864:20::436])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C10B60592\n\tfor ;\n\tTue, 7 Dec 2021 23:03:41 +0100 (CET)","by mail-wr1-x436.google.com with SMTP id v11so557346wrw.10\n\tfor ;\n\tTue, 07 Dec 2021 14:03:41 -0800 (PST)","from [192.168.0.16]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net. [86.13.91.161])\n\tby smtp.gmail.com with ESMTPSA id\n\tc6sm5236962wmq.46.2021.12.07.14.03.39\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 07 Dec 2021 14:03:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ZUwMFixc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=HCJ0lRwUDFlzIiYHDw+GJtCTFCocZyTyQdI7Os7To/Y=;\n\tb=ZUwMFixcqZhEJKfrJu5VmhjKGaxgOZClBDHs22acf9qUQJ7cITJcgxiK5Phy+SSZZY\n\tUXwdLZ/7flFXfhxXD9kX7zQ5iT1iUA9N+dLiV2/YjiSWvvGchOePUiuwbhRnhofGYcuF\n\t4zcXOh39qukMyQPTBb+5hXGG9nXEEbtAQLVKMBHdJaa9rnMDdkd54ooBSO+AQfY0chx7\n\t6YIOgXuQ1rdAPMC27Kz2uetaSb39OcLFJgz7sv/cgpBcbkaBaveUtvSk++6TWPeykPQX\n\tMKbB8WJhSS5OziFTb8GtJBdsd8rz6MHhy+HFY2XXmsdenkd68RJ14DIA3UH8B7GbptXw\n\tmhhw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=HCJ0lRwUDFlzIiYHDw+GJtCTFCocZyTyQdI7Os7To/Y=;\n\tb=2BzaS7S9UNkm8cf8otND5pGgT5tShCkOkBrcQD0btTqT8PEMwsZSPfCT41kh/r2oIq\n\tDHOQbhMSY7+5ksY/u80yGucnVfkKbCgnQLMzFsjQlgpUCxhPeXxfK1bWOiDPgwhJgKcv\n\t2tAIn41eD+qjtZNdUFkMwxmiRM6Lq2lcb6BTXiCcv+lkATxS5IhDmhRnnCIOoxwwdraL\n\tlI1UjtEgZDgPkQ5JvQIVDXWqs0U+T2bJIhouJuOMQbx3XwN5v9wjGXUjY1nraI2W5l9C\n\tzixNSTEfQPiJaT3A9jFoKZ5NZ6mZVx3ZL+ZSq7YiHLsQ2ixZWJhfQwBs0DMF157ZdDw6\n\t0SDg==","X-Gm-Message-State":"AOAM5330beucUrvJqAuK980GRKw8OjLHQGaVFykNef2LbNlsDJat99Lb\n\tbLxjMnFkzmDLGcGYcAkw+vSy15XvDdA=","X-Google-Smtp-Source":"ABdhPJy6ftwxHlKBg0DRAmkInuttVG9zBm87bs0fFX4olIe8rBq1O3u2CFjq26nQezx4ZMfXcKqgIg==","X-Received":"by 2002:adf:e8d1:: with SMTP id\n\tk17mr52956752wrn.465.1638914620809; \n\tTue, 07 Dec 2021 14:03:40 -0800 (PST)","To":"Laurent Pinchart ","References":"<20211203224230.38700-1-djrscally@gmail.com>\n\t<20211203224230.38700-5-djrscally@gmail.com>\n\t\n\t<4d3efdd2-0fbf-a7d0-c889-4f1aedbef20d@gmail.com>\n\t","From":"Daniel Scally ","Message-ID":"<7caa890f-b3a4-cabf-ce3f-ddd70eb72333@gmail.com>","Date":"Tue, 7 Dec 2021 22:03:39 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.14.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/7] libcamera: camera_lens: Add\n\tfunction to fetch subdev controls","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\" "}}]