[{"id":23291,"web_url":"https://patchwork.libcamera.org/comment/23291/","msgid":"","date":"2022-06-02T08:38:08","subject":"Re: [libcamera-devel] [PATCH v2 4/5] android: camera_device: Use\n\tYUV post-processor","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul and Hiro,\n\nThank you for the patch.\n\nOn Fri, May 27, 2022 at 06:34:39PM +0900, Paul Elder via libcamera-devel wrote:\n> From: Hirokazu Honda \n> \n> When creating the list of StreamConfiguration to be requested to the camera,\n> map NV12 streams of equal size and format together, so that they will be\n> generated by using the YUV post-processor.\n> \n> Signed-off-by: Hirokazu Honda \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++---\n> 1 file changed, 29 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 95c14f60..9ee34b93 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -605,14 +605,40 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\tcontinue;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * While gralloc usage flags are supposed to report usage\n> +\t\t * patterns to select a suitable buffer allocation strategy, in\n> +\t\t * practice they're also used to make other decisions, such as\n> +\t\t * selecting the actual format for the IMPLEMENTATION_DEFINED\n> +\t\t * HAL pixel format. To avoid issues, we thus have to set the\n> +\t\t * GRALLOC_USAGE_HW_CAMERA_WRITE flag unconditionally, even for\n> +\t\t * streams that will be produced in software.\n> +\t\t */\n> +\t\tstream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;\n> +\n> +\t\t/*\n> +\t\t * If a CameraStream with the same size and format of the\n\ns/of the/as the/\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t * current stream has already been requested, associate the two.\n> +\t\t */\n> +\t\tauto iter = std::find_if(\n> +\t\t\tstreamConfigs.begin(), streamConfigs.end(),\n> +\t\t\t[&size, &format](const Camera3StreamConfig &streamConfig) {\n> +\t\t\t\treturn streamConfig.config.size == size &&\n> +\t\t\t\t streamConfig.config.pixelFormat == format;\n> +\t\t\t});\n> +\t\tif (iter != streamConfigs.end()) {\n> +\t\t\t/* Add usage to copy the buffer in streams[0] to stream. */\n> +\t\t\titer->streams[0].stream->usage |= GRALLOC_USAGE_SW_READ_OFTEN;\n> +\t\t\tstream->usage |= GRALLOC_USAGE_SW_WRITE_OFTEN;\n> +\t\t\titer->streams.push_back({ stream, CameraStream::Type::Mapped });\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tCamera3StreamConfig streamConfig;\n> \t\tstreamConfig.streams = { { stream, CameraStream::Type::Direct } };\n> \t\tstreamConfig.config.size = size;\n> \t\tstreamConfig.config.pixelFormat = format;\n> \t\tstreamConfigs.push_back(std::move(streamConfig));\n> -\n> -\t\t/* This stream will be produced by hardware. */\n> -\t\tstream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;\n> \t}\n> \n> \t/* Now handle the MJPEG streams, adding a new stream if required. */","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 9BD84BD161\n\tfor ;\n\tThu, 2 Jun 2022 08:38:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B05E665641;\n\tThu, 2 Jun 2022 10:38:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A81165636\n\tfor ;\n\tThu, 2 Jun 2022 10:38:13 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lmontsouris-659-1-41-236.w92-154.abo.wanadoo.fr [92.154.76.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BE8FC6BD;\n\tThu, 2 Jun 2022 10:38:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654159094;\n\tbh=v4PpjH1rRF9teE0HMvI/Aaowmmu0f/pO74XMuHIIC60=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pp+449KYB78lSpLj6KQc3rglpwxiuxQMzud4+pg6f4tHvyXoYy9CtgUQjQaU4HAqj\n\tpzr+ykCp8GdA0m3syjn0CaJrB+3IBQCP1E3tywfKYaT5T7rtuncMAjryrRi3r0e0LR\n\tl5YNTbZ4JpgMXdYOusIzge5/k6VZKdSTf6DmsKSCgfnmAXQjVd0HBq5/YwjRTLXUCL\n\tStVKruE3UWhwfDyxZ6KISJCngWi5uzSCOGoFGCOv7FDaonitvybAfTZHGCLYk6dLRM\n\tsU29RAKtaIhjz7VgMLW8LUYhfibT+ds1aGt3GMFFp3oXvMfHN88ouR6VhiFqqwdWdE\n\t8o+h22nteDNUA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654159092;\n\tbh=v4PpjH1rRF9teE0HMvI/Aaowmmu0f/pO74XMuHIIC60=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SNalnnFt6B0d2GgsiH9j+OeiV7h5wkRrdaSIfhelxY4AvXCjSvVndqHgijiOAGpYt\n\t2tDTYJ5aRg2Qysg0Mk31JiTxljPvcbmtawGNGXDJbjgdO3ryu8VPB7nuiJqJDrPcNZ\n\te9/zKkipL1zc3dxr5SrXTHxJYvZthPKZa5f4yz6I="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"SNalnnFt\"; dkim-atps=neutral","Date":"Thu, 2 Jun 2022 11:38:08 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-5-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220527093440.953377-5-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] android: camera_device: Use\n\tYUV post-processor","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23294,"web_url":"https://patchwork.libcamera.org/comment/23294/","msgid":"<282dc40e-a16a-bb27-2577-c395085089e9@ideasonboard.com>","date":"2022-06-02T08:48:32","subject":"Re: [libcamera-devel] [PATCH v2 4/5] android: camera_device: Use\n\tYUV post-processor","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi all,\n\nThank you for the patch.\n\nThe patch looks good in what its supposed to do, but just that I've \ntrouble understanding the GRALLOC flags..\n\nOn 5/27/22 11:34, Paul Elder via libcamera-devel wrote:\n> From: Hirokazu Honda \n>\n> When creating the list of StreamConfiguration to be requested to the camera,\n> map NV12 streams of equal size and format together, so that they will be\n> generated by using the YUV post-processor.\n\n\nMight add one line to add a summary/update of usage of GRALLOC flags too.\nBut it's there in the comment so consider them optional.\n\n>\n> Signed-off-by: Hirokazu Honda \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++---\n> 1 file changed, 29 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 95c14f60..9ee34b93 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -605,14 +605,40 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \t\t\tcontinue;\n> \t\t}\n> \n> +\t\t/*\n> +\t\t * While gralloc usage flags are supposed to report usage\n> +\t\t * patterns to select a suitable buffer allocation strategy, in\n> +\t\t * practice they're also used to make other decisions, such as\n> +\t\t * selecting the actual format for the IMPLEMENTATION_DEFINED\n> +\t\t * HAL pixel format. To avoid issues, we thus have to set the\n> +\t\t * GRALLOC_USAGE_HW_CAMERA_WRITE flag unconditionally, even for\n> +\t\t * streams that will be produced in software.\n> +\t\t */\n> +\t\tstream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;\n\n\nOk, we set this unconditionally for all camera3_stream_t(s) in the list.\nIt's not really \"setting it\" rather appending to the existing flags\n\n> +\n> +\t\t/*\n> +\t\t * If a CameraStream with the same size and format of the\n> +\t\t * current stream has already been requested, associate the two.\n> +\t\t */\n> +\t\tauto iter = std::find_if(\n> +\t\t\tstreamConfigs.begin(), streamConfigs.end(),\n> +\t\t\t[&size, &format](const Camera3StreamConfig &streamConfig) {\n> +\t\t\t\treturn streamConfig.config.size == size &&\n> +\t\t\t\t streamConfig.config.pixelFormat == format;\n> +\t\t\t});\n> +\t\tif (iter != streamConfigs.end()) {\n> +\t\t\t/* Add usage to copy the buffer in streams[0] to stream. */\n> +\t\t\titer->streams[0].stream->usage |= GRALLOC_USAGE_SW_READ_OFTEN;\n> +\t\t\tstream->usage |= GRALLOC_USAGE_SW_WRITE_OFTEN;\n\n\nAnd then if the similar stream is previously requested, we append \nGRALLOC_USAGE_SW_WRITE_OFTEN as well.\nSo stream->usage will now currently have both \nGRALLOC_USAGE_HW_CAMERA_WRITE and GRALLOC_USAGE_SW_WRITE_OFTEN ?\n\nI am wondering if that's OK / accepted? or is it just one of two. Is \nthere any priority mechanism for flags? There is limited visibility on \nusage of these flags\n\nI am reading map_usage_to_memtrack() in \ninclude/android/hardware/libhardware/include/hardware/gralloc.h\nIt seems it will just return on checking GRALLOC_USAGE_HW_CAMERA_WRITE \nand that's it. So no other flags are taken into consideration if that's set.\n(Also, there might other functions handling the flags  as well in the \nframe work)\n\nSo yes, I am not confident with the flags yet, so as long as they seem \nto work okay (Thank you Jacopo for multiple CTS run testing)\n\nReviewed-by: Umang Jain \n\n> +\t\t\titer->streams.push_back({ stream, CameraStream::Type::Mapped });\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tCamera3StreamConfig streamConfig;\n> \t\tstreamConfig.streams = { { stream, CameraStream::Type::Direct } };\n> \t\tstreamConfig.config.size = size;\n> \t\tstreamConfig.config.pixelFormat = format;\n> \t\tstreamConfigs.push_back(std::move(streamConfig));\n> -\n> -\t\t/* This stream will be produced by hardware. */\n> -\t\tstream->usage |= GRALLOC_USAGE_HW_CAMERA_WRITE;\n> \t}\n> \n> \t/* Now handle the MJPEG streams, adding a new stream if required. */","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 35572BD161\n\tfor ;\n\tThu, 2 Jun 2022 08:48:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E3B886563A;\n\tThu, 2 Jun 2022 10:48:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C4BF65633\n\tfor ;\n\tThu, 2 Jun 2022 10:48:35 +0200 (CEST)","from [192.168.0.71] (static-127-186-62-95.ipcom.comunitel.net\n\t[95.62.186.127])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C1086BD;\n\tThu, 2 Jun 2022 10:48:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654159715;\n\tbh=HBKbs4ETnNhnC2SP7I+cCtgpk80C1/ab9aq3u3GWyFo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=lKRh7+ULc3CizNfzFowXMnVZMt/QKRugrwttCgJstp5vpr5pZNbpL7QAg5rkNaKBY\n\tns3XOd/JDtfoBFXWgr0JrcsDwY8zM0Gb8yslx/nSg20BOGpv4jwHMKf0Bh7/XVEqKY\n\t2YGMWru/CZFqEU+y+rLpYPtrkAHZRBnaxzdSj4OMGNJUmgrb6DEJIeKFDIjJKi9Q9F\n\tLc70MRmvts3Z12BlJzQUPHEdlKdhD9ZcUIZamDwHLSlm/++0O7gWA4wSN1Rk73NA9w\n\tZ1Wuw50LVH4kIF8hbPaC5xWZe4Xw+tlusa6AzuP5W2Yo7khwJeyer4uXHET0n+5ziH\n\tkDsBiMGiyjzrw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654159715;\n\tbh=HBKbs4ETnNhnC2SP7I+cCtgpk80C1/ab9aq3u3GWyFo=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=rKccSbhQRLQoJz8U3t0K/XFH5oGxClkcn5l3awr4v3ehQe2HGvdJegtYD3h1NSAkN\n\tqk82s8kZZ5MB8p6gOZ2r4sHi39ojEgrhtfjVLyy8hjaCgJ4nlllYuK8AnsH/ETTNfH\n\tepp5n3yAMKXAM1P7jlsDePSOAYXV3a1EceEyQZaY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rKccSbhQ\"; dkim-atps=neutral","Message-ID":"<282dc40e-a16a-bb27-2577-c395085089e9@ideasonboard.com>","Date":"Thu, 2 Jun 2022 10:48:32 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220527093440.953377-1-paul.elder@ideasonboard.com>\n\t<20220527093440.953377-5-paul.elder@ideasonboard.com>","In-Reply-To":"<20220527093440.953377-5-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/5] android: camera_device: Use\n\tYUV post-processor","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]