[{"id":18286,"web_url":"https://patchwork.libcamera.org/comment/18286/","msgid":"","date":"2021-07-23T13:26:42","subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn 7/15/21 9:02 PM, Jacopo Mondi wrote:\n> The validation of RAW stream support is performed in two different\n> places:\n>\n> - At initializeStreamConfigurations() time, by verifying that the\n> libcamera format associated with HAL_PIXEL_FORMAT_BLOB is a Raw format\n> and ensuring the Camera successfully validates it\n> - As initializeStaticMetadata() time by generating a CameraConfiguration\n> for the Raw stream role and ensuring it is a Raw format with a 16 bit\n> depth\n>\n> The first check is used to build the list of supported Raw stream\n> resolutions. The latter is used to register the\n> ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability.\n>\n> As building the list of supported Raw streams doesn't serve any\n> purpose if the RAW capability is nor registered, centralize the Raw\ns/nor/not\n> format support verification at initializeStreamConfigurations() time by\n> ensuring the supported format is a Raw one with a 16 bit depth.\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_capabilities.cpp | 23 ++++++++---------------\n> src/android/camera_capabilities.h | 1 +\n> 2 files changed, 9 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 15e54192adff..9e2714f132c4 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -122,6 +122,7 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> \tcamera_ = camera;\n> \torientation_ = orientation;\n> \tfacing_ = facing;\n> +\trawStreamAvailable_ = false;\n> \n> \t/* Acquire the camera and initialize available stream configurations. */\n> \tint ret = camera_->acquire();\n> @@ -324,11 +325,14 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \n> \t\tstd::vector resolutions;\n> \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> +\t\t info.bitsPerPixel == 16) {\n> +\t\t\trawStreamAvailable_ = true;\n\nNice, to have this in one place now.\n\nReviewed-by: Umang Jain \n\n> \t\t\tresolutions = initializeRawResolutions(mappedFormat);\n> -\t\telse\n> +\t\t} else {\n> \t\t\tresolutions = initializeYUVResolutions(mappedFormat,\n> \t\t\t\t\t\t\t cameraResolutions);\n> +\t\t}\n> \n> \t\tfor (const Size &res : resolutions) {\n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> @@ -866,22 +870,11 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t};\n> \n> \t/* Report if camera supports RAW. */\n> -\tbool rawStreamAvailable = false;\n> -\tstd::unique_ptr cameraConfig =\n> -\t\tcamera_->generateConfiguration({ StreamRole::Raw });\n> -\tif (cameraConfig && !cameraConfig->empty()) {\n> -\t\tconst PixelFormatInfo &info =\n> -\t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> -\t\t/* Only advertise RAW support if RAW16 is possible. */\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> -\t\t info.bitsPerPixel == 16) {\n> -\t\t\trawStreamAvailable = true;\n> +\tif (rawStreamAvailable_)\n> \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> -\t\t}\n> -\t}\n> \n> \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> -\tint32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };\n> +\tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n> \n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index e7aa46c0a689..42a976d3b482 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -55,6 +55,7 @@ private:\n> \n> \tint facing_;\n> \tint orientation_;\n> +\tbool rawStreamAvailable_;\n> \n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;","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 00E68C322C\n\tfor ;\n\tFri, 23 Jul 2021 13:26:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74381687A9;\n\tFri, 23 Jul 2021 15:26:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB45F68537\n\tfor ;\n\tFri, 23 Jul 2021 15:26:48 +0200 (CEST)","from [192.168.0.107] (unknown [103.251.226.103])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E820B3F2;\n\tFri, 23 Jul 2021 15:26:47 +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=\"ZdLPAgau\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627046808;\n\tbh=NbJjuduqEi6H1PAHe4dkJx4DAGt3CZQIxvLESnloJ64=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ZdLPAgauicZA472Z/gu2A7uImtBHU44BYPLaoM93/juJkNfExd6dQGtiV6bBsnmcl\n\t+5M1k4COUukaLLHkkhq58zzqcOlyntybZLAMzdPGODwe8qwRDiNUpEacesubfJ/1Y8\n\tzrMNwECTd5wMDCrLgnDrNaW6NZ6x63JOBWiswtzY=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-4-jacopo@jmondi.org>","From":"Umang Jain ","Message-ID":"","Date":"Fri, 23 Jul 2021 18:56:42 +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-4-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 3/3] android: capabilities: Centralize\n\tRAW support check","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18291,"web_url":"https://patchwork.libcamera.org/comment/18291/","msgid":"","date":"2021-07-23T13:43:19","subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","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:41PM +0200, Jacopo Mondi wrote:\n> The validation of RAW stream support is performed in two different\n> places:\n> \n> - At initializeStreamConfigurations() time, by verifying that the\n> libcamera format associated with HAL_PIXEL_FORMAT_BLOB is a Raw format\n> and ensuring the Camera successfully validates it\n> - As initializeStaticMetadata() time by generating a CameraConfiguration\n> for the Raw stream role and ensuring it is a Raw format with a 16 bit\n> depth\n> \n> The first check is used to build the list of supported Raw stream\n> resolutions. The latter is used to register the\n> ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability.\n> \n> As building the list of supported Raw streams doesn't serve any\n> purpose if the RAW capability is nor registered, centralize the Raw\n> format support verification at initializeStreamConfigurations() time by\n> ensuring the supported format is a Raw one with a 16 bit depth.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_capabilities.cpp | 23 ++++++++---------------\n> src/android/camera_capabilities.h | 1 +\n> 2 files changed, 9 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 15e54192adff..9e2714f132c4 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -122,6 +122,7 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> \tcamera_ = camera;\n> \torientation_ = orientation;\n> \tfacing_ = facing;\n> +\trawStreamAvailable_ = false;\n> \n> \t/* Acquire the camera and initialize available stream configurations. */\n> \tint ret = camera_->acquire();\n> @@ -324,11 +325,14 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \n> \t\tstd::vector resolutions;\n> \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> +\t\t info.bitsPerPixel == 16) {\n> +\t\t\trawStreamAvailable_ = true;\n> \t\t\tresolutions = initializeRawResolutions(mappedFormat);\n> -\t\telse\n> +\t\t} else {\n\nShouldn't you skip here the raw formats that have other bpp than 16 ?\nAnd probably the RGB formats too ?\n\n> \t\t\tresolutions = initializeYUVResolutions(mappedFormat,\n> \t\t\t\t\t\t\t cameraResolutions);\n> +\t\t}\n> \n> \t\tfor (const Size &res : resolutions) {\n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> @@ -866,22 +870,11 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t};\n> \n> \t/* Report if camera supports RAW. */\n> -\tbool rawStreamAvailable = false;\n> -\tstd::unique_ptr cameraConfig =\n> -\t\tcamera_->generateConfiguration({ StreamRole::Raw });\n> -\tif (cameraConfig && !cameraConfig->empty()) {\n> -\t\tconst PixelFormatInfo &info =\n> -\t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> -\t\t/* Only advertise RAW support if RAW16 is possible. */\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> -\t\t info.bitsPerPixel == 16) {\n> -\t\t\trawStreamAvailable = true;\n> +\tif (rawStreamAvailable_)\n> \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> -\t\t}\n> -\t}\n> \n> \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> -\tint32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };\n> +\tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n> \n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index e7aa46c0a689..42a976d3b482 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -55,6 +55,7 @@ private:\n> \n> \tint facing_;\n> \tint orientation_;\n> +\tbool rawStreamAvailable_;\n> \n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;","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 3A707C0109\n\tfor ;\n\tFri, 23 Jul 2021 13:43:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C508687AC;\n\tFri, 23 Jul 2021 15:43:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87C4E68537\n\tfor ;\n\tFri, 23 Jul 2021 15:43:21 +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 119053F2;\n\tFri, 23 Jul 2021 15:43:21 +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=\"g2aQ8i9L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627047801;\n\tbh=OI5Ea8kiN6mBWngzZp3acW/GFI8PTYL3sfyfIDwxiUc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=g2aQ8i9Lmqe/YStTdsNQ9C6h31XY43576hozPVsAfhvF4Na436LbZkSLjlJDpaZB7\n\t0WA/FQ/zHRl83irAG1aBr9bg+gMsWzK+rwMC804jUyzagUFCWKvFG2gWzjh1PFWaS/\n\t+uh7KFgjwHVprrL9GZbyvGVuBEiEHxY28ONzT5G8=","Date":"Fri, 23 Jul 2021 16:43:19 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210715153241.63971-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18343,"web_url":"https://patchwork.libcamera.org/comment/18343/","msgid":"<20210726085128.nrcqf5vatfbfyykz@uno.localdomain>","date":"2021-07-26T08:51:28","subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Fri, Jul 23, 2021 at 04:43:19PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Jul 15, 2021 at 05:32:41PM +0200, Jacopo Mondi wrote:\n> > The validation of RAW stream support is performed in two different\n> > places:\n> >\n> > - At initializeStreamConfigurations() time, by verifying that the\n> > libcamera format associated with HAL_PIXEL_FORMAT_BLOB is a Raw format\n> > and ensuring the Camera successfully validates it\n> > - As initializeStaticMetadata() time by generating a CameraConfiguration\n> > for the Raw stream role and ensuring it is a Raw format with a 16 bit\n> > depth\n> >\n> > The first check is used to build the list of supported Raw stream\n> > resolutions. The latter is used to register the\n> > ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability.\n> >\n> > As building the list of supported Raw streams doesn't serve any\n> > purpose if the RAW capability is nor registered, centralize the Raw\n> > format support verification at initializeStreamConfigurations() time by\n> > ensuring the supported format is a Raw one with a 16 bit depth.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_capabilities.cpp | 23 ++++++++---------------\n> > src/android/camera_capabilities.h | 1 +\n> > 2 files changed, 9 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 15e54192adff..9e2714f132c4 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -122,6 +122,7 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> > \tcamera_ = camera;\n> > \torientation_ = orientation;\n> > \tfacing_ = facing;\n> > +\trawStreamAvailable_ = false;\n> >\n> > \t/* Acquire the camera and initialize available stream configurations. */\n> > \tint ret = camera_->acquire();\n> > @@ -324,11 +325,14 @@ int CameraCapabilities::initializeStreamConfigurations()\n> >\n> > \t\tstd::vector resolutions;\n> > \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> > -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> > +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> > +\t\t info.bitsPerPixel == 16) {\n> > +\t\t\trawStreamAvailable_ = true;\n> > \t\t\tresolutions = initializeRawResolutions(mappedFormat);\n> > -\t\telse\n> > +\t\t} else {\n>\n> Shouldn't you skip here the raw formats that have other bpp than 16 ?\n\nCorrect! Raw !16bpp formats should not be enumerated in the else\nbranch\n\n> And probably the RGB formats too ?\n\nShould we ? The initialization walks the camera3FormatsMap where the\nformats mapping is defined. If for any reason (processed) RGB formats are\nlisted there (in example to support IMPLEMENTATION_DEFINED) why would\nwe ignore them ?\n\nThanks\n j\n\n>\n> > \t\t\tresolutions = initializeYUVResolutions(mappedFormat,\n> > \t\t\t\t\t\t\t cameraResolutions);\n> > +\t\t}\n> >\n> > \t\tfor (const Size &res : resolutions) {\n> > \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> > @@ -866,22 +870,11 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t};\n> >\n> > \t/* Report if camera supports RAW. */\n> > -\tbool rawStreamAvailable = false;\n> > -\tstd::unique_ptr cameraConfig =\n> > -\t\tcamera_->generateConfiguration({ StreamRole::Raw });\n> > -\tif (cameraConfig && !cameraConfig->empty()) {\n> > -\t\tconst PixelFormatInfo &info =\n> > -\t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> > -\t\t/* Only advertise RAW support if RAW16 is possible. */\n> > -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> > -\t\t info.bitsPerPixel == 16) {\n> > -\t\t\trawStreamAvailable = true;\n> > +\tif (rawStreamAvailable_)\n> > \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > -\t\t}\n> > -\t}\n> >\n> > \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> > -\tint32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };\n> > +\tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> > \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> > \t\t\t\t numOutStreams);\n> >\n> > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > index e7aa46c0a689..42a976d3b482 100644\n> > --- a/src/android/camera_capabilities.h\n> > +++ b/src/android/camera_capabilities.h\n> > @@ -55,6 +55,7 @@ private:\n> >\n> > \tint facing_;\n> > \tint orientation_;\n> > +\tbool rawStreamAvailable_;\n> >\n> > \tstd::vector streamConfigurations_;\n> > \tstd::map formatsMap_;\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 3C2EAC322C\n\tfor ;\n\tMon, 26 Jul 2021 08:50:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00E7C687B8;\n\tMon, 26 Jul 2021 10:50:42 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 163D8687B3\n\tfor ;\n\tMon, 26 Jul 2021 10:50:41 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id A60A0240011;\n\tMon, 26 Jul 2021 08:50:40 +0000 (UTC)"],"Date":"Mon, 26 Jul 2021 10:51:28 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20210726085128.nrcqf5vatfbfyykz@uno.localdomain>","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-4-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18344,"web_url":"https://patchwork.libcamera.org/comment/18344/","msgid":"","date":"2021-07-26T09:21:54","subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jul 26, 2021 at 10:51:28AM +0200, Jacopo Mondi wrote:\n> On Fri, Jul 23, 2021 at 04:43:19PM +0300, Laurent Pinchart wrote:\n> > On Thu, Jul 15, 2021 at 05:32:41PM +0200, Jacopo Mondi wrote:\n> > > The validation of RAW stream support is performed in two different\n> > > places:\n> > >\n> > > - At initializeStreamConfigurations() time, by verifying that the\n> > > libcamera format associated with HAL_PIXEL_FORMAT_BLOB is a Raw format\n> > > and ensuring the Camera successfully validates it\n> > > - As initializeStaticMetadata() time by generating a CameraConfiguration\n> > > for the Raw stream role and ensuring it is a Raw format with a 16 bit\n> > > depth\n> > >\n> > > The first check is used to build the list of supported Raw stream\n> > > resolutions. The latter is used to register the\n> > > ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability.\n> > >\n> > > As building the list of supported Raw streams doesn't serve any\n> > > purpose if the RAW capability is nor registered, centralize the Raw\n> > > format support verification at initializeStreamConfigurations() time by\n> > > ensuring the supported format is a Raw one with a 16 bit depth.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_capabilities.cpp | 23 ++++++++---------------\n> > > src/android/camera_capabilities.h | 1 +\n> > > 2 files changed, 9 insertions(+), 15 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 15e54192adff..9e2714f132c4 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -122,6 +122,7 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> > > \tcamera_ = camera;\n> > > \torientation_ = orientation;\n> > > \tfacing_ = facing;\n> > > +\trawStreamAvailable_ = false;\n> > >\n> > > \t/* Acquire the camera and initialize available stream configurations. */\n> > > \tint ret = camera_->acquire();\n> > > @@ -324,11 +325,14 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > >\n> > > \t\tstd::vector resolutions;\n> > > \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> > > -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> > > +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> > > +\t\t info.bitsPerPixel == 16) {\n> > > +\t\t\trawStreamAvailable_ = true;\n> > > \t\t\tresolutions = initializeRawResolutions(mappedFormat);\n> > > -\t\telse\n> > > +\t\t} else {\n> >\n> > Shouldn't you skip here the raw formats that have other bpp than 16 ?\n> \n> Correct! Raw !16bpp formats should not be enumerated in the else\n> branch\n> \n> > And probably the RGB formats too ?\n> \n> Should we ? The initialization walks the camera3FormatsMap where the\n> formats mapping is defined. If for any reason (processed) RGB formats are\n> listed there (in example to support IMPLEMENTATION_DEFINED) why would\n> we ignore them ?\n\nIndeed. That's probably unlikely, but it may happen. However, calling\ninitializeYUVResolutions() for an RGB format sounds weird.\n\n> > > \t\t\tresolutions = initializeYUVResolutions(mappedFormat,\n> > > \t\t\t\t\t\t\t cameraResolutions);\n> > > +\t\t}\n> > >\n> > > \t\tfor (const Size &res : resolutions) {\n> > > \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> > > @@ -866,22 +870,11 @@ int CameraCapabilities::initializeStaticMetadata()\n> > > \t};\n> > >\n> > > \t/* Report if camera supports RAW. */\n> > > -\tbool rawStreamAvailable = false;\n> > > -\tstd::unique_ptr cameraConfig =\n> > > -\t\tcamera_->generateConfiguration({ StreamRole::Raw });\n> > > -\tif (cameraConfig && !cameraConfig->empty()) {\n> > > -\t\tconst PixelFormatInfo &info =\n> > > -\t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> > > -\t\t/* Only advertise RAW support if RAW16 is possible. */\n> > > -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> > > -\t\t info.bitsPerPixel == 16) {\n> > > -\t\t\trawStreamAvailable = true;\n> > > +\tif (rawStreamAvailable_)\n> > > \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > > -\t\t}\n> > > -\t}\n> > >\n> > > \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> > > -\tint32_t numOutStreams[] = { rawStreamAvailable, 2, 1 };\n> > > +\tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> > > \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> > > \t\t\t\t numOutStreams);\n> > >\n> > > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > > index e7aa46c0a689..42a976d3b482 100644\n> > > --- a/src/android/camera_capabilities.h\n> > > +++ b/src/android/camera_capabilities.h\n> > > @@ -55,6 +55,7 @@ private:\n> > >\n> > > \tint facing_;\n> > > \tint orientation_;\n> > > +\tbool rawStreamAvailable_;\n> > >\n> > > \tstd::vector streamConfigurations_;\n> > > \tstd::map formatsMap_;","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 9E5B8C0109\n\tfor ;\n\tMon, 26 Jul 2021 09:22:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E93A1687BA;\n\tMon, 26 Jul 2021 11:21:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6031360272\n\tfor ;\n\tMon, 26 Jul 2021 11:21:59 +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 D19063F;\n\tMon, 26 Jul 2021 11:21:58 +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=\"f5X1jTWn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627291319;\n\tbh=DJnkhSwj0ZGFcfYRCpnLDfbL2N1Cmjg857xRg7zlkH8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=f5X1jTWnaA2lMLaFwyNhc8TmKBZ8J9c8tT9hiFtU7Tc/VCEWWdb7K0j99VqaEM4Pm\n\tH5FZ1pivxq8IBgRm3rIGhCpYF9p44VpCqyQN+J+MldmUl6qIQHAD7swKkgwkXtDBv7\n\tdTXXEOXXgbjnonC/uT4J80UMthO3tyFAzT4v8Usk=","Date":"Mon, 26 Jul 2021 12:21:54 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210715153241.63971-1-jacopo@jmondi.org>\n\t<20210715153241.63971-4-jacopo@jmondi.org>\n\t\n\t<20210726085128.nrcqf5vatfbfyykz@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210726085128.nrcqf5vatfbfyykz@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: capabilities: Centralize\n\tRAW support check","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]