[{"id":15230,"web_url":"https://patchwork.libcamera.org/comment/15230/","msgid":"<20210219023909.GD1826@pyrite.rasen.tech>","date":"2021-02-19T02:39:09","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Feb 18, 2021 at 12:48:24PM +0000, Naushir Patuck wrote:\n> Only update the lens shading control if it is present in the\n> ControlList.\n> \n> Add the dmabuf file descriptor to the lens shading control in-place\n> rather than taking a copy.\n> \n> Signed-off-by: Naushir Patuck \n\nReviewed-by: Paul Elder \n\n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 21 +++++++++----------\n> 1 file changed, 10 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index acf2d56cddb2..a60415d93705 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1338,17 +1338,16 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> \n> void RPiCameraData::setIspControls(const ControlList &controls)\n> {\n> -\tControlList ctrls = controls;\n> -\n> -\tSpan s =\n> -\t\tctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> -\tbcm2835_isp_lens_shading ls =\n> -\t\t*reinterpret_cast(s.data());\n> -\tls.dmabuf = lsTable_.fd();\n> -\n> -\tControlValue c(Span{ reinterpret_cast(&ls),\n> -\t\t\t\t\t sizeof(ls) });\n> -\tctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> +\tControlList ctrls = std::move(controls);\n> +\n> +\tif (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> +\t\tControlValue &value =\n> +\t\t\tconst_cast(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n> +\t\tSpan s = value.data();\n> +\t\tbcm2835_isp_lens_shading *ls =\n> +\t\t\treinterpret_cast(s.data());\n> +\t\tls->dmabuf = lsTable_.fd();\n> +\t}\n> \n> \tisp_[Isp::Input].dev()->setControls(&ctrls);\n> \thandleState();\n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 4AA3ABD1F6\n\tfor ;\n\tFri, 19 Feb 2021 02:39:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7979689B6;\n\tFri, 19 Feb 2021 03:39: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 365AB602F5\n\tfor ;\n\tFri, 19 Feb 2021 03:39:17 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A1C0F596;\n\tFri, 19 Feb 2021 03:39:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Vlz0qvdK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613702356;\n\tbh=GoHnGl3BmhliWeULmyD8voy9iuU+pvgsKKJX+WikDrY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vlz0qvdKRI17dbjmytJj7i2sZ63njL/V+j4S8nF9xZepbHSwm5VK01/KhuRUd2VCz\n\t0hWDxJCi17QZR0Mj2exzNGeFRefa/yr7DoMxhedwqVOiUhekj42etehqElBj+thgF8\n\trewhuOWTB1yCh5AKcB0FGzaTwroniaHySJjkIPo4=","Date":"Fri, 19 Feb 2021 11:39:09 +0900","From":"paul.elder@ideasonboard.com","To":"Naushir Patuck ","Message-ID":"<20210219023909.GD1826@pyrite.rasen.tech>","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210218124824.1825418-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15236,"web_url":"https://patchwork.libcamera.org/comment/15236/","msgid":"<91e4f688-b6e3-32aa-1ef0-10333a6a31fe@ideasonboard.com>","date":"2021-02-19T11:08:16","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 18/02/2021 12:48, Naushir Patuck wrote:\n> Only update the lens shading control if it is present in the\n> ControlList.\n> \n> Add the dmabuf file descriptor to the lens shading control in-place\n> rather than taking a copy.\n> \n> Signed-off-by: Naushir Patuck \n\nReviewed-by: Kieran Bingham \n\n\n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 21 +++++++++----------\n> 1 file changed, 10 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index acf2d56cddb2..a60415d93705 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1338,17 +1338,16 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> \n> void RPiCameraData::setIspControls(const ControlList &controls)\n> {\n> -\tControlList ctrls = controls;\n> -\n> -\tSpan s =\n> -\t\tctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> -\tbcm2835_isp_lens_shading ls =\n> -\t\t*reinterpret_cast(s.data());\n> -\tls.dmabuf = lsTable_.fd();\n> -\n> -\tControlValue c(Span{ reinterpret_cast(&ls),\n> -\t\t\t\t\t sizeof(ls) });\n> -\tctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> +\tControlList ctrls = std::move(controls);\n> +\n> +\tif (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> +\t\tControlValue &value =\n> +\t\t\tconst_cast(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n> +\t\tSpan s = value.data();\n> +\t\tbcm2835_isp_lens_shading *ls =\n> +\t\t\treinterpret_cast(s.data());\n> +\t\tls->dmabuf = lsTable_.fd();\n> +\t}\n> \n> \tisp_[Isp::Input].dev()->setControls(&ctrls);\n> \thandleState();\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 30774BD1F1\n\tfor ;\n\tFri, 19 Feb 2021 11:08:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6873637C9;\n\tFri, 19 Feb 2021 12:08:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FEAD602F5\n\tfor ;\n\tFri, 19 Feb 2021 12:08:21 +0100 (CET)","from [192.168.0.20]\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 1DA78344;\n\tFri, 19 Feb 2021 12:08:20 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wAO77fLz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613732901;\n\tbh=pRFaAdkQJPLI5bE2X+tQZc+WGikPqWDW9s3mLjTOT18=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=wAO77fLzzzhnxYVLPKqHRNNOjSrWg0NK/efvUDxJAedDBDzLMIfcfL7sxsK3xL2UR\n\tGahIxD2iJopJ15uvXiOw6fv9MPCIsoFu9fngDJ+XAN3DL0vARz2Torvv50pzBmH3+B\n\t6eqH/Z3Ig8eg1CGgu2jRJzSdbqJxC/SvVM4o3lf4=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<91e4f688-b6e3-32aa-1ef0-10333a6a31fe@ideasonboard.com>","Date":"Fri, 19 Feb 2021 11:08:16 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210218124824.1825418-5-naush@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15257,"web_url":"https://patchwork.libcamera.org/comment/15257/","msgid":"","date":"2021-02-21T13:06:46","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Thu, Feb 18, 2021 at 12:48:24PM +0000, Naushir Patuck wrote:\n> Only update the lens shading control if it is present in the\n> ControlList.\n> \n> Add the dmabuf file descriptor to the lens shading control in-place\n> rather than taking a copy.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 21 +++++++++----------\n> 1 file changed, 10 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index acf2d56cddb2..a60415d93705 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1338,17 +1338,16 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> \n> void RPiCameraData::setIspControls(const ControlList &controls)\n> {\n> -\tControlList ctrls = controls;\n> -\n> -\tSpan s =\n> -\t\tctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> -\tbcm2835_isp_lens_shading ls =\n> -\t\t*reinterpret_cast(s.data());\n> -\tls.dmabuf = lsTable_.fd();\n> -\n> -\tControlValue c(Span{ reinterpret_cast(&ls),\n> -\t\t\t\t\t sizeof(ls) });\n> -\tctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> +\tControlList ctrls = std::move(controls);\n\nstd::move() won't have the expected effect here. controls is a const\nreference, so ctrls will be a copy. It's a bit annoying that the\ncompiler doesn't warn us :-/\n\nThe code will run fine, but I think avoiding std::move() would be more\nexplicit as it's easy to overlook the fact that a copy is created\notherwise. No need to change this now, but if you get the chance at some\npoint, it would be nice.\n\n> +\n> +\tif (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> +\t\tControlValue &value =\n> +\t\t\tconst_cast(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n\nThe const_cast is not nice. If we want to support this kind of usage, we\nshould extend the ControlList API to return a non-const ControlValue.\n\n> +\t\tSpan s = value.data();\n> +\t\tbcm2835_isp_lens_shading *ls =\n> +\t\t\treinterpret_cast(s.data());\n> +\t\tls->dmabuf = lsTable_.fd();\n> +\t}\n> \n> \tisp_[Isp::Input].dev()->setControls(&ctrls);\n> \thandleState();","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 E90E8BD1F6\n\tfor ;\n\tSun, 21 Feb 2021 13:07:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B45D5689E3;\n\tSun, 21 Feb 2021 14:07:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D640689C2\n\tfor ;\n\tSun, 21 Feb 2021 14:07:20 +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 09185344;\n\tSun, 21 Feb 2021 14:07:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"A+CXL7dn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613912840;\n\tbh=6KqxgHQwS7xXN5BLAPvVMmuJPw5Fyjyx17M54vrn/Ys=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=A+CXL7dn1kTtNtkolVkLmFPWKzvF/rJ53MOfeaWuBPc17vchWigvkgg82SSADH3I0\n\tigX4siuNOtmxyKeFiwgyKoAOOVaLgCp18A4QatkzK4NxWQ+ftqHZC9QL7wffeNOxdw\n\t5Z1vGr+RcIySW+Eb+CBNwuaLgoqF/39IepfZiWfM=","Date":"Sun, 21 Feb 2021 15:06:46 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210218124824.1825418-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15261,"web_url":"https://patchwork.libcamera.org/comment/15261/","msgid":"","date":"2021-02-21T19:39:38","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Sun, 21 Feb 2021 at 13:07, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Thu, Feb 18, 2021 at 12:48:24PM +0000, Naushir Patuck wrote:\n> > Only update the lens shading control if it is present in the\n> > ControlList.\n> >\n> > Add the dmabuf file descriptor to the lens shading control in-place\n> > rather than taking a copy.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 21 +++++++++----------\n> > 1 file changed, 10 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index acf2d56cddb2..a60415d93705 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1338,17 +1338,16 @@ void RPiCameraData::embeddedComplete(uint32_t\n> bufferId)\n> >\n> > void RPiCameraData::setIspControls(const ControlList &controls)\n> > {\n> > - ControlList ctrls = controls;\n> > -\n> > - Span s =\n> > - ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> > - bcm2835_isp_lens_shading ls =\n> > - *reinterpret_cast *>(s.data());\n> > - ls.dmabuf = lsTable_.fd();\n> > -\n> > - ControlValue c(Span{ reinterpret_cast *>(&ls),\n> > - sizeof(ls) });\n> > - ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> > + ControlList ctrls = std::move(controls);\n>\n> std::move() won't have the expected effect here. controls is a const\n> reference, so ctrls will be a copy. It's a bit annoying that the\n> compiler doesn't warn us :-/\n>\n> The code will run fine, but I think avoiding std::move() would be more\n> explicit as it's easy to overlook the fact that a copy is created\n> otherwise. No need to change this now, but if you get the chance at some\n> point, it would be nice.\n>\n\nUnderstood, will put it on my todo list.\n\nJust curious though, what is the issue with the old method of getting the\nIPA to fill in the file descriptor in the struct?\nThis would avoid the need of modifying the control list like this in the\npipeline handler. Is it to do with testing process\nisolation between the pipeline handler and IPA?\n\nThanks,\nNaush\n\n\n>\n> > +\n> > + if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> > + ControlValue &value =\n> > + const_cast &>(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n>\n> The const_cast is not nice. If we want to support this kind of usage, we\n> should extend the ControlList API to return a non-const ControlValue.\n>\n> > + Span s = value.data();\n> > + bcm2835_isp_lens_shading *ls =\n> > + reinterpret_cast *>(s.data());\n> > + ls->dmabuf = lsTable_.fd();\n> > + }\n> >\n> > isp_[Isp::Input].dev()->setControls(&ctrls);\n> > handleState();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 6F03DBD1F6\n\tfor ;\n\tSun, 21 Feb 2021 19:39:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9E29689E4;\n\tSun, 21 Feb 2021 20:39:56 +0100 (CET)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C658C689C2\n\tfor ;\n\tSun, 21 Feb 2021 20:39:55 +0100 (CET)","by mail-lj1-x22a.google.com with SMTP id c17so52145326ljn.0\n\tfor ;\n\tSun, 21 Feb 2021 11:39:55 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"f+Y7yREB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=JmQIHAF4Q8nqgujlWdQ6ztWfUlHCUrRv2L8Y3+KxbsM=;\n\tb=f+Y7yREBtsKCHyzXU7XrntU6KYpMbHPTlES7giSSGZS5WLxcm62BYkzCXyCJdJlLJM\n\tR0g2LpfKuLHJBny2G9YVv210zlZSyX6byhKq9XKKRc0zrOGBvLW2Va91TigcW4pA6M5O\n\tDPGJIxWixEF28w+nxHzaXiycYyQvUiCCIkvLe7d+Y6jtIxzE4nndd7NATjqy0l3bQVBY\n\tQSNbBfEo4qSfd1FAsDZ39E9SXAgf8ECc+ciQqc6+lV6FHXbms9MDKYeaJ2gri+zhu0DT\n\tviHaraYLZBBeJo350ix5V9CfpxLRQYmoO+lArqZlH2R/w8BE16nLnHFPTpZorqOpknYn\n\tYTCg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=JmQIHAF4Q8nqgujlWdQ6ztWfUlHCUrRv2L8Y3+KxbsM=;\n\tb=WSP0bCdOsQg43QnG6dMETYWAanD/vipagCrPM03GEQH5R5Wv0EJssLSBUax72WR2Eg\n\t9pYoEOSmVk6t1rfr14qw9ldOfL+TR37hTGvPEEFq3qNFF+cYt7Qy4SoesenJf34ukYAG\n\tYdiPS9Z2qWCrQytcSaP0r+eBeAwLMmbfyZC08m/ItUIBXoaadiviBK4sBOCMm913R0pB\n\td6X6+/Shkbst+3ysO7K6eY2RxThIf07WjpVKoZcHW0HwE2C0PWA/7/3pebwEyBOAZ5Ie\n\tuDwn2tXsLFNe+Q3fAc98hBZgVfcVMZz0Dv+unEDxkZphwzucg5YOorzrNoVq45OA7zba\n\tCEZQ==","X-Gm-Message-State":"AOAM530RmUhxejExbZYtYlfjwtN43O4Gj4BRVUI3t56RjfMwXmQsbkBr\n\tIY+CbmLLaVdlKX2Ur/Ha/6UdmupVDX4m5V/daqakJg==","X-Google-Smtp-Source":"ABdhPJwTQQ7dJAEm/1zRU2godon5MJoKznqWPy5bb74yLvibhlrtkX+W4APrXwAOhAawlr9rj+zW/0l4fGGoaS2cOIw=","X-Received":"by 2002:a05:6512:2118:: with SMTP id\n\tq24mr11575656lfr.133.1613936394936; \n\tSun, 21 Feb 2021 11:39:54 -0800 (PST)","MIME-Version":"1.0","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Sun, 21 Feb 2021 19:39:38 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============7101995347425752683==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15262,"web_url":"https://patchwork.libcamera.org/comment/15262/","msgid":"","date":"2021-02-21T19:52:12","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Feb 21, 2021 at 07:39:38PM +0000, Naushir Patuck wrote:\n> On Sun, 21 Feb 2021 at 13:07, Laurent Pinchart wrote: \n> > On Thu, Feb 18, 2021 at 12:48:24PM +0000, Naushir Patuck wrote:\n> > > Only update the lens shading control if it is present in the\n> > > ControlList.\n> > >\n> > > Add the dmabuf file descriptor to the lens shading control in-place\n> > > rather than taking a copy.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 21 +++++++++----------\n> > > 1 file changed, 10 insertions(+), 11 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index acf2d56cddb2..a60415d93705 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -1338,17 +1338,16 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> > >\n> > > void RPiCameraData::setIspControls(const ControlList &controls)\n> > > {\n> > > - ControlList ctrls = controls;\n> > > -\n> > > - Span s =\n> > > - ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> > > - bcm2835_isp_lens_shading ls =\n> > > - *reinterpret_cast(s.data());\n> > > - ls.dmabuf = lsTable_.fd();\n> > > -\n> > > - ControlValue c(Span{ reinterpret_cast(&ls),\n> > > - sizeof(ls) });\n> > > - ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> > > + ControlList ctrls = std::move(controls);\n> >\n> > std::move() won't have the expected effect here. controls is a const\n> > reference, so ctrls will be a copy. It's a bit annoying that the\n> > compiler doesn't warn us :-/\n> >\n> > The code will run fine, but I think avoiding std::move() would be more\n> > explicit as it's easy to overlook the fact that a copy is created\n> > otherwise. No need to change this now, but if you get the chance at some\n> > point, it would be nice.\n> \n> Understood, will put it on my todo list.\n> \n> Just curious though, what is the issue with the old method of getting the\n> IPA to fill in the file descriptor in the struct?\n> This would avoid the need of modifying the control list like this in the\n> pipeline handler. Is it to do with testing process\n> isolation between the pipeline handler and IPA?\n\nYes, it's related to process isolation. When the IPA is run within the\nlibcamera process, file descriptors are just integers, and they can be\npassed back and forth between the pipeline handler and IPA without any\nissue.\n\nWhen isolating an IPA in a process, the file descriptors have to be\npassed through the IPC pipe (the IPA IPC mechanism handles this\ntransparently), and the receiving side will see the file descriptor as\nan integer local to the process, whose numerical value will usually be\ndifferent than the one in the sending process. The lens shading table\nfile descriptor can be used in the IPA to map the buffer, but if its\nnumerical value is set in the V4L2_CID_USER_BCM2835_ISP_LENS_SHADING\ncontrol, there will be no translation on the way back to the pipeline\nhandler as the IPA IPC layer isn't aware that the V4L2 control contains\na file descriptor. The control will be set by the libcamera process,\nusing a file descriptor value that corresponds to the IPA process.\n\nOne way to handle this would be to pass both the file descriptor and its\nvalue as an integer to the IPA, but it's a bit of a hack, the IPA\nshouldn't have to care about this. That's why it's patched in the\npipeline handler.\n\n> > > +\n> > > + if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> > > + ControlValue &value =\n> > > + const_cast(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n> >\n> > The const_cast is not nice. If we want to support this kind of usage, we\n> > should extend the ControlList API to return a non-const ControlValue.\n\nDo you think we should have such an extension ?\n\n> > > + Span s = value.data();\n> > > + bcm2835_isp_lens_shading *ls =\n> > > + reinterpret_cast(s.data());\n> > > + ls->dmabuf = lsTable_.fd();\n> > > + }\n> > >\n> > > isp_[Isp::Input].dev()->setControls(&ctrls);\n> > > handleState();","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 A0F7DBD1F1\n\tfor ;\n\tSun, 21 Feb 2021 19:52:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 479C8689E7;\n\tSun, 21 Feb 2021 20:52:41 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7DB1689C2\n\tfor ;\n\tSun, 21 Feb 2021 20:52:39 +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 772AEEF;\n\tSun, 21 Feb 2021 20:52:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WIMpsCcC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613937158;\n\tbh=AC01+h419z8Zu0M7RSu91qodqaL5Of91wmgwu/N2l9c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WIMpsCcC/qVkEqmfNYjPOc+5oKeHLmkT7JOOkAev0LrCLE0ZaXXsuZpVJWbncxuHZ\n\tV0I2Xrok4ayW+VPmKhA1hinaNYfh/qNt/nV+8EHBgiRlXRjnU92tJTmOjVWObtqDPx\n\tYW30sQ5ApQaTyu6MSRXvDKv5pOFijXi+sCC3AUb8=","Date":"Sun, 21 Feb 2021 21:52:12 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15263,"web_url":"https://patchwork.libcamera.org/comment/15263/","msgid":"","date":"2021-02-21T19:55:47","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Sun, 21 Feb 2021, 7:52 pm Laurent Pinchart, <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Sun, Feb 21, 2021 at 07:39:38PM +0000, Naushir Patuck wrote:\n> > On Sun, 21 Feb 2021 at 13:07, Laurent Pinchart wrote:\n> > > On Thu, Feb 18, 2021 at 12:48:24PM +0000, Naushir Patuck wrote:\n> > > > Only update the lens shading control if it is present in the\n> > > > ControlList.\n> > > >\n> > > > Add the dmabuf file descriptor to the lens shading control in-place\n> > > > rather than taking a copy.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > .../pipeline/raspberrypi/raspberrypi.cpp | 21\n> +++++++++----------\n> > > > 1 file changed, 10 insertions(+), 11 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index acf2d56cddb2..a60415d93705 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -1338,17 +1338,16 @@ void\n> RPiCameraData::embeddedComplete(uint32_t bufferId)\n> > > >\n> > > > void RPiCameraData::setIspControls(const ControlList &controls)\n> > > > {\n> > > > - ControlList ctrls = controls;\n> > > > -\n> > > > - Span s =\n> > > > -\n> ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();\n> > > > - bcm2835_isp_lens_shading ls =\n> > > > - *reinterpret_cast *>(s.data());\n> > > > - ls.dmabuf = lsTable_.fd();\n> > > > -\n> > > > - ControlValue c(Span{ reinterpret_cast *>(&ls),\n> > > > - sizeof(ls) });\n> > > > - ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);\n> > > > + ControlList ctrls = std::move(controls);\n> > >\n> > > std::move() won't have the expected effect here. controls is a const\n> > > reference, so ctrls will be a copy. It's a bit annoying that the\n> > > compiler doesn't warn us :-/\n> > >\n> > > The code will run fine, but I think avoiding std::move() would be more\n> > > explicit as it's easy to overlook the fact that a copy is created\n> > > otherwise. No need to change this now, but if you get the chance at\n> some\n> > > point, it would be nice.\n> >\n> > Understood, will put it on my todo list.\n> >\n> > Just curious though, what is the issue with the old method of getting the\n> > IPA to fill in the file descriptor in the struct?\n> > This would avoid the need of modifying the control list like this in the\n> > pipeline handler. Is it to do with testing process\n> > isolation between the pipeline handler and IPA?\n>\n> Yes, it's related to process isolation. When the IPA is run within the\n> libcamera process, file descriptors are just integers, and they can be\n> passed back and forth between the pipeline handler and IPA without any\n> issue.\n>\n> When isolating an IPA in a process, the file descriptors have to be\n> passed through the IPC pipe (the IPA IPC mechanism handles this\n> transparently), and the receiving side will see the file descriptor as\n> an integer local to the process, whose numerical value will usually be\n> different than the one in the sending process. The lens shading table\n> file descriptor can be used in the IPA to map the buffer, but if its\n> numerical value is set in the V4L2_CID_USER_BCM2835_ISP_LENS_SHADING\n> control, there will be no translation on the way back to the pipeline\n> handler as the IPA IPC layer isn't aware that the V4L2 control contains\n> a file descriptor. The control will be set by the libcamera process,\n> using a file descriptor value that corresponds to the IPA process.\n>\n> One way to handle this would be to pass both the file descriptor and its\n> value as an integer to the IPA, but it's a bit of a hack, the IPA\n> shouldn't have to care about this. That's why it's patched in the\n> pipeline handler.\n>\n> > > > +\n> > > > + if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {\n> > > > + ControlValue &value =\n> > > > + const_cast &>(ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING));\n> > >\n> > > The const_cast is not nice. If we want to support this kind of usage,\n> we\n> > > should extend the ControlList API to return a non-const ControlValue.\n>\n> Do you think we should have such an extension ?\n>\n\nYes please :-)\n\nThis would certainly simplify code like the above!\n\n\n> > > > + Span s = value.data();\n> > > > + bcm2835_isp_lens_shading *ls =\n> > > > + reinterpret_cast *>(s.data());\n> > > > + ls->dmabuf = lsTable_.fd();\n> > > > + }\n> > > >\n> > > > isp_[Isp::Input].dev()->setControls(&ctrls);\n> > > > handleState();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 70FAEBD1F1\n\tfor ;\n\tSun, 21 Feb 2021 19:56:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EE949689EA;\n\tSun, 21 Feb 2021 20:56:00 +0100 (CET)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18C8C689C2\n\tfor ;\n\tSun, 21 Feb 2021 20:55:59 +0100 (CET)","by mail-lj1-x22b.google.com with SMTP id r11so7484548ljk.9\n\tfor ;\n\tSun, 21 Feb 2021 11:55:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"kQd3Yk1r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=/DFgzA2SlWtBpG8MHhnlzLbGsxYBoEy08gmArPAi7T8=;\n\tb=kQd3Yk1r4BhXsviQL2QEuxTDiHeFXGF97laqwXN0jNcqWvtcdT4gS/pI3/loqvqu0y\n\tbw999b0K+OmnvPF6ZRYG+FvNWiZtSweBG2tpVbSmKvV77YV46/9gjzP3Qiq0qtjZ8WBl\n\tXaJJsXgQDJp8EM5EhSfUulxSD/dAcj6DrLXPbFqFOsUS2S0qy4ZmUqyM+l7T5qww4Ffw\n\tViC6MDzPsosAFcS5HtYbkbFIFJM3HFzt517mWE1kjbV+E7Rs3WYQqQBl8hSqOTgDRjcq\n\tAYGwEBuoBYVBZWSzQwGdbRgo+p9MpWqq/nmGRLL1AWfTekhsaKTvJ6TZI4tyWX9/dJpi\n\t+9mg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=/DFgzA2SlWtBpG8MHhnlzLbGsxYBoEy08gmArPAi7T8=;\n\tb=YLY8Su34Hq/ckOh9Hoaa29K21iunVE7eeftjBwCjT9F21cuQKlvpFDIM8Gw79YXRJY\n\tXU3tZ8e4bpDl5UNxPiZu3cHVDz2r7AQ81p/YKLwztydZs483o/oxD+XsWmCDAU55jN6J\n\tvTuz6lCoLKnaujHaGysK2Y4rntSBg/V1MP52cmpphdzIebQGcW7nRlHzxfMVjXzWLTXr\n\tdRr+w4kTBQp5SX0zRPBKCT5sWniwLhTGFwQ+15bc1LTb0m6WYzl5z7xFF93SyVt1duFo\n\tQ9AEe9PnBHxMm3cvD+9diQIv5ekjT7Tuuj7+xNgQREi4lIwymdpWaL3vvJwewLiAEjaz\n\t8KUQ==","X-Gm-Message-State":"AOAM532ugMKgB9OhsYegYrZSEHRv6oR83wyUI53bHzgCnPPJLnFnQKP+\n\tN5UE+04QiOgD1nrI4u4ZHZrkG74GqkaqiOukSve2+GBm4e7WQw==","X-Google-Smtp-Source":"ABdhPJxRnQfeZzSrzZoBBWskG8aNoqT+TOScG/HAvhV59Dwl9/MDk43Z85QfGgiAY4od9Cl7VWUE4FqPNaF2JRWu9gE=","X-Received":"by 2002:a05:6512:208c:: with SMTP id\n\tt12mr11516044lfr.210.1613937358553; \n\tSun, 21 Feb 2021 11:55:58 -0800 (PST)","MIME-Version":"1.0","References":"<20210218124824.1825418-1-naush@raspberrypi.com>\n\t<20210218124824.1825418-5-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Sun, 21 Feb 2021 19:55:47 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Update\n\tthe lens shading control in-place","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============1785029560205480460==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]