[{"id":14429,"web_url":"https://patchwork.libcamera.org/comment/14429/","msgid":"","date":"2020-12-31T15:16:48","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Only\n\tadvertise RAW support if RAW16 is available","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, Dec 31, 2020 at 11:02:18AM +0100, Niklas Söderlund wrote:\n> The HAL expects RAW16 support if the RAW capability is set, add a check\n\nMaybe \"The Android camera service\" instead of \"The HAL\" to emphasize\nthis is not an internal requirement of our implementation ? Or maybe we\nshould even expand the commit message to explain the requirement ? It\nwasn't clear from the documentation (although re-reading it now, I\nwonder why) and took us some time to figure it out, it's useful to\ncapture it. How about this ?\n\nThe Android camera2 API defines a RAW capture capability ([1]) for\ndevices that support \"outputting RAW buffers and metadata for\ninterpreting them\". This capability requires the camera device to\nsupport RAW_SENSOR ([2]) as an output format. Despite what its name may\nsound like, the RAW_SENSOR format is defined as a 16 bits RAW format,\nnot an opaque implementation-dependent format (which is instead called\nRAW_PRIVATE). Devices may additionally support the RAW10 and RAW12\nformats, but that isn't enough to claim RAW capture capability.\n\nTo comply with the API requirements, only report the\nANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability when 16-bit RAW is\nsupported.\n\n[1] https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#REQUEST_AVAILABLE_CAPABILITIES_RAW\n[2] https://developer.android.com/reference/android/graphics/ImageFormat#RAW_SENSOR\n\n> for this.\n> \n> Suggested-by: Jacopo Mondi \n> Signed-off-by: Niklas Söderlund \n\nTested-by: Laurent Pinchart \nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_device.cpp | 4 +++-\n> 1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 7678d4485ce987b5..23be2a593a2aee5e 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1099,7 +1099,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tif (cameraConfig && !cameraConfig->empty()) {\n> \t\tconst PixelFormatInfo &info =\n> \t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\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> \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> \t\t}","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 E2C76C0F1A\n\tfor ;\n\tThu, 31 Dec 2020 15:17:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5DFF66031B;\n\tThu, 31 Dec 2020 16:17:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E21BF6031B\n\tfor ;\n\tThu, 31 Dec 2020 16:16:59 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C60252F;\n\tThu, 31 Dec 2020 16:16:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"flh/vfo+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609427819;\n\tbh=wsmBS8jpFpdQ7x53Jz7z0ZYyUQIafk3bqgUhOlQP8Rs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=flh/vfo+pcoy7nep915LiWW9Lc+31tsJzAyQBFnBcNQmWXk1ErbbcPtIKUOSsTPIT\n\tKiUwdj2LooLSLLDjlk7FiT5DFSeEnAGGMnSNQGnTPZ5oC8sfSIWOiwJLiDoa6apvsr\n\tnNAf1FYItGGpulscURou4FRB9TL5WaIjurvyNs+Y=","Date":"Thu, 31 Dec 2020 17:16:48 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20201231100218.1365123-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201231100218.1365123-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Only\n\tadvertise RAW support if RAW16 is available","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":14430,"web_url":"https://patchwork.libcamera.org/comment/14430/","msgid":"","date":"2020-12-31T15:17:25","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Only\n\tadvertise RAW support if RAW16 is available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Dec 31, 2020 at 05:16:49PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n\nOr rather Niklas. My fingers type Jacopo's name automatically when\nreplying to patches for the HAL :-)\n\n> Thank you for the patch.\n> \n> On Thu, Dec 31, 2020 at 11:02:18AM +0100, Niklas Söderlund wrote:\n> > The HAL expects RAW16 support if the RAW capability is set, add a check\n> \n> Maybe \"The Android camera service\" instead of \"The HAL\" to emphasize\n> this is not an internal requirement of our implementation ? Or maybe we\n> should even expand the commit message to explain the requirement ? It\n> wasn't clear from the documentation (although re-reading it now, I\n> wonder why) and took us some time to figure it out, it's useful to\n> capture it. How about this ?\n> \n> The Android camera2 API defines a RAW capture capability ([1]) for\n> devices that support \"outputting RAW buffers and metadata for\n> interpreting them\". This capability requires the camera device to\n> support RAW_SENSOR ([2]) as an output format. Despite what its name may\n> sound like, the RAW_SENSOR format is defined as a 16 bits RAW format,\n> not an opaque implementation-dependent format (which is instead called\n> RAW_PRIVATE). Devices may additionally support the RAW10 and RAW12\n> formats, but that isn't enough to claim RAW capture capability.\n> \n> To comply with the API requirements, only report the\n> ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW capability when 16-bit RAW is\n> supported.\n> \n> [1] https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#REQUEST_AVAILABLE_CAPABILITIES_RAW\n> [2] https://developer.android.com/reference/android/graphics/ImageFormat#RAW_SENSOR\n> \n> > for this.\n> > \n> > Suggested-by: Jacopo Mondi \n> > Signed-off-by: Niklas Söderlund \n> \n> Tested-by: Laurent Pinchart \n> Reviewed-by: Laurent Pinchart \n> \n> > ---\n> > src/android/camera_device.cpp | 4 +++-\n> > 1 file changed, 3 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 7678d4485ce987b5..23be2a593a2aee5e 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1099,7 +1099,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > \tif (cameraConfig && !cameraConfig->empty()) {\n> > \t\tconst PixelFormatInfo &info =\n> > \t\t\tPixelFormatInfo::info(cameraConfig->at(0).pixelFormat);\n> > -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\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> > \t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > \t\t}","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 08EDDC0F1A\n\tfor ;\n\tThu, 31 Dec 2020 15:17:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C083C6031D;\n\tThu, 31 Dec 2020 16:17:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4713C6031B\n\tfor ;\n\tThu, 31 Dec 2020 16:17:37 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C74EB52F;\n\tThu, 31 Dec 2020 16:17:36 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bez4uBd5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609427857;\n\tbh=IKywf0ev7Qhrohhm8jGtfAixH7YeScrVNRf/TUxerGk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bez4uBd5axt6mRxY7PL82zJn92t4pCNvnsVcfY1hZzNdUQw7oxS9amEqXPqVQH+K7\n\t/fqUVlhtYTi+qJjv2WNk3/a1gWiDRCZkYBHlsjoRanka8VvEbaMvn440z5b+cW1c6V\n\tuk2TL/nsOrRu0Ss0VxHNBwniwqXOH20fIMUN7jJ0=","Date":"Thu, 31 Dec 2020 17:17:25 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20201231100218.1365123-1-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Only\n\tadvertise RAW support if RAW16 is available","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\" "}}]