[{"id":18288,"web_url":"https://patchwork.libcamera.org/comment/18288/","msgid":"<YPrFO6hwTUVHqV/I@pendragon.ideasonboard.com>","date":"2021-07-23T13:33:47","subject":"Re: [libcamera-devel] [PATCH 1/3] android: capabilities: Use a\n\tthrow-away config for YUV stream building","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Jul 15, 2021 at 05:32:39PM +0200, Jacopo Mondi wrote:\n> When building the list of supported YUV streams in getYUVResolutions()\n> the CameraConfiguration provided by the caller as first parameters is used.\n> \n> As the CameraConfiguration will be later actually applied to the Camera,\n> avoid any possible overlap of the configuration parameters by using a\n> throw-away CameraConfiguration generated for the Viewfinder stream role\n> in getYUVResolutions().\n> \n> It's also nicer to avoid having two functions with a similar purpose\n> such as getYUVResolutions() and getRawResolutions() with different\n> parameter lists, as the presence of a CameraConfiguration as first\n> parameter might be confusing to the reader.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/android/camera_capabilities.cpp | 10 +++++-----\n>  src/android/camera_capabilities.h   |  3 +--\n>  2 files changed, 6 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 6b5edb66fad2..6543c2bbc6c3 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -138,13 +138,14 @@ int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,\n>  \treturn initializeStaticMetadata();\n>  }\n>  \n> -std::vector<Size> CameraCapabilities::getYUVResolutions(CameraConfiguration *cameraConfig,\n> -\t\t\t\t\t\t\tconst PixelFormat &pixelFormat,\n> +std::vector<Size> CameraCapabilities::getYUVResolutions(const PixelFormat &pixelFormat,\n>  \t\t\t\t\t\t\tconst std::vector<Size> &resolutions)\n>  {\n>  \tstd::vector<Size> supportedResolutions;\n> -\n> +\tstd::unique_ptr<CameraConfiguration> cameraConfig =\n> +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n>  \tStreamConfiguration &cfg = cameraConfig->at(0);\n> +\n>  \tfor (const Size &res : resolutions) {\n>  \t\tcfg.pixelFormat = pixelFormat;\n>  \t\tcfg.size = res;\n> @@ -324,8 +325,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n>  \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n>  \t\t\tresolutions = getRawResolutions(mappedFormat);\n>  \t\telse\n> -\t\t\tresolutions = getYUVResolutions(cameraConfig.get(),\n> -\t\t\t\t\t\t\tmappedFormat,\n> +\t\t\tresolutions = getYUVResolutions(mappedFormat,\n>  \t\t\t\t\t\t\tcameraResolutions);\n>  \n>  \t\tfor (const Size &res : resolutions) {\n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index 4f5be82595d6..e72bf084cd65 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -43,8 +43,7 @@ private:\n>  \t};\n>  \n>  \tstd::vector<libcamera::Size>\n> -\tgetYUVResolutions(libcamera::CameraConfiguration *cameraConfig,\n> -\t\t\t  const libcamera::PixelFormat &pixelFormat,\n> +\tgetYUVResolutions(const libcamera::PixelFormat &pixelFormat,\n>  \t\t\t  const std::vector<libcamera::Size> &resolutions);\n>  \tstd::vector<libcamera::Size>\n>  \tgetRawResolutions(const libcamera::PixelFormat &pixelFormat);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 ED0F9C0109\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 23 Jul 2021 13:33:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEA1C687A9;\n\tFri, 23 Jul 2021 15:33:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CB8B68537\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 23 Jul 2021 15:33:50 +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 AC5C83F2;\n\tFri, 23 Jul 2021 15:33:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nrYtzckr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627047229;\n\tbh=o9g73ljQ7nq5MlbntcdWQLJtsR8wtUyEWSi3hKBvvew=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nrYtzckrNE+q0cb2hurHPyVZaSkSClKfjYiX9AAPLPgA3cWeWSarPTcqlq3cg2VTw\n\tNTvFELKrsLFbaQ52NnXDDhpjFCUte7YBawYqzXNl3fLntKC/X5N9buu+wl+zL0paFb\n\tp4lkgHEE7so3Zsv5gsK7q2nXwkpoKCOeWu8KUsWw=","Date":"Fri, 23 Jul 2021 16:33:47 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YPrFO6hwTUVHqV/I@pendragon.ideasonboard.com>","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210715153241.63971-2-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 1/3] android: capabilities: Use a\n\tthrow-away config for YUV stream building","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18289,"web_url":"https://patchwork.libcamera.org/comment/18289/","msgid":"<1a2adec7-6e09-ee47-76d8-f3568c277a62@ideasonboard.com>","date":"2021-07-23T13:36:58","subject":"Re: [libcamera-devel] [PATCH 1/3] android: capabilities: Use a\n\tthrow-away config for YUV stream building","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nThanks for the patch\n\nOn 7/15/21 9:02 PM, Jacopo Mondi wrote:\n> When building the list of supported YUV streams in getYUVResolutions()\n> the CameraConfiguration provided by the caller as first parameters is used.\n>\n> As the CameraConfiguration will be later actually applied to the Camera,\n> avoid any possible overlap of the configuration parameters by using a\n> throw-away CameraConfiguration generated for the Viewfinder stream role\n> in getYUVResolutions().\n>\n> It's also nicer to avoid having two functions with a similar purpose\n> such as getYUVResolutions() and getRawResolutions() with different\n> parameter lists, as the presence of a CameraConfiguration as first\n> parameter might be confusing to the reader.\n>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>   src/android/camera_capabilities.cpp | 10 +++++-----\n>   src/android/camera_capabilities.h   |  3 +--\n>   2 files changed, 6 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 6b5edb66fad2..6543c2bbc6c3 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -138,13 +138,14 @@ int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,\n>   \treturn initializeStaticMetadata();\n>   }\n>   \n> -std::vector<Size> CameraCapabilities::getYUVResolutions(CameraConfiguration *cameraConfig,\n> -\t\t\t\t\t\t\tconst PixelFormat &pixelFormat,\n> +std::vector<Size> CameraCapabilities::getYUVResolutions(const PixelFormat &pixelFormat,\n>   \t\t\t\t\t\t\tconst std::vector<Size> &resolutions)\n>   {\n>   \tstd::vector<Size> supportedResolutions;\n> -\n> +\tstd::unique_ptr<CameraConfiguration> cameraConfig =\n> +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n>   \tStreamConfiguration &cfg = cameraConfig->at(0);\n> +\n>   \tfor (const Size &res : resolutions) {\n>   \t\tcfg.pixelFormat = pixelFormat;\n>   \t\tcfg.size = res;\n> @@ -324,8 +325,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n>   \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n>   \t\t\tresolutions = getRawResolutions(mappedFormat);\n>   \t\telse\n> -\t\t\tresolutions = getYUVResolutions(cameraConfig.get(),\n> -\t\t\t\t\t\t\tmappedFormat,\n> +\t\t\tresolutions = getYUVResolutions(mappedFormat,\n>   \t\t\t\t\t\t\tcameraResolutions);\n>   \n>   \t\tfor (const Size &res : resolutions) {\n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index 4f5be82595d6..e72bf084cd65 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -43,8 +43,7 @@ private:\n>   \t};\n>   \n>   \tstd::vector<libcamera::Size>\n> -\tgetYUVResolutions(libcamera::CameraConfiguration *cameraConfig,\n> -\t\t\t  const libcamera::PixelFormat &pixelFormat,\n> +\tgetYUVResolutions(const libcamera::PixelFormat &pixelFormat,\n>   \t\t\t  const std::vector<libcamera::Size> &resolutions);\n>   \tstd::vector<libcamera::Size>\n>   \tgetRawResolutions(const libcamera::PixelFormat &pixelFormat);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 0D2CFC322C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 23 Jul 2021 13:37:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54147687A9;\n\tFri, 23 Jul 2021 15:37:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E8B968537\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 23 Jul 2021 15:37:02 +0200 (CEST)","from [192.168.0.107] (unknown [103.251.226.103])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B167D3F2;\n\tFri, 23 Jul 2021 15:37:01 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lNkLRE//\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627047422;\n\tbh=KTAGakiUViqxySvyczSH54AWdzIxSQNLSx5VjM/+aII=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=lNkLRE//MceIiICSRO5Y7RYFXxPeQ61ZqaPkTSHZc5SdOLe1CI1SxEQoBQH2B9yyC\n\tSFvNOD/SzRk2mSpOd1LUzilfqxaQ60cac2H0wH28sWNHZkUT8HRMRJUydotvgEWQ0I\n\t+t7UnDYte1j2N8DkzVc9o58C46UX2jK/dTwuPcCM=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-2-jacopo@jmondi.org>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<1a2adec7-6e09-ee47-76d8-f3568c277a62@ideasonboard.com>","Date":"Fri, 23 Jul 2021 19:06:58 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210715153241.63971-2-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 1/3] android: capabilities: Use a\n\tthrow-away config for YUV stream building","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]