[{"id":13036,"web_url":"https://patchwork.libcamera.org/comment/13036/","msgid":"","date":"2020-10-07T08:52:05","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Andrey,\n\nOn 07/10/2020 08:54, Andrey Konovalov wrote:\n> The current simple pipeline handler refuses to work with capture devices\n> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device capabilities\n> field. This is too restrictive, as devices supporting the multi-planar API\n> can be using contiguous memory for semi-planar and planar formats, and this\n> would just work without any changes to libcamera.\n\nAgreed, and I'm sure in-kernel, the use of the mplane api is preferred\nover the splane, so this likely isn't an uncommon case. It's just that\nwe haven't hit it ourselves yet.\n\n\n> Drop the guard against MPLANE devices, and replace it with the check of\n> the number of planes in the format the simple pipeline handler is going to\n> use for capture. This will let MPLANE devices which don't use non-contiguous\n> memory for frame buffers to work with the simple pipeline handler.\n\nIt sounds to me like this is a more accurate check of the restrictions\nwe are currently have imposed.\n\n> Signed-off-by: Andrey Konovalov \n\nReviewed-by: Kieran Bingham \n\n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 10223a9b..8dc23623 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tif (captureFormat.planesCount != 1) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Planar formats using non-contiguous memory not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tif (captureFormat.fourcc != videoFormat ||\n> \t captureFormat.size != pipeConfig.captureSize) {\n> \t\tLOG(SimplePipeline, Error)\n> @@ -845,12 +851,6 @@ V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n> \tif (video->open() < 0)\n> \t\treturn nullptr;\n> \n> -\tif (video->caps().isMultiplanar()) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"V4L2 multiplanar devices are not supported\";\n> -\t\treturn nullptr;\n> -\t}\n> -\n> \tvideo->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n> \n> \tauto element = videos_.emplace(entity, std::move(video));\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 47860BEEDF\n\tfor ;\n\tWed, 7 Oct 2020 08:52:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD85B63BE4;\n\tWed, 7 Oct 2020 10:52:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 30A926035F\n\tfor ;\n\tWed, 7 Oct 2020 10:52:09 +0200 (CEST)","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 8B49C9DA;\n\tWed, 7 Oct 2020 10:52:08 +0200 (CEST)"],"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=\"YbZmt2ql\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602060728;\n\tbh=4HmLxuumz5spkPewxnxPPR5gB+hzGkAf/lTLCmOcoOE=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=YbZmt2qlrUw7SZUtaKbDmwVVaa3tb1QuxCxJ+91EJInJSjLiNwgtPOh3SNNPCZCcY\n\t0uF48qAhljMuMZkjBy/ylvDcEjJtUn4oUBZtJ9ZOvYpdm1dAlfVBmyJTby1LTaeYVV\n\tB1ezL+vWPn52QPCDXnylYCVqPzuLPbUla4fE+rTM=","To":"Andrey Konovalov ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>","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":"","Date":"Wed, 7 Oct 2020 09:52:05 +0100","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":"<20201007075457.4455-1-andrey.konovalov@linaro.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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":13057,"web_url":"https://patchwork.libcamera.org/comment/13057/","msgid":"<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>","date":"2020-10-07T13:01:12","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello Andrey,\n\nThanks for your patch.\n\nOn 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n> The current simple pipeline handler refuses to work with capture devices\n> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device capabilities\n> field. This is too restrictive, as devices supporting the multi-planar API\n> can be using contiguous memory for semi-planar and planar formats, and this\n> would just work without any changes to libcamera.\n> \n> Drop the guard against MPLANE devices, and replace it with the check of\n> the number of planes in the format the simple pipeline handler is going to\n> use for capture. This will let MPLANE devices which don't use non-contiguous\n> memory for frame buffers to work with the simple pipeline handler.\n\nI wonder if the check should not be moved to SimpleCameraData::init() \nwhere the formats_ array is built. The array contains all supported \nformats of the camera and excluding mplaner formats from it will make it \nnot show up at all for applications. Also validate() would Adjust if any \nformat is asked for that is not in the formats_ array.\n\n> \n> Signed-off-by: Andrey Konovalov \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 10223a9b..8dc23623 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tif (captureFormat.planesCount != 1) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Planar formats using non-contiguous memory not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> \tif (captureFormat.fourcc != videoFormat ||\n> \t captureFormat.size != pipeConfig.captureSize) {\n> \t\tLOG(SimplePipeline, Error)\n> @@ -845,12 +851,6 @@ V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n> \tif (video->open() < 0)\n> \t\treturn nullptr;\n> \n> -\tif (video->caps().isMultiplanar()) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"V4L2 multiplanar devices are not supported\";\n> -\t\treturn nullptr;\n> -\t}\n> -\n> \tvideo->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n> \n> \tauto element = videos_.emplace(entity, std::move(video));\n> -- \n> 2.17.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 DFEBFBEEE0\n\tfor ;\n\tWed, 7 Oct 2020 13:01:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ADD19605B2;\n\tWed, 7 Oct 2020 15:01:15 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 331926055E\n\tfor ;\n\tWed, 7 Oct 2020 15:01:14 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id a4so1847184lji.12\n\tfor ;\n\tWed, 07 Oct 2020 06:01:14 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt1sm319447lfr.115.2020.10.07.06.01.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 07 Oct 2020 06:01:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"ZazGsMuS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=9/y7PiKu4m3+I7QQCoX0v9yY3yvdceJLQLaL46Dynxc=;\n\tb=ZazGsMuStPcfRwzkPMkqYCCkjANGi/Bgjo+n5NyZF2iVbnbbDyBKvVSZo3UmS82AEZ\n\tyo26sSV2JH+XiwfNBtuM4QxkkJCwutOjmyw1oCCKP+8tjXJvRIFmDXnEL8ONjE+xQcIy\n\t5utLdPod7rihe70xvxFm6hGsnsuGTg2fmY+OCbdudFTTF8qrh8N75Wt0p+pXgHG0DOu8\n\t3WN2KLHb0WD2P3g3A+Fm/G6ykV/THWuMVsYaVBZeKW/bVTawDc3BVgLQNtqcjkbITZ29\n\tG2Z17UpdEquZOZsk1MUXMiCr6ur1wVDfV4wDi3HXSt8Vvrt+cBlvLB9gD4/kgIicfWMY\n\tJPCQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=9/y7PiKu4m3+I7QQCoX0v9yY3yvdceJLQLaL46Dynxc=;\n\tb=oA+n3KeZ1kVR0tHBZfC0dNc4ESloM+cH8fCb7bc3QDKzZgBg0JrwK/yiFR5nU+0HPX\n\t4PAm52OZ/nLbKoliA3NupmzkRXGNPdrS94lW5aj9nJ4GXQ70Thu4aV6FlfQth96r4n5v\n\tY1CxCRrzXHUoaQKYbEVy08nKwQSHL+reRPnE/mmKmOThMtevad8rQem/Kn5W0u8i7PD+\n\tGBG13jN2rapqBAhjiT6ubNcVzV3ud3sMgDuIO37nEGxJj1vHyZTZa35tTaz74nRd8nrE\n\t2IY3fgDIqJO8Sqru+kWGZi7zW8YruziQLQWm2Cf630OAv6X+HzGGiaWu/dPXrXWKRHCM\n\tJVjQ==","X-Gm-Message-State":"AOAM5307G854n4Y+6IZUo0mF13rYn1G6OLyslBMRaSaeO4UgQ0JLEYa2\n\t7uD7sEEYTiYADoOuzUthJp7ehg==","X-Google-Smtp-Source":"ABdhPJzphHJJHzCqAM8hFbooIQpq0iLJVusg1kTwXs6dm6uRzyPdDJ6vICa1UgXh1OV2xV0VDZC9PQ==","X-Received":"by 2002:a05:651c:1077:: with SMTP id\n\ty23mr1207367ljm.245.1602075673575; \n\tWed, 07 Oct 2020 06:01:13 -0700 (PDT)","Date":"Wed, 7 Oct 2020 15:01:12 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= ","To":"Andrey Konovalov ","Message-ID":"<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007075457.4455-1-andrey.konovalov@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13058,"web_url":"https://patchwork.libcamera.org/comment/13058/","msgid":"","date":"2020-10-07T13:04:11","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas, Andrey,\n\nOn 07/10/2020 14:01, Niklas Söderlund wrote:\n> Hello Andrey,\n> \n> Thanks for your patch.\n> \n> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n>> The current simple pipeline handler refuses to work with capture devices\n>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device capabilities\n>> field. This is too restrictive, as devices supporting the multi-planar API\n>> can be using contiguous memory for semi-planar and planar formats, and this\n>> would just work without any changes to libcamera.\n>>\n>> Drop the guard against MPLANE devices, and replace it with the check of\n>> the number of planes in the format the simple pipeline handler is going to\n>> use for capture. This will let MPLANE devices which don't use non-contiguous\n>> memory for frame buffers to work with the simple pipeline handler.\n> \n> I wonder if the check should not be moved to SimpleCameraData::init() \n> where the formats_ array is built. The array contains all supported \n> formats of the camera and excluding mplaner formats from it will make it \n> not show up at all for applications. Also validate() would Adjust if any \n> format is asked for that is not in the formats_ array.\n\n\nThat sounds pretty good too. If we go that route, I think it will need a\nhighlighting '\\todo: support mplane formats' so that it's clear/easy to\nfind the code which is mysteriously removing supported formats from a\ndevice which is capable of using them ;-) (after we really support\nMultiplanar).\n\n--\nKieran\n\n\n\n> \n>>\n>> Signed-off-by: Andrey Konovalov \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n>> 1 file changed, 6 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 10223a9b..8dc23623 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>> \tif (ret)\n>> \t\treturn ret;\n>> \n>> +\tif (captureFormat.planesCount != 1) {\n>> +\t\tLOG(SimplePipeline, Error)\n>> +\t\t\t<< \"Planar formats using non-contiguous memory not supported\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> \tif (captureFormat.fourcc != videoFormat ||\n>> \t captureFormat.size != pipeConfig.captureSize) {\n>> \t\tLOG(SimplePipeline, Error)\n>> @@ -845,12 +851,6 @@ V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n>> \tif (video->open() < 0)\n>> \t\treturn nullptr;\n>> \n>> -\tif (video->caps().isMultiplanar()) {\n>> -\t\tLOG(SimplePipeline, Error)\n>> -\t\t\t<< \"V4L2 multiplanar devices are not supported\";\n>> -\t\treturn nullptr;\n>> -\t}\n>> -\n>> \tvideo->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n>> \n>> \tauto element = videos_.emplace(entity, std::move(video));\n>> -- \n>> 2.17.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\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 B2A6CBEEDF\n\tfor ;\n\tWed, 7 Oct 2020 13:04:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4AD7660579;\n\tWed, 7 Oct 2020 15:04:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FB386055E\n\tfor ;\n\tWed, 7 Oct 2020 15:04:15 +0200 (CEST)","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 7FF809DA;\n\tWed, 7 Oct 2020 15:04:14 +0200 (CEST)"],"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=\"Wgd/HJIq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602075854;\n\tbh=n1ZFWYgR+5aQ7qImp7AWEAUUqK/jET7ukHjWZ7/CAV4=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Wgd/HJIqI6rfIc+qO7ED7NNFW5ZwuPYasjBp/qeJ+LXUfHhQO2GcHuMd2F2S3qjeq\n\tb0kgL/28N8Cwyoz4PScGHCLbYwfxaqjj06XfdHSoyCd5UUvFLqkFaIK4wR6xsrbjO1\n\tE6wvQ0ZZgHN1x7dFxXgWqMEqNuVvnmCXUzd/mOQ8=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tAndrey Konovalov ","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>","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":"","Date":"Wed, 7 Oct 2020 14:04:11 +0100","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":"<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13087,"web_url":"https://patchwork.libcamera.org/comment/13087/","msgid":"<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>","date":"2020-10-07T17:16:16","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Niklas, Kieran,\n\nOn 07.10.2020 16:04, Kieran Bingham wrote:\n> Hi Niklas, Andrey,\n> \n> On 07/10/2020 14:01, Niklas Söderlund wrote:\n>> Hello Andrey,\n>>\n>> Thanks for your patch.\n>>\n>> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n>>> The current simple pipeline handler refuses to work with capture devices\n>>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device capabilities\n>>> field. This is too restrictive, as devices supporting the multi-planar API\n>>> can be using contiguous memory for semi-planar and planar formats, and this\n>>> would just work without any changes to libcamera.\n>>>\n>>> Drop the guard against MPLANE devices, and replace it with the check of\n>>> the number of planes in the format the simple pipeline handler is going to\n>>> use for capture. This will let MPLANE devices which don't use non-contiguous\n>>> memory for frame buffers to work with the simple pipeline handler.\n>>\n>> I wonder if the check should not be moved to SimpleCameraData::init()\n>> where the formats_ array is built. The array contains all supported\n>> formats of the camera and excluding mplaner formats from it will make it\n>> not show up at all for applications. Also validate() would Adjust if any\n>> format is asked for that is not in the formats_ array.\n\nYes, this is a better option, thanks! I'll send the v2 shortly.\n\n> That sounds pretty good too. If we go that route, I think it will need a\n> highlighting '\\todo: support mplane formats' so that it's clear/easy to\n> find the code which is mysteriously removing supported formats from a\n> device which is capable of using them ;-) (after we really support\n> Multiplanar).\n\nOK, will add the \\todo.\nBTW, the \"mplane format\" in this context means the one using non-contiguous\nmemory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'\n(aka V4L2_PIX_FMT_NV16M) would be excluded.\n\nThanks,\nAndrey\n\n> --\n> Kieran\n> \n> \n> \n>>\n>>>\n>>> Signed-off-by: Andrey Konovalov \n>>> ---\n>>> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n>>> 1 file changed, 6 insertions(+), 6 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 10223a9b..8dc23623 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>> \tif (ret)\n>>> \t\treturn ret;\n>>> \n>>> +\tif (captureFormat.planesCount != 1) {\n>>> +\t\tLOG(SimplePipeline, Error)\n>>> +\t\t\t<< \"Planar formats using non-contiguous memory not supported\";\n>>> +\t\treturn -EINVAL;\n>>> +\t}\n>>> +\n>>> \tif (captureFormat.fourcc != videoFormat ||\n>>> \t captureFormat.size != pipeConfig.captureSize) {\n>>> \t\tLOG(SimplePipeline, Error)\n>>> @@ -845,12 +851,6 @@ V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)\n>>> \tif (video->open() < 0)\n>>> \t\treturn nullptr;\n>>> \n>>> -\tif (video->caps().isMultiplanar()) {\n>>> -\t\tLOG(SimplePipeline, Error)\n>>> -\t\t\t<< \"V4L2 multiplanar devices are not supported\";\n>>> -\t\treturn nullptr;\n>>> -\t}\n>>> -\n>>> \tvideo->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n>>> \n>>> \tauto element = videos_.emplace(entity, std::move(video));\n>>> -- \n>>> 2.17.1\n>>>\n>>> _______________________________________________\n>>> libcamera-devel mailing list\n>>> libcamera-devel@lists.libcamera.org\n>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>\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 64EA7BEEDF\n\tfor ;\n\tWed, 7 Oct 2020 17:16:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBAF2605C1;\n\tWed, 7 Oct 2020 19:16:20 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4BAC605AF\n\tfor ;\n\tWed, 7 Oct 2020 19:16:18 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id c21so2736656ljn.13\n\tfor ;\n\tWed, 07 Oct 2020 10:16:18 -0700 (PDT)","from [192.168.118.216] (37-144-159-139.broadband.corbina.ru.\n\t[37.144.159.139]) by smtp.gmail.com with ESMTPSA id\n\tf25sm444755ljn.29.2020.10.07.10.16.16\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 07 Oct 2020 10:16:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"QC7bASSq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\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=IhjK4M56Dbp1XW00sqxFJPc099M0ZT8NJTRiolTGsmM=;\n\tb=QC7bASSqtD7iJPYEJ4Zvoxv7eFrLhVbBnrFBXEXGpoGTIop+MIbMHiuyoDGrqRd+22\n\tTYvCiTz+YLbSOziamlIAdRnYjMR20xjFz3lnbUM9ffrI90/LTLLcFe6b57qC99fKCf5Y\n\tXiiFuUyA1iWfihaSc4f+mu4w6lIJ2Wd5EoW2Pcic9caAjkQipXCOpfGhEsupaE2icgoC\n\tCWJpIAZGJ2mJ0pnugdWfNdlcqX1cIA2o/CRfdQw9kkVD44p1Ri+rwZtontzRes/hg9Lq\n\t2pNW1+abmXABxHylO8WLqXjnyov+1tUcXj+w01kGZO9oj7UfE71IFgrmMVkL6+tZwm1j\n\tvhdw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\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=IhjK4M56Dbp1XW00sqxFJPc099M0ZT8NJTRiolTGsmM=;\n\tb=K8qA8/MJnij1tWLxlg1driWR3aFtcw30wvODwCKifsYUdbOSCNIWzbkgZsIr/s0d4x\n\tRm/w20n7eXkU39awgQTlUPnT+OAdT/A/3+qb72/9RhcPqi0602TCxnO8St4Rrjk8wqGL\n\tughznkGnI2xoMG5gXWtlmTmnEqGMEbUoD4tohw5sKiKMlgB+reZGlnR/YD5g0/Veb8Mi\n\tffQMfzxPZLeneZl3COhIR/arY0VQVHFXzrdTofxkYCtXch5WgBJhnGG7gDTGTigLJI7V\n\tsiLacnhC8v6YwS4kp93xBAEtKHJE1Bfh3G617yZQc3jQAtr+nB4ZVCEs+ABNQrP9bwkn\n\t22sQ==","X-Gm-Message-State":"AOAM5311VjV1UlaxFgZkeYknNZv3m/f3iXZa1lH9tL/bqxNHW2z4oJjY\n\tVYyLWYXOEAKvq6xC/Zl6/IKW0CX0EODRxg==","X-Google-Smtp-Source":"ABdhPJxAWKvPkn4q57Bc7/f2RaJGkU70dQ2SqGQFEAPkADh0we6Qv70JnMikBtBaf4r5JNChd3lUtQ==","X-Received":"by 2002:a2e:80d0:: with SMTP id\n\tr16mr1603515ljg.161.1602090977974; \n\tWed, 07 Oct 2020 10:16:17 -0700 (PDT)","To":"kieran.bingham@ideasonboard.com, =?utf-8?q?Niklas_S=C3=B6derlund?=\n\t","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>\n\t","From":"Andrey Konovalov ","Message-ID":"<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>","Date":"Wed, 7 Oct 2020 20:16:16 +0300","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":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13088,"web_url":"https://patchwork.libcamera.org/comment/13088/","msgid":"<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>","date":"2020-10-07T18:22:40","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Andrey,\n\nOn 07/10/2020 18:16, Andrey Konovalov wrote:\n> Hi Niklas, Kieran,\n> \n> On 07.10.2020 16:04, Kieran Bingham wrote:\n>> Hi Niklas, Andrey,\n>>\n>> On 07/10/2020 14:01, Niklas Söderlund wrote:\n>>> Hello Andrey,\n>>>\n>>> Thanks for your patch.\n>>>\n>>> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n>>>> The current simple pipeline handler refuses to work with capture\n>>>> devices\n>>>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device\n>>>> capabilities\n>>>> field. This is too restrictive, as devices supporting the\n>>>> multi-planar API\n>>>> can be using contiguous memory for semi-planar and planar formats,\n>>>> and this\n>>>> would just work without any changes to libcamera.\n>>>>\n>>>> Drop the guard against MPLANE devices, and replace it with the check of\n>>>> the number of planes in the format the simple pipeline handler is\n>>>> going to\n>>>> use for capture. This will let MPLANE devices which don't use\n>>>> non-contiguous\n>>>> memory for frame buffers to work with the simple pipeline handler.\n>>>\n>>> I wonder if the check should not be moved to SimpleCameraData::init()\n>>> where the formats_ array is built. The array contains all supported\n>>> formats of the camera and excluding mplaner formats from it will make it\n>>> not show up at all for applications. Also validate() would Adjust if any\n>>> format is asked for that is not in the formats_ array.\n> \n> Yes, this is a better option, thanks! I'll send the v2 shortly.\n> \n>> That sounds pretty good too. If we go that route, I think it will need a\n>> highlighting '\\todo: support mplane formats' so that it's clear/easy to\n>> find the code which is mysteriously removing supported formats from a\n>> device which is capable of using them ;-) (after we really support\n>> Multiplanar).\n> \n> OK, will add the \\todo.\n> BTW, the \"mplane format\" in this context means the one using non-contiguous\n> memory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'\n> (aka V4L2_PIX_FMT_NV16M) would be excluded.\n\nYes indeed - Agreed ;-)\n--\nKieran\n\n\n> \n> Thanks,\n> Andrey\n> \n>> -- \n>> Kieran\n>>\n>>\n>>\n>>>\n>>>>\n>>>> Signed-off-by: Andrey Konovalov \n>>>> ---\n>>>>   src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n>>>>   1 file changed, 6 insertions(+), 6 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp\n>>>> b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 10223a9b..8dc23623 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera\n>>>> *camera, CameraConfiguration *c)\n>>>>       if (ret)\n>>>>           return ret;\n>>>>   +    if (captureFormat.planesCount != 1) {\n>>>> +        LOG(SimplePipeline, Error)\n>>>> +            <<  \"Planar formats using non-contiguous memory not\n>>>> supported\";\n>>>> +        return -EINVAL;\n>>>> +    }\n>>>> +\n>>>>       if (captureFormat.fourcc != videoFormat ||\n>>>>           captureFormat.size != pipeConfig.captureSize) {\n>>>>           LOG(SimplePipeline, Error)\n>>>> @@ -845,12 +851,6 @@ V4L2VideoDevice\n>>>> *SimplePipelineHandler::video(const MediaEntity *entity)\n>>>>       if (video->open() < 0)\n>>>>           return nullptr;\n>>>>   -    if (video->caps().isMultiplanar()) {\n>>>> -        LOG(SimplePipeline, Error)\n>>>> -            << \"V4L2 multiplanar devices are not supported\";\n>>>> -        return nullptr;\n>>>> -    }\n>>>> -\n>>>>       video->bufferReady.connect(this,\n>>>> &SimplePipelineHandler::bufferReady);\n>>>>         auto element = videos_.emplace(entity, std::move(video));\n>>>> -- \n>>>> 2.17.1\n>>>>\n>>>> _______________________________________________\n>>>> libcamera-devel mailing list\n>>>> libcamera-devel@lists.libcamera.org\n>>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>>\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 D39D5BEEE0\n\tfor ;\n\tWed, 7 Oct 2020 18:22:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40039605B6;\n\tWed, 7 Oct 2020 20:22:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E2686039B\n\tfor ;\n\tWed, 7 Oct 2020 20:22:43 +0200 (CEST)","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 D01BE9DA;\n\tWed, 7 Oct 2020 20:22:42 +0200 (CEST)"],"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=\"LdrPf2mt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602094963;\n\tbh=fh/h7IgIfAf24D8038WlHWw2kyWq5RBoluvCqgUNRQQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=LdrPf2mtuwrCt4EJixJuWOK0Duc2shZbHuLWBgvaD5MwNvwFrilHkm0khSDqGjGa3\n\t3WbK3bnc/ikjbqs4OnpCE+B0vKNbQlz9vJ5FpBDZSyd2UNcHz8spQPjgQ597md14p5\n\tTnSjqYC4crExwBP2w1Sj9fXGR62CSXlQudCLLuWI=","To":"Andrey Konovalov , =?utf-8?q?Niklas_S?=\n\t=?utf-8?b?w7ZkZXJsdW5k?= ","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>\n\t\n\t<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>","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":"<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>","Date":"Wed, 7 Oct 2020 19:22:40 +0100","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":"<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13116,"web_url":"https://patchwork.libcamera.org/comment/13116/","msgid":"","date":"2020-10-08T21:48:14","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi,\n\nAs suggested by Laurent during the discussion on #libcamera irc channel,\nI considered filtering out pixel formats not supported in libcamera in\nSimpleCameraData::init().\n\nThe thing is that in SimpleCameraData::init() we only have the information\nfrom V4L2VideoDevice::formats() (enum_fmt ioctl) which doesn't include\nthe number of memory planes for a given format.\nWhile in SimplePipelineHandler::configure() we have the data returned by\ns_fmt ioctl, so the number of memory planes for the format set is known,\nand I could easily use it in v1 of the patch.\n\nAs libcamera currently supports only single memory plane (contiguous\nmemory) formats, this is enough to filter out unsupported formats\nfrom what V4L2VideoDevice::formats() returns (easy to implement in\nSimpleCameraData::init()). And checking the number of memory planes\nfor a given format isn't necessary.\n\nBut after looking at SimpleCameraData::init(), it turned out that unsupported\nformats are filtered out by the already existing code:\n\n-----8<-----\n /*\n * Store the configuration in the formats_ map, mapping the\n * PixelFormat to the corresponding configuration. Any\n * previously stored value is overwritten, as the pipeline\n * handler currently doesn't care about how a particular\n * PixelFormat is achieved.\n */\n for (const auto &videoFormat : videoFormats) {\n PixelFormat pixelFormat = videoFormat.first.toPixelFormat();\n if (!pixelFormat)\n continue;\n-----8<-----\n\nV4L2PixelFormat::toPixelFormat() method checks if the V4L2 pixel format is present\nin the vpf2pf map, and if it is not there returns the invalid PixelFormat (with zero fourcc_).\n\nAnd indeed with the \"if (video->caps().isMultiplanar())\" check removed, and the NV61 entry\ncommented out from the vpf2pf map (to simulate unsupported format) I've got:\n\n-----8<-----\n[2:29:09.524492795] [1396] DEBUG SimplePipeline simple.cpp:292 Adding configuration for 2592x1944 in pixel formats [ NV61, NV16]\n[2:29:09.524833004] [1396] WARN V4L2 v4l2_pixelformat.cpp:180 Unsupported V4L2 pixel format NV61\n[2:29:09.527873323] [1396] DEBUG Camera camera_manager.cpp:152 Pipeline handler \"SimplePipelineHandler\" matched\nUsing camera /base/soc/cci@1b0c000/i2c-bus@0/camera_rear@3c\n[2:29:09.529992233] [1395] DEBUG Camera camera.cpp:754 streams configuration: (0) 2592x1944-NV16\n[2:29:09.530364682] [1395] INFO Camera camera.cpp:811 configuring streams: (0) 2592x1944-NV16\n-----8<-----\n\nSo the \"unsupported\" NV61 format was filtered out by the existing code, and NV16 was used for the capture instead.\n\nThis leaves us with the two options for v2 of the patch:\n* v2a\n Just drop the \"if (video->caps().isMultiplanar())\" guard.\n Don't add any new checks. As unsupported formats are filtered out in SimpleCameraData::init(),\n and all the currently supported formats will just work with the simple pipeline handler.\n* v2b\n Only change the commit message in the v1 patch.\n To explain that the added check is no op in the current libcamera, but\n this \"if (captureFormat.planesCount != 1)\" will trigger and serve as the reminder to review\n the simple pipeline handler code when 1) libcamera gets support for non-contiguous formats,\n and 2) the hardware which uses such formats is enabled in the simple pipeline handler.\n\nWhat would be the best option?\n\n\nThanks,\nAndrey\n\nOn 07.10.2020 21:22, Kieran Bingham wrote:\n> Hi Andrey,\n> \n> On 07/10/2020 18:16, Andrey Konovalov wrote:\n>> Hi Niklas, Kieran,\n>>\n>> On 07.10.2020 16:04, Kieran Bingham wrote:\n>>> Hi Niklas, Andrey,\n>>>\n>>> On 07/10/2020 14:01, Niklas Söderlund wrote:\n>>>> Hello Andrey,\n>>>>\n>>>> Thanks for your patch.\n>>>>\n>>>> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n>>>>> The current simple pipeline handler refuses to work with capture\n>>>>> devices\n>>>>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device\n>>>>> capabilities\n>>>>> field. This is too restrictive, as devices supporting the\n>>>>> multi-planar API\n>>>>> can be using contiguous memory for semi-planar and planar formats,\n>>>>> and this\n>>>>> would just work without any changes to libcamera.\n>>>>>\n>>>>> Drop the guard against MPLANE devices, and replace it with the check of\n>>>>> the number of planes in the format the simple pipeline handler is\n>>>>> going to\n>>>>> use for capture. This will let MPLANE devices which don't use\n>>>>> non-contiguous\n>>>>> memory for frame buffers to work with the simple pipeline handler.\n>>>>\n>>>> I wonder if the check should not be moved to SimpleCameraData::init()\n>>>> where the formats_ array is built. The array contains all supported\n>>>> formats of the camera and excluding mplaner formats from it will make it\n>>>> not show up at all for applications. Also validate() would Adjust if any\n>>>> format is asked for that is not in the formats_ array.\n>>\n>> Yes, this is a better option, thanks! I'll send the v2 shortly.\n>>\n>>> That sounds pretty good too. If we go that route, I think it will need a\n>>> highlighting '\\todo: support mplane formats' so that it's clear/easy to\n>>> find the code which is mysteriously removing supported formats from a\n>>> device which is capable of using them ;-) (after we really support\n>>> Multiplanar).\n>>\n>> OK, will add the \\todo.\n>> BTW, the \"mplane format\" in this context means the one using non-contiguous\n>> memory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'\n>> (aka V4L2_PIX_FMT_NV16M) would be excluded.\n> \n> Yes indeed - Agreed ;-)\n> --\n> Kieran\n> \n> \n>>\n>> Thanks,\n>> Andrey\n>>\n>>> -- \n>>> Kieran\n>>>\n>>>\n>>>\n>>>>\n>>>>>\n>>>>> Signed-off-by: Andrey Konovalov \n>>>>> ---\n>>>>>   src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n>>>>>   1 file changed, 6 insertions(+), 6 deletions(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp\n>>>>> b/src/libcamera/pipeline/simple/simple.cpp\n>>>>> index 10223a9b..8dc23623 100644\n>>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera\n>>>>> *camera, CameraConfiguration *c)\n>>>>>       if (ret)\n>>>>>           return ret;\n>>>>>   +    if (captureFormat.planesCount != 1) {\n>>>>> +        LOG(SimplePipeline, Error)\n>>>>> +            <<  \"Planar formats using non-contiguous memory not\n>>>>> supported\";\n>>>>> +        return -EINVAL;\n>>>>> +    }\n>>>>> +\n>>>>>       if (captureFormat.fourcc != videoFormat ||\n>>>>>           captureFormat.size != pipeConfig.captureSize) {\n>>>>>           LOG(SimplePipeline, Error)\n>>>>> @@ -845,12 +851,6 @@ V4L2VideoDevice\n>>>>> *SimplePipelineHandler::video(const MediaEntity *entity)\n>>>>>       if (video->open() < 0)\n>>>>>           return nullptr;\n>>>>>   -    if (video->caps().isMultiplanar()) {\n>>>>> -        LOG(SimplePipeline, Error)\n>>>>> -            << \"V4L2 multiplanar devices are not supported\";\n>>>>> -        return nullptr;\n>>>>> -    }\n>>>>> -\n>>>>>       video->bufferReady.connect(this,\n>>>>> &SimplePipelineHandler::bufferReady);\n>>>>>         auto element = videos_.emplace(entity, std::move(video));\n>>>>> -- \n>>>>> 2.17.1\n>>>>>\n>>>>> _______________________________________________\n>>>>> libcamera-devel mailing list\n>>>>> libcamera-devel@lists.libcamera.org\n>>>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>>>\n>>>\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 9102DBEEE0\n\tfor ;\n\tThu, 8 Oct 2020 21:48:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C9E4606FD;\n\tThu, 8 Oct 2020 23:48:19 +0200 (CEST)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7F5060361\n\tfor ;\n\tThu, 8 Oct 2020 23:48:17 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id h6so8380915lfj.3\n\tfor ;\n\tThu, 08 Oct 2020 14:48:17 -0700 (PDT)","from [192.168.118.216] (37-144-159-139.broadband.corbina.ru.\n\t[37.144.159.139]) by smtp.gmail.com with ESMTPSA id\n\tl3sm140016lfp.219.2020.10.08.14.48.15\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 08 Oct 2020 14:48:15 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"VlZiZwu2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\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=RSK/XhyQm9w4jotKTF47PHAZzlZI/OxNC1uCHnnYplk=;\n\tb=VlZiZwu2nPrt6IsBvyCHzK7+WafEnjQNt2dlHfI/MXqQh/VVXhGPCP/RDdwCMlU7Pd\n\tgZAnA3/z6HEPsXzkssgtz5J2SHJt9CY9NL3Fm7aqVjiMrDcHeEhCx2EHrt8qWfikCHlZ\n\tKgPy7CBgEe3Ks1kgjnhzO7T499ecduaLYxIS+o+mWwql4SHG6j8zaU/1+zhlEZJGxLt1\n\tt2MWRX+1VEvze3kMYRw+xS1QYAO2TiUYK6Z9prFUzAxf9Nptth02d3FrAom2zX8L0DJk\n\tPZR3eFFxbCCRH32yGVbAjuycWq+cgmMD/3ThqV5PjywGny+aX4nG4Eo+GPCbYl0td6jR\n\teIKw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\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=RSK/XhyQm9w4jotKTF47PHAZzlZI/OxNC1uCHnnYplk=;\n\tb=m9KU6V8w2kjvhbWCx3YxSDFIO/QKXP42BsaqXM8iYWUCw2T8DY+eh45tsVEJsLARRd\n\tECiP/2Mo97f2lEsh0XZ7F/9kJJWnN8Redg5V+IsWQQBZuv8Ex/ERSyJ1namQpxomGLjV\n\tJ2pekFtY30WU5dNo0PN7zDL/lfwzT1pAmvZNIuzoz8X5jUzEDNxopye/2vOA3ePUb6Hm\n\t5czuCg6DYJxLbnnKw6Ejp5JYk4myjlf4lGm7jmPaHE3oSdSA7bJ9A0j/jrL5VwPHnjlb\n\tTVxc3WP/XqQbTrEC3uwZbdBUZ0y6DYx4eEeTmIoj9dl0aoYjAVOmwviU3hwx56rD11H0\n\tQbkg==","X-Gm-Message-State":"AOAM533/+hSpDCVkln3wfbOrPVJPgDFp+iKQT9myuNpZBbjnbjKZM0WN\n\t2YnggUGG5PxmkMAp4u5pPuzQhw==","X-Google-Smtp-Source":"ABdhPJwjdGAOKgY51382X05uh9Ll0rO7sqmSOks+VnkHTrCcgwspjDNP3H5fB/KaWPgQS9Gv6wRVvA==","X-Received":"by 2002:a19:cb55:: with SMTP id\n\tb82mr3581944lfg.212.1602193696853; \n\tThu, 08 Oct 2020 14:48:16 -0700 (PDT)","To":"kieran.bingham@ideasonboard.com, =?utf-8?q?Niklas_S=C3=B6derlund?=\n\t","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>\n\t\n\t<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>\n\t<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>","From":"Andrey Konovalov ","Message-ID":"","Date":"Fri, 9 Oct 2020 00:48:14 +0300","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":"<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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-Transfer-Encoding":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13117,"web_url":"https://patchwork.libcamera.org/comment/13117/","msgid":"<20201009000541.GK3939@pendragon.ideasonboard.com>","date":"2020-10-09T00:05:41","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Andrey,\n\nOn Fri, Oct 09, 2020 at 12:48:14AM +0300, Andrey Konovalov wrote:\n> Hi,\n> \n> As suggested by Laurent during the discussion on #libcamera irc channel,\n> I considered filtering out pixel formats not supported in libcamera in\n> SimpleCameraData::init().\n> \n> The thing is that in SimpleCameraData::init() we only have the information\n> from V4L2VideoDevice::formats() (enum_fmt ioctl) which doesn't include\n> the number of memory planes for a given format.\n> While in SimplePipelineHandler::configure() we have the data returned by\n> s_fmt ioctl, so the number of memory planes for the format set is known,\n> and I could easily use it in v1 of the patch.\n> \n> As libcamera currently supports only single memory plane (contiguous\n> memory) formats, this is enough to filter out unsupported formats\n> from what V4L2VideoDevice::formats() returns (easy to implement in\n> SimpleCameraData::init()). And checking the number of memory planes\n> for a given format isn't necessary.\n> \n> But after looking at SimpleCameraData::init(), it turned out that unsupported\n> formats are filtered out by the already existing code:\n> \n> -----8<-----\n> /*\n> * Store the configuration in the formats_ map, mapping the\n> * PixelFormat to the corresponding configuration. Any\n> * previously stored value is overwritten, as the pipeline\n> * handler currently doesn't care about how a particular\n> * PixelFormat is achieved.\n> */\n> for (const auto &videoFormat : videoFormats) {\n> PixelFormat pixelFormat = videoFormat.first.toPixelFormat();\n> if (!pixelFormat)\n> continue;\n> -----8<-----\n> \n> V4L2PixelFormat::toPixelFormat() method checks if the V4L2 pixel format is present\n> in the vpf2pf map, and if it is not there returns the invalid PixelFormat (with zero fourcc_).\n> \n> And indeed with the \"if (video->caps().isMultiplanar())\" check removed, and the NV61 entry\n> commented out from the vpf2pf map (to simulate unsupported format) I've got:\n> \n> -----8<-----\n> [2:29:09.524492795] [1396] DEBUG SimplePipeline simple.cpp:292 Adding configuration for 2592x1944 in pixel formats [ NV61, NV16]\n> [2:29:09.524833004] [1396] WARN V4L2 v4l2_pixelformat.cpp:180 Unsupported V4L2 pixel format NV61\n> [2:29:09.527873323] [1396] DEBUG Camera camera_manager.cpp:152 Pipeline handler \"SimplePipelineHandler\" matched\n> Using camera /base/soc/cci@1b0c000/i2c-bus@0/camera_rear@3c\n> [2:29:09.529992233] [1395] DEBUG Camera camera.cpp:754 streams configuration: (0) 2592x1944-NV16\n> [2:29:09.530364682] [1395] INFO Camera camera.cpp:811 configuring streams: (0) 2592x1944-NV16\n> -----8<-----\n> \n> So the \"unsupported\" NV61 format was filtered out by the existing code, and NV16 was used for the capture instead.\n> \n> This leaves us with the two options for v2 of the patch:\n> * v2a\n> Just drop the \"if (video->caps().isMultiplanar())\" guard.\n> Don't add any new checks. As unsupported formats are filtered out in SimpleCameraData::init(),\n> and all the currently supported formats will just work with the simple pipeline handler.\n> * v2b\n> Only change the commit message in the v1 patch.\n> To explain that the added check is no op in the current libcamera, but\n> this \"if (captureFormat.planesCount != 1)\" will trigger and serve as the reminder to review\n> the simple pipeline handler code when 1) libcamera gets support for non-contiguous formats,\n> and 2) the hardware which uses such formats is enabled in the simple pipeline handler.\n> \n> What would be the best option?\n\nI'd go for the second option, a reminder is useful, but I don't have a\nvery strong preference.\n\n> On 07.10.2020 21:22, Kieran Bingham wrote:\n> > On 07/10/2020 18:16, Andrey Konovalov wrote:\n> >> On 07.10.2020 16:04, Kieran Bingham wrote:\n> >>> On 07/10/2020 14:01, Niklas Söderlund wrote:\n> >>>> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n> >>>>> The current simple pipeline handler refuses to work with capture\n> >>>>> devices\n> >>>>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device\n> >>>>> capabilities\n> >>>>> field. This is too restrictive, as devices supporting the\n> >>>>> multi-planar API\n> >>>>> can be using contiguous memory for semi-planar and planar formats,\n> >>>>> and this\n> >>>>> would just work without any changes to libcamera.\n> >>>>>\n> >>>>> Drop the guard against MPLANE devices, and replace it with the check of\n> >>>>> the number of planes in the format the simple pipeline handler is\n> >>>>> going to\n> >>>>> use for capture. This will let MPLANE devices which don't use\n> >>>>> non-contiguous\n> >>>>> memory for frame buffers to work with the simple pipeline handler.\n> >>>>\n> >>>> I wonder if the check should not be moved to SimpleCameraData::init()\n> >>>> where the formats_ array is built. The array contains all supported\n> >>>> formats of the camera and excluding mplaner formats from it will make it\n> >>>> not show up at all for applications. Also validate() would Adjust if any\n> >>>> format is asked for that is not in the formats_ array.\n> >>\n> >> Yes, this is a better option, thanks! I'll send the v2 shortly.\n> >>\n> >>> That sounds pretty good too. If we go that route, I think it will need a\n> >>> highlighting '\\todo: support mplane formats' so that it's clear/easy to\n> >>> find the code which is mysteriously removing supported formats from a\n> >>> device which is capable of using them ;-) (after we really support\n> >>> Multiplanar).\n> >>\n> >> OK, will add the \\todo.\n> >> BTW, the \"mplane format\" in this context means the one using non-contiguous\n> >> memory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'\n> >> (aka V4L2_PIX_FMT_NV16M) would be excluded.\n> > \n> > Yes indeed - Agreed ;-)\n> > \n> >>>>> Signed-off-by: Andrey Konovalov \n> >>>>> ---\n> >>>>>   src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n> >>>>>   1 file changed, 6 insertions(+), 6 deletions(-)\n> >>>>>\n> >>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp\n> >>>>> b/src/libcamera/pipeline/simple/simple.cpp\n> >>>>> index 10223a9b..8dc23623 100644\n> >>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >>>>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera\n> >>>>> *camera, CameraConfiguration *c)\n> >>>>>       if (ret)\n> >>>>>           return ret;\n> >>>>> +    if (captureFormat.planesCount != 1) {\n> >>>>> +        LOG(SimplePipeline, Error)\n> >>>>> +            <<  \"Planar formats using non-contiguous memory not supported\";\n> >>>>> +        return -EINVAL;\n> >>>>> +    }\n> >>>>> +\n> >>>>>       if (captureFormat.fourcc != videoFormat ||\n> >>>>>           captureFormat.size != pipeConfig.captureSize) {\n> >>>>>           LOG(SimplePipeline, Error)\n> >>>>> @@ -845,12 +851,6 @@ V4L2VideoDevice\n> >>>>> *SimplePipelineHandler::video(const MediaEntity *entity)\n> >>>>>       if (video->open() < 0)\n> >>>>>           return nullptr;\n> >>>>> -    if (video->caps().isMultiplanar()) {\n> >>>>> -        LOG(SimplePipeline, Error)\n> >>>>> -            << \"V4L2 multiplanar devices are not supported\";\n> >>>>> -        return nullptr;\n> >>>>> -    }\n> >>>>> -\n> >>>>>       video->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n> >>>>>         auto element = videos_.emplace(entity, std::move(video));","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 92467BEEDF\n\tfor ;\n\tFri, 9 Oct 2020 00:06:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1865460710;\n\tFri, 9 Oct 2020 02:06:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 589B7605BF\n\tfor ;\n\tFri, 9 Oct 2020 02:06:25 +0200 (CEST)","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 B68EB59E;\n\tFri, 9 Oct 2020 02:06:24 +0200 (CEST)"],"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=\"VfOS4rZy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602201984;\n\tbh=pstkB0MQCLPo2m2F+qfNfVu27ZJoO4/lTmKG521oJrk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VfOS4rZysu4VAPf2tCsUK0fjHd47W+TPvv7Da+SiFg0Xj/7YDk6F8BDGklKK0qBgf\n\t8fp9Gb6FvS97+sz3Wz0bTemaShIaqMgoyHtJb+fT5z0vgwdUe0JtncoP4GynVdGcuz\n\tpsV8ETg7LpMFagfIRtjFNU5gEA+We1lXUKrGfjRw=","Date":"Fri, 9 Oct 2020 03:05:41 +0300","From":"Laurent Pinchart ","To":"Andrey Konovalov ","Message-ID":"<20201009000541.GK3939@pendragon.ideasonboard.com>","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>\n\t\n\t<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>\n\t<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13118,"web_url":"https://patchwork.libcamera.org/comment/13118/","msgid":"<20201009001737.axeqc7bhrhlz6vt5@oden.dyn.berto.se>","date":"2020-10-09T00:17:37","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Andrey,\n\nThanks for researching this and proposing a way forward.\n\nOn 2020-10-09 03:05:41 +0300, Laurent Pinchart wrote:\n> Hi Andrey,\n> \n> On Fri, Oct 09, 2020 at 12:48:14AM +0300, Andrey Konovalov wrote:\n> > Hi,\n> > \n> > As suggested by Laurent during the discussion on #libcamera irc channel,\n> > I considered filtering out pixel formats not supported in libcamera in\n> > SimpleCameraData::init().\n> > \n> > The thing is that in SimpleCameraData::init() we only have the information\n> > from V4L2VideoDevice::formats() (enum_fmt ioctl) which doesn't include\n> > the number of memory planes for a given format.\n> > While in SimplePipelineHandler::configure() we have the data returned by\n> > s_fmt ioctl, so the number of memory planes for the format set is known,\n> > and I could easily use it in v1 of the patch.\n> > \n> > As libcamera currently supports only single memory plane (contiguous\n> > memory) formats, this is enough to filter out unsupported formats\n> > from what V4L2VideoDevice::formats() returns (easy to implement in\n> > SimpleCameraData::init()). And checking the number of memory planes\n> > for a given format isn't necessary.\n> > \n> > But after looking at SimpleCameraData::init(), it turned out that unsupported\n> > formats are filtered out by the already existing code:\n> > \n> > -----8<-----\n> > /*\n> > * Store the configuration in the formats_ map, mapping the\n> > * PixelFormat to the corresponding configuration. Any\n> > * previously stored value is overwritten, as the pipeline\n> > * handler currently doesn't care about how a particular\n> > * PixelFormat is achieved.\n> > */\n> > for (const auto &videoFormat : videoFormats) {\n> > PixelFormat pixelFormat = videoFormat.first.toPixelFormat();\n> > if (!pixelFormat)\n> > continue;\n> > -----8<-----\n> > \n> > V4L2PixelFormat::toPixelFormat() method checks if the V4L2 pixel format is present\n> > in the vpf2pf map, and if it is not there returns the invalid PixelFormat (with zero fourcc_).\n> > \n> > And indeed with the \"if (video->caps().isMultiplanar())\" check removed, and the NV61 entry\n> > commented out from the vpf2pf map (to simulate unsupported format) I've got:\n> > \n> > -----8<-----\n> > [2:29:09.524492795] [1396] DEBUG SimplePipeline simple.cpp:292 Adding configuration for 2592x1944 in pixel formats [ NV61, NV16]\n> > [2:29:09.524833004] [1396] WARN V4L2 v4l2_pixelformat.cpp:180 Unsupported V4L2 pixel format NV61\n> > [2:29:09.527873323] [1396] DEBUG Camera camera_manager.cpp:152 Pipeline handler \"SimplePipelineHandler\" matched\n> > Using camera /base/soc/cci@1b0c000/i2c-bus@0/camera_rear@3c\n> > [2:29:09.529992233] [1395] DEBUG Camera camera.cpp:754 streams configuration: (0) 2592x1944-NV16\n> > [2:29:09.530364682] [1395] INFO Camera camera.cpp:811 configuring streams: (0) 2592x1944-NV16\n> > -----8<-----\n> > \n> > So the \"unsupported\" NV61 format was filtered out by the existing code, and NV16 was used for the capture instead.\n> > \n> > This leaves us with the two options for v2 of the patch:\n> > * v2a\n> > Just drop the \"if (video->caps().isMultiplanar())\" guard.\n> > Don't add any new checks. As unsupported formats are filtered out in SimpleCameraData::init(),\n> > and all the currently supported formats will just work with the simple pipeline handler.\n> > * v2b\n> > Only change the commit message in the v1 patch.\n> > To explain that the added check is no op in the current libcamera, but\n> > this \"if (captureFormat.planesCount != 1)\" will trigger and serve as the reminder to review\n> > the simple pipeline handler code when 1) libcamera gets support for non-contiguous formats,\n> > and 2) the hardware which uses such formats is enabled in the simple pipeline handler.\n> > \n> > What would be the best option?\n> \n> I'd go for the second option, a reminder is useful, but I don't have a\n> very strong preference.\n\nI also have no strong preference and reminders are good I'm also OK with \nthe second option.\n\n> \n> > On 07.10.2020 21:22, Kieran Bingham wrote:\n> > > On 07/10/2020 18:16, Andrey Konovalov wrote:\n> > >> On 07.10.2020 16:04, Kieran Bingham wrote:\n> > >>> On 07/10/2020 14:01, Niklas Söderlund wrote:\n> > >>>> On 2020-10-07 10:54:57 +0300, Andrey Konovalov wrote:\n> > >>>>> The current simple pipeline handler refuses to work with capture\n> > >>>>> devices\n> > >>>>> which have V4L2_CAP_VIDEO_CAPTURE_MPLANE flag set in the device\n> > >>>>> capabilities\n> > >>>>> field. This is too restrictive, as devices supporting the\n> > >>>>> multi-planar API\n> > >>>>> can be using contiguous memory for semi-planar and planar formats,\n> > >>>>> and this\n> > >>>>> would just work without any changes to libcamera.\n> > >>>>>\n> > >>>>> Drop the guard against MPLANE devices, and replace it with the check of\n> > >>>>> the number of planes in the format the simple pipeline handler is\n> > >>>>> going to\n> > >>>>> use for capture. This will let MPLANE devices which don't use\n> > >>>>> non-contiguous\n> > >>>>> memory for frame buffers to work with the simple pipeline handler.\n> > >>>>\n> > >>>> I wonder if the check should not be moved to SimpleCameraData::init()\n> > >>>> where the formats_ array is built. The array contains all supported\n> > >>>> formats of the camera and excluding mplaner formats from it will make it\n> > >>>> not show up at all for applications. Also validate() would Adjust if any\n> > >>>> format is asked for that is not in the formats_ array.\n> > >>\n> > >> Yes, this is a better option, thanks! I'll send the v2 shortly.\n> > >>\n> > >>> That sounds pretty good too. If we go that route, I think it will need a\n> > >>> highlighting '\\todo: support mplane formats' so that it's clear/easy to\n> > >>> find the code which is mysteriously removing supported formats from a\n> > >>> device which is capable of using them ;-) (after we really support\n> > >>> Multiplanar).\n> > >>\n> > >> OK, will add the \\todo.\n> > >> BTW, the \"mplane format\" in this context means the one using non-contiguous\n> > >> memory for frame buffers. E.g. 'NV16' I've tested is allowed, while 'NM16'\n> > >> (aka V4L2_PIX_FMT_NV16M) would be excluded.\n> > > \n> > > Yes indeed - Agreed ;-)\n> > > \n> > >>>>> Signed-off-by: Andrey Konovalov \n> > >>>>> ---\n> > >>>>>   src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n> > >>>>>   1 file changed, 6 insertions(+), 6 deletions(-)\n> > >>>>>\n> > >>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp\n> > >>>>> b/src/libcamera/pipeline/simple/simple.cpp\n> > >>>>> index 10223a9b..8dc23623 100644\n> > >>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n> > >>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > >>>>> @@ -592,6 +592,12 @@ int SimplePipelineHandler::configure(Camera\n> > >>>>> *camera, CameraConfiguration *c)\n> > >>>>>       if (ret)\n> > >>>>>           return ret;\n> > >>>>> +    if (captureFormat.planesCount != 1) {\n> > >>>>> +        LOG(SimplePipeline, Error)\n> > >>>>> +            <<  \"Planar formats using non-contiguous memory not supported\";\n> > >>>>> +        return -EINVAL;\n> > >>>>> +    }\n> > >>>>> +\n> > >>>>>       if (captureFormat.fourcc != videoFormat ||\n> > >>>>>           captureFormat.size != pipeConfig.captureSize) {\n> > >>>>>           LOG(SimplePipeline, Error)\n> > >>>>> @@ -845,12 +851,6 @@ V4L2VideoDevice\n> > >>>>> *SimplePipelineHandler::video(const MediaEntity *entity)\n> > >>>>>       if (video->open() < 0)\n> > >>>>>           return nullptr;\n> > >>>>> -    if (video->caps().isMultiplanar()) {\n> > >>>>> -        LOG(SimplePipeline, Error)\n> > >>>>> -            << \"V4L2 multiplanar devices are not supported\";\n> > >>>>> -        return nullptr;\n> > >>>>> -    }\n> > >>>>> -\n> > >>>>>       video->bufferReady.connect(this, &SimplePipelineHandler::bufferReady);\n> > >>>>>         auto element = videos_.emplace(entity, std::move(video));\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 02CFFBEEE0\n\tfor ;\n\tFri, 9 Oct 2020 00:17:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81EA360725;\n\tFri, 9 Oct 2020 02:17:42 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 32866605BF\n\tfor ;\n\tFri, 9 Oct 2020 02:17:40 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id 133so7819692ljj.0\n\tfor ;\n\tThu, 08 Oct 2020 17:17:40 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tx17sm923946lfc.33.2020.10.08.17.17.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 08 Oct 2020 17:17:38 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"vSeWA2ot\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=uSo7o4XReMeGxSm7ovVZawsfeEBvZtTa+B4aFussTvw=;\n\tb=vSeWA2otGUI4jXBQA180HPNu/hbZ+7lqsi/dhcg9YJsJOXlrRdKitc/i+O33/BZuM+\n\tL8W3LdXt6hzh443++iWT7WlK6lwQIrbNUCLJD8vzfQtx0OqMVfhRxgBlx6cBtnOS+3bz\n\tviWMBBw9xI6PT50VidgAQCKh9yScZWxuNHFBpx3JUI3vjZFxmaygu6Ozm8tDTSE2bt9i\n\tflduXQkGfwfU+ILmfRDWED6BUoHEGE0lXKCewuDXcBQZyh1b1cBJexe9Gt0pIIpp5jhp\n\tLP0cr9J6A+RXDl22ofbrJ3yXRxoZ6rU8RB4VgiIF5KCqsIhukCE2zWk0OBS9021ukHRT\n\tCGBg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=uSo7o4XReMeGxSm7ovVZawsfeEBvZtTa+B4aFussTvw=;\n\tb=cLAxP7Q6mNNb8nu3pFIFdhLHRxY63pEuRy+U3T8ZlJ1k+iSEUfYl5FwChkoMyBlTIs\n\tw98q7CpXGQgPej2FAaqWFnu34Ee1B0b3tQIWHB5lMOl6PM80J+T6vTx9lSs40tYKTj2J\n\tJ12Zk3am2yuv6qonUGxbcI23/3BZhzGrNIKQFosk/EgP7E+nSfN3SeagjBKe50jC5RdZ\n\trLWogOexK5yIeOsc/MXTC2Y6j4ekgmY5DXJLAUATnoo65SHssU0fY8wRfimY9TEwVb8y\n\tsDttfi/G2oThJMPEEEqSc3txL7M05HDYbYuEchuVhobRt18CCbFmorhStl/x1vd9ESxY\n\tQOjg==","X-Gm-Message-State":"AOAM533YqlLQ0F95sCla6o0mbW4T07WF4uCqX1oahi01+WulzNFhVbR9\n\tjWckjPVzarv9vO8c/SF19+fHVkartF6DHw==","X-Google-Smtp-Source":"ABdhPJymBudCWqLfp7Bvkvcbkp/gsMpvLot5NmFUZ6PTol0Ew/e81/+q+WJKVOWJg1kumMCQoABSrQ==","X-Received":"by 2002:a2e:9854:: with SMTP id\n\te20mr4329951ljj.152.1602202659295; \n\tThu, 08 Oct 2020 17:17:39 -0700 (PDT)","Date":"Fri, 9 Oct 2020 02:17:37 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20201009001737.axeqc7bhrhlz6vt5@oden.dyn.berto.se>","References":"<20201007075457.4455-1-andrey.konovalov@linaro.org>\n\t<20201007130112.qay3e7izrxpnw5kg@oden.dyn.berto.se>\n\t\n\t<802dffa5-a3f2-6ff0-55e9-1d70eb5f2615@linaro.org>\n\t<882ca631-4619-5eb1-f175-1b28445bd1ba@ideasonboard.com>\n\t\n\t<20201009000541.GK3939@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201009000541.GK3939@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: simple: enable\n\tmplane devices using contiguous memory","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]