[{"id":21468,"web_url":"https://patchwork.libcamera.org/comment/21468/","msgid":"<20211130213041.lnhvlfr5nn7q7qsn@uno.localdomain>","date":"2021-11-30T21:30:41","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro\n\nOn Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:\n> This adds a function to CameraCapabilities of converting PixelFormat\n> to android format.\n>\n> Signed-off-by: Hirokazu Honda \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/android/camera_capabilities.cpp | 18 ++++++++++++++++++\n> src/android/camera_capabilities.h | 2 ++\n> 2 files changed, 20 insertions(+)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index f357902e..4c1dc87c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \t\t */\n> \t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> \t\t\tformatsMap_[androidFormat] = formats::MJPEG;\n> +\t\t\trevFormatsMap_[formats::MJPEG] = androidFormat;\n> \t\t\tLOG(HAL, Debug) << \"Mapped Android format \"\n> \t\t\t\t\t<< camera3Format.name << \" to \"\n> \t\t\t\t\t<< formats::MJPEG.toString()\n> @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \t\t * stream configurations map, by testing the image resolutions.\n> \t\t */\n> \t\tformatsMap_[androidFormat] = mappedFormat;\n> +\t\trevFormatsMap_[mappedFormat] = androidFormat;\n> \t\tLOG(HAL, Debug) << \"Mapped Android format \"\n> \t\t\t\t<< camera3Format.name << \" to \"\n> \t\t\t\t<< mappedFormat.toString();\n> @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const\n> \treturn it->second;\n> }\n>\n> +/*\n> + * Translate libcamera pixel format to Android format code. -1 if no mapped\n> + * android format is found.\n> + */\n> +int CameraCapabilities::toAndroidFormat(PixelFormat format) const\n> +{\n> +\tauto it = revFormatsMap_.find(format);\n> +\tif (it == revFormatsMap_.end()) {\n> +\t\tLOG(HAL, Error) << \"Requested format \" << format.toString()\n> +\t\t\t\t<< \" not supported\";\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn it->second;\n> +}\n> +\n> std::unique_ptr CameraCapabilities::requestTemplateManual() const\n> {\n> \tif (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {\n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index 2cf97ae8..df6361f1 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -30,6 +30,7 @@ public:\n>\n> \tCameraMetadata *staticMetadata() const { return staticMetadata_.get(); }\n> \tlibcamera::PixelFormat toPixelFormat(int format) const;\n> +\tint toAndroidFormat(libcamera::PixelFormat format) const;\n> \tunsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }\n>\n> \tstd::unique_ptr requestTemplateManual() const;\n> @@ -77,6 +78,7 @@ private:\n>\n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;\n> +\tstd::map revFormatsMap_;\n> \tstd::unique_ptr staticMetadata_;\n> \tunsigned int maxJpegBufferSize_;\n>\n> --\n> 2.34.0.rc2.393.gf8c9666880-goog\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 A8E3ABDB13\n\tfor ;\n\tTue, 30 Nov 2021 21:29:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2E86060720;\n\tTue, 30 Nov 2021 22:29:51 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 139C1605C4\n\tfor ;\n\tTue, 30 Nov 2021 22:29:50 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 3B5EA1BF205;\n\tTue, 30 Nov 2021 21:29:49 +0000 (UTC)"],"Date":"Tue, 30 Nov 2021 22:30:41 +0100","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20211130213041.lnhvlfr5nn7q7qsn@uno.localdomain>","References":"<20211130124428.2163669-1-hiroh@chromium.org>\n\t<20211130124428.2163669-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211130124428.2163669-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","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":21487,"web_url":"https://patchwork.libcamera.org/comment/21487/","msgid":"","date":"2021-12-01T02:26:16","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:\n> This adds a function to CameraCapabilities of converting PixelFormat\n> to android format.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> src/android/camera_capabilities.cpp | 18 ++++++++++++++++++\n> src/android/camera_capabilities.h | 2 ++\n> 2 files changed, 20 insertions(+)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index f357902e..4c1dc87c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \t\t */\n> \t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> \t\t\tformatsMap_[androidFormat] = formats::MJPEG;\n> +\t\t\trevFormatsMap_[formats::MJPEG] = androidFormat;\n> \t\t\tLOG(HAL, Debug) << \"Mapped Android format \"\n> \t\t\t\t\t<< camera3Format.name << \" to \"\n> \t\t\t\t\t<< formats::MJPEG.toString()\n> @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> \t\t * stream configurations map, by testing the image resolutions.\n> \t\t */\n> \t\tformatsMap_[androidFormat] = mappedFormat;\n> +\t\trevFormatsMap_[mappedFormat] = androidFormat;\n> \t\tLOG(HAL, Debug) << \"Mapped Android format \"\n> \t\t\t\t<< camera3Format.name << \" to \"\n> \t\t\t\t<< mappedFormat.toString();\n> @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const\n> \treturn it->second;\n> }\n> \n> +/*\n> + * Translate libcamera pixel format to Android format code. -1 if no mapped\n> + * android format is found.\n> + */\n> +int CameraCapabilities::toAndroidFormat(PixelFormat format) const\n> +{\n> +\tauto it = revFormatsMap_.find(format);\n> +\tif (it == revFormatsMap_.end()) {\n> +\t\tLOG(HAL, Error) << \"Requested format \" << format.toString()\n> +\t\t\t\t<< \" not supported\";\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn it->second;\n\nI think there's a conceptual problem here. While a given Android format\ncan always be translated to a PixelFormat, the mapping isn't 1:1. We\nalready map different Android formats to NV21 for instance, and I expect\nthis to continue as we support more formats. A reverse map is thus\nill-defined, there's no guarantee that\n\n\ttoAndroidFormat(toPixelFormat(format)) == format\n\nwhich I think can lead to subtle bugs. Can't we instead, in the next\npatch, get the Android format from camera3Stream_->format ?\n\n> +}\n> +\n> std::unique_ptr CameraCapabilities::requestTemplateManual() const\n> {\n> \tif (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {\n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index 2cf97ae8..df6361f1 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -30,6 +30,7 @@ public:\n> \n> \tCameraMetadata *staticMetadata() const { return staticMetadata_.get(); }\n> \tlibcamera::PixelFormat toPixelFormat(int format) const;\n> +\tint toAndroidFormat(libcamera::PixelFormat format) const;\n> \tunsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }\n> \n> \tstd::unique_ptr requestTemplateManual() const;\n> @@ -77,6 +78,7 @@ private:\n> \n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;\n> +\tstd::map revFormatsMap_;\n> \tstd::unique_ptr staticMetadata_;\n> \tunsigned int maxJpegBufferSize_;\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 9CF95BDB13\n\tfor ;\n\tWed, 1 Dec 2021 02:26:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 048C560710;\n\tWed, 1 Dec 2021 03:26:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B405F60592\n\tfor ;\n\tWed, 1 Dec 2021 03:26:42 +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 BCB408AE;\n\tWed, 1 Dec 2021 03:26:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Y1VZDhMJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638325602;\n\tbh=Kx9NFm17AIxHu8iegFeCgUzJH3CCCFPGlM8Pz68A9bE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Y1VZDhMJxepev7vUDNAhraNJuuxzyvho3sNBjhJgf65h1sHUXu34gv9OeEyKJ0HHk\n\tuVbh/9vTmFTGulj8a1kiTTMVhW2Fv+r6xiU9Uh4cBuU4flmZRchBUFmTUP/i0sQxfp\n\thWghxYW0ffQh9Wt9RqGnfpp6FXUpmqGltQREpcCQ=","Date":"Wed, 1 Dec 2021 04:26:16 +0200","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211130124428.2163669-1-hiroh@chromium.org>\n\t<20211130124428.2163669-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211130124428.2163669-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","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":21491,"web_url":"https://patchwork.libcamera.org/comment/21491/","msgid":"","date":"2021-12-01T07:28:07","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent, thank you for reviewing.\n\nOn Wed, Dec 1, 2021 at 11:26 AM Laurent Pinchart\n wrote:\n>\n> Hi Hiro,\n>\n> Thank you for the patch.\n>\n> On Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:\n> > This adds a function to CameraCapabilities of converting PixelFormat\n> > to android format.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > src/android/camera_capabilities.cpp | 18 ++++++++++++++++++\n> > src/android/camera_capabilities.h | 2 ++\n> > 2 files changed, 20 insertions(+)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index f357902e..4c1dc87c 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > */\n> > if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> > formatsMap_[androidFormat] = formats::MJPEG;\n> > + revFormatsMap_[formats::MJPEG] = androidFormat;\n> > LOG(HAL, Debug) << \"Mapped Android format \"\n> > << camera3Format.name << \" to \"\n> > << formats::MJPEG.toString()\n> > @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > * stream configurations map, by testing the image resolutions.\n> > */\n> > formatsMap_[androidFormat] = mappedFormat;\n> > + revFormatsMap_[mappedFormat] = androidFormat;\n> > LOG(HAL, Debug) << \"Mapped Android format \"\n> > << camera3Format.name << \" to \"\n> > << mappedFormat.toString();\n> > @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const\n> > return it->second;\n> > }\n> >\n> > +/*\n> > + * Translate libcamera pixel format to Android format code. -1 if no mapped\n> > + * android format is found.\n> > + */\n> > +int CameraCapabilities::toAndroidFormat(PixelFormat format) const\n> > +{\n> > + auto it = revFormatsMap_.find(format);\n> > + if (it == revFormatsMap_.end()) {\n> > + LOG(HAL, Error) << \"Requested format \" << format.toString()\n> > + << \" not supported\";\n> > + return -1;\n> > + }\n> > +\n> > + return it->second;\n>\n> I think there's a conceptual problem here. While a given Android format\n> can always be translated to a PixelFormat, the mapping isn't 1:1. We\n> already map different Android formats to NV21 for instance, and I expect\n> this to continue as we support more formats. A reverse map is thus\n> ill-defined, there's no guarantee that\n>\n> toAndroidFormat(toPixelFormat(format)) == format\n>\n> which I think can lead to subtle bugs. Can't we instead, in the next\n> patch, get the Android format from camera3Stream_->format ?\n>\n\ncamera3Stream_->format is what Android HAL client requests.\nFrom camera3Stream_->format, we decide to request configuration to camera.\nSince the created buffer is the input to camera, its format must be\nconfiguration().pixelFormat.\nBoth PlatformFrameBufferAllocator (gralloc and\ncros::CameraBufferManager) demands android format.\nFurther, pixelFormat resolved from android format is dependent on the\nbackend implementation.\nTherefore it is probably correct to lookup a proper android format\nfrom pixelFormat.\nBut there is no such API in gralloc API and cros::CameraBufferManager.\n\n-Hiro\n> > +}\n> > +\n> > std::unique_ptr CameraCapabilities::requestTemplateManual() const\n> > {\n> > if (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {\n> > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > index 2cf97ae8..df6361f1 100644\n> > --- a/src/android/camera_capabilities.h\n> > +++ b/src/android/camera_capabilities.h\n> > @@ -30,6 +30,7 @@ public:\n> >\n> > CameraMetadata *staticMetadata() const { return staticMetadata_.get(); }\n> > libcamera::PixelFormat toPixelFormat(int format) const;\n> > + int toAndroidFormat(libcamera::PixelFormat format) const;\n> > unsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }\n> >\n> > std::unique_ptr requestTemplateManual() const;\n> > @@ -77,6 +78,7 @@ private:\n> >\n> > std::vector streamConfigurations_;\n> > std::map formatsMap_;\n> > + std::map revFormatsMap_;\n> > std::unique_ptr staticMetadata_;\n> > unsigned int maxJpegBufferSize_;\n> >\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 4C62BBDB13\n\tfor ;\n\tWed, 1 Dec 2021 07:28:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B572060720;\n\tWed, 1 Dec 2021 08:28:18 +0100 (CET)","from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com\n\t[IPv6:2a00:1450:4864:20::52d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B262160592\n\tfor ;\n\tWed, 1 Dec 2021 08:28:17 +0100 (CET)","by mail-ed1-x52d.google.com with SMTP id z5so32510345edd.3\n\tfor ;\n\tTue, 30 Nov 2021 23:28:17 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"NFgzwOdb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=1FCceijWkfrRKuK5QTLKrI4J4um/aUr8oreoinQY2Lk=;\n\tb=NFgzwOdbtGg4CHJBVblJ3wOV+XNk9NYV0wslM6QfuUK7bY+eM3tnRxNO0/Nyw/PhLe\n\tdBrPTFNfo4Ykte4x2P48x/8aU7be1/OjaKhUJzqK8wvmaIEL2rtBzt7WdnuldeKev5TU\n\tXfof7uENDNAMGQ5fNFhjQX9NQC95Lj3gvW04s=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=1FCceijWkfrRKuK5QTLKrI4J4um/aUr8oreoinQY2Lk=;\n\tb=VAlAf+H7HwlAn+myVlG4w8EyFrw8zDe/vKNdkI9E//t+LdYPqTnnyxGEfJtbiXVHEo\n\t09UPF2hk/nVCzLjkRtkBwJS1FnXnrOUR9xjmBsc0jgF2q70nzDNzBfGZRgRrT2FSTqFf\n\tvVd2j6aTnW0scjl9y7bHs0C4WP0isay2sV6GDMRUKXNiu2JPAmQdIJSXaVP6mwQrPHHS\n\tME0MN67pyLo5Rplrac0ANFu+/uC6BEqXIXkuh0ThZofNZReB5sS1HTfavypKhhxP+uSP\n\tvjnuuR8ZWbf3lmVnD8LDp501F1CSjvwnt649Xj5lR7WMLri8slyNmXZF1lNJm4NtSJyt\n\tatfw==","X-Gm-Message-State":"AOAM530Hmk9Z77OARUXWv1AXgdSpwg8k9f2bK4yTlJbB1xHfj/MkKj9Q\n\t0dfea/VOLJpiRNLn74jwX+wwvB6VnP1lLrWwbHSxiMqG2xmB8g==","X-Google-Smtp-Source":"ABdhPJzC0Yl1P7byY73o/zdt2ZSP/GRUUZSWQedNOHcJsLzlP6JUwH3zJz02Q4LGbPDc1GQXrs8MKkVEtbubbnxrz0o=","X-Received":"by 2002:a17:906:7688:: with SMTP id\n\to8mr5023071ejm.291.1638343697200; \n\tTue, 30 Nov 2021 23:28:17 -0800 (PST)","MIME-Version":"1.0","References":"<20211130124428.2163669-1-hiroh@chromium.org>\n\t<20211130124428.2163669-2-hiroh@chromium.org>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Wed, 1 Dec 2021 16:28:07 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","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":21602,"web_url":"https://patchwork.libcamera.org/comment/21602/","msgid":"","date":"2021-12-06T11:09:31","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nOn Wed, Dec 01, 2021 at 04:28:07PM +0900, Hirokazu Honda wrote:\n> On Wed, Dec 1, 2021 at 11:26 AM Laurent Pinchart wrote:\n> > On Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:\n> > > This adds a function to CameraCapabilities of converting PixelFormat\n> > > to android format.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > src/android/camera_capabilities.cpp | 18 ++++++++++++++++++\n> > > src/android/camera_capabilities.h | 2 ++\n> > > 2 files changed, 20 insertions(+)\n> > >\n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index f357902e..4c1dc87c 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > > */\n> > > if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> > > formatsMap_[androidFormat] = formats::MJPEG;\n> > > + revFormatsMap_[formats::MJPEG] = androidFormat;\n> > > LOG(HAL, Debug) << \"Mapped Android format \"\n> > > << camera3Format.name << \" to \"\n> > > << formats::MJPEG.toString()\n> > > @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > > * stream configurations map, by testing the image resolutions.\n> > > */\n> > > formatsMap_[androidFormat] = mappedFormat;\n> > > + revFormatsMap_[mappedFormat] = androidFormat;\n> > > LOG(HAL, Debug) << \"Mapped Android format \"\n> > > << camera3Format.name << \" to \"\n> > > << mappedFormat.toString();\n> > > @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const\n> > > return it->second;\n> > > }\n> > >\n> > > +/*\n> > > + * Translate libcamera pixel format to Android format code. -1 if no mapped\n> > > + * android format is found.\n> > > + */\n> > > +int CameraCapabilities::toAndroidFormat(PixelFormat format) const\n> > > +{\n> > > + auto it = revFormatsMap_.find(format);\n> > > + if (it == revFormatsMap_.end()) {\n> > > + LOG(HAL, Error) << \"Requested format \" << format.toString()\n> > > + << \" not supported\";\n> > > + return -1;\n> > > + }\n> > > +\n> > > + return it->second;\n> >\n> > I think there's a conceptual problem here. While a given Android format\n> > can always be translated to a PixelFormat, the mapping isn't 1:1. We\n> > already map different Android formats to NV21 for instance, and I expect\n> > this to continue as we support more formats. A reverse map is thus\n> > ill-defined, there's no guarantee that\n> >\n> > toAndroidFormat(toPixelFormat(format)) == format\n> >\n> > which I think can lead to subtle bugs. Can't we instead, in the next\n> > patch, get the Android format from camera3Stream_->format ?\n> \n> camera3Stream_->format is what Android HAL client requests.\n> From camera3Stream_->format, we decide to request configuration to camera.\n> Since the created buffer is the input to camera, its format must be\n> configuration().pixelFormat.\n> Both PlatformFrameBufferAllocator (gralloc and\n> cros::CameraBufferManager) demands android format.\n> Further, pixelFormat resolved from android format is dependent on the\n> backend implementation.\n> Therefore it is probably correct to lookup a proper android format\n> from pixelFormat.\n> But there is no such API in gralloc API and cros::CameraBufferManager.\n\nI understand that the gralloc and cros::CameraBufferManager APIs are\nbased on Android pixel formats and not on 4CCs. My concern is that the\nCameraCapabilities::toAndroidFormat() function is meant to be generic,\nbut doesn't actually guarantee that\n\n\ttoAndroidFormat(toPixelFormat(format)) == format\n\nIt's probably fine as-is for the specific use case of patch 2/2, but I\nthink it could cause issues if we start using it elsewhere.\n\nIf my understanding is correct, we can't really use\ncamera3Stream_->format in a generic way for internal streams. Because\ninternal streams are internal, they're not created directly from an\nAndroid pixel format. At this time, we want to use post-processing to\nproduce\n\n- JPEG streams, which we hardcode being produced from an internal NV12\n stream in case no compatible non-compressed streams is also requested.\n\n- YUV streams, produced by scalign through libyuv, but without format\n conversion.\n\nThere may be other use cases in the future that would involve format\nconversion, but I wonder if it wouldn't be best at this point to use the\nAndroid pixel format directly in both cases instead of converting from a\nlibcamera pixel format.\n\n> > > +}\n> > > +\n> > > std::unique_ptr CameraCapabilities::requestTemplateManual() const\n> > > {\n> > > if (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {\n> > > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > > index 2cf97ae8..df6361f1 100644\n> > > --- a/src/android/camera_capabilities.h\n> > > +++ b/src/android/camera_capabilities.h\n> > > @@ -30,6 +30,7 @@ public:\n> > >\n> > > CameraMetadata *staticMetadata() const { return staticMetadata_.get(); }\n> > > libcamera::PixelFormat toPixelFormat(int format) const;\n> > > + int toAndroidFormat(libcamera::PixelFormat format) const;\n> > > unsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }\n> > >\n> > > std::unique_ptr requestTemplateManual() const;\n> > > @@ -77,6 +78,7 @@ private:\n> > >\n> > > std::vector streamConfigurations_;\n> > > std::map formatsMap_;\n> > > + std::map revFormatsMap_;\n> > > std::unique_ptr staticMetadata_;\n> > > unsigned int maxJpegBufferSize_;\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 2B139BF415\n\tfor ;\n\tMon, 6 Dec 2021 11:10:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 28A846086C;\n\tMon, 6 Dec 2021 12:10:00 +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 4F1CE60725\n\tfor ;\n\tMon, 6 Dec 2021 12:09: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 A773AEE;\n\tMon, 6 Dec 2021 12:09:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Uji5Ei7v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638788998;\n\tbh=sdp/IefI/u1xlhOBKHYy5DFqqPZgXFgU0buc8fxZlJw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Uji5Ei7vRA2MlMmaydSf99Enn1NE46urUMA59Lz62rG8XUgTWNeOy8uLsOaC8vm7W\n\tPSE1gA1OfoD4F9V8oNMTgcOawf4imSHWMIf37n9xwrze76f6njl0U1VXaascSZmlmN\n\twF8q0tIyH3RqN0G+oIRtB+a1W2WSaQ3j74zN/Vxc=","Date":"Mon, 6 Dec 2021 13:09:31 +0200","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20211130124428.2163669-1-hiroh@chromium.org>\n\t<20211130124428.2163669-2-hiroh@chromium.org>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","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":21960,"web_url":"https://patchwork.libcamera.org/comment/21960/","msgid":"","date":"2022-01-06T05:28:59","subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Thanks Laurent for the comment.\n\nIt sounds a good solution so far.\nI also noticed Jacopo has merged a patch with your idea.\nThanks Jacopo.\n\nBest Regards,\n-Hiro\n\nOn Mon, Dec 6, 2021 at 8:09 PM Laurent Pinchart\n wrote:\n>\n> Hi Hiro,\n>\n> On Wed, Dec 01, 2021 at 04:28:07PM +0900, Hirokazu Honda wrote:\n> > On Wed, Dec 1, 2021 at 11:26 AM Laurent Pinchart wrote:\n> > > On Tue, Nov 30, 2021 at 09:44:27PM +0900, Hirokazu Honda wrote:\n> > > > This adds a function to CameraCapabilities of converting PixelFormat\n> > > > to android format.\n> > > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > src/android/camera_capabilities.cpp | 18 ++++++++++++++++++\n> > > > src/android/camera_capabilities.h | 2 ++\n> > > > 2 files changed, 20 insertions(+)\n> > > >\n> > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > > index f357902e..4c1dc87c 100644\n> > > > --- a/src/android/camera_capabilities.cpp\n> > > > +++ b/src/android/camera_capabilities.cpp\n> > > > @@ -545,6 +545,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > > > */\n> > > > if (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> > > > formatsMap_[androidFormat] = formats::MJPEG;\n> > > > + revFormatsMap_[formats::MJPEG] = androidFormat;\n> > > > LOG(HAL, Debug) << \"Mapped Android format \"\n> > > > << camera3Format.name << \" to \"\n> > > > << formats::MJPEG.toString()\n> > > > @@ -596,6 +597,7 @@ int CameraCapabilities::initializeStreamConfigurations()\n> > > > * stream configurations map, by testing the image resolutions.\n> > > > */\n> > > > formatsMap_[androidFormat] = mappedFormat;\n> > > > + revFormatsMap_[mappedFormat] = androidFormat;\n> > > > LOG(HAL, Debug) << \"Mapped Android format \"\n> > > > << camera3Format.name << \" to \"\n> > > > << mappedFormat.toString();\n> > > > @@ -1422,6 +1424,22 @@ PixelFormat CameraCapabilities::toPixelFormat(int format) const\n> > > > return it->second;\n> > > > }\n> > > >\n> > > > +/*\n> > > > + * Translate libcamera pixel format to Android format code. -1 if no mapped\n> > > > + * android format is found.\n> > > > + */\n> > > > +int CameraCapabilities::toAndroidFormat(PixelFormat format) const\n> > > > +{\n> > > > + auto it = revFormatsMap_.find(format);\n> > > > + if (it == revFormatsMap_.end()) {\n> > > > + LOG(HAL, Error) << \"Requested format \" << format.toString()\n> > > > + << \" not supported\";\n> > > > + return -1;\n> > > > + }\n> > > > +\n> > > > + return it->second;\n> > >\n> > > I think there's a conceptual problem here. While a given Android format\n> > > can always be translated to a PixelFormat, the mapping isn't 1:1. We\n> > > already map different Android formats to NV21 for instance, and I expect\n> > > this to continue as we support more formats. A reverse map is thus\n> > > ill-defined, there's no guarantee that\n> > >\n> > > toAndroidFormat(toPixelFormat(format)) == format\n> > >\n> > > which I think can lead to subtle bugs. Can't we instead, in the next\n> > > patch, get the Android format from camera3Stream_->format ?\n> >\n> > camera3Stream_->format is what Android HAL client requests.\n> > From camera3Stream_->format, we decide to request configuration to camera.\n> > Since the created buffer is the input to camera, its format must be\n> > configuration().pixelFormat.\n> > Both PlatformFrameBufferAllocator (gralloc and\n> > cros::CameraBufferManager) demands android format.\n> > Further, pixelFormat resolved from android format is dependent on the\n> > backend implementation.\n> > Therefore it is probably correct to lookup a proper android format\n> > from pixelFormat.\n> > But there is no such API in gralloc API and cros::CameraBufferManager.\n>\n> I understand that the gralloc and cros::CameraBufferManager APIs are\n> based on Android pixel formats and not on 4CCs. My concern is that the\n> CameraCapabilities::toAndroidFormat() function is meant to be generic,\n> but doesn't actually guarantee that\n>\n> toAndroidFormat(toPixelFormat(format)) == format\n>\n> It's probably fine as-is for the specific use case of patch 2/2, but I\n> think it could cause issues if we start using it elsewhere.\n>\n> If my understanding is correct, we can't really use\n> camera3Stream_->format in a generic way for internal streams. Because\n> internal streams are internal, they're not created directly from an\n> Android pixel format. At this time, we want to use post-processing to\n> produce\n>\n> - JPEG streams, which we hardcode being produced from an internal NV12\n> stream in case no compatible non-compressed streams is also requested.\n>\n> - YUV streams, produced by scalign through libyuv, but without format\n> conversion.\n>\n> There may be other use cases in the future that would involve format\n> conversion, but I wonder if it wouldn't be best at this point to use the\n> Android pixel format directly in both cases instead of converting from a\n> libcamera pixel format.\n>\n> > > > +}\n> > > > +\n> > > > std::unique_ptr CameraCapabilities::requestTemplateManual() const\n> > > > {\n> > > > if (!capabilities_.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR)) {\n> > > > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > > > index 2cf97ae8..df6361f1 100644\n> > > > --- a/src/android/camera_capabilities.h\n> > > > +++ b/src/android/camera_capabilities.h\n> > > > @@ -30,6 +30,7 @@ public:\n> > > >\n> > > > CameraMetadata *staticMetadata() const { return staticMetadata_.get(); }\n> > > > libcamera::PixelFormat toPixelFormat(int format) const;\n> > > > + int toAndroidFormat(libcamera::PixelFormat format) const;\n> > > > unsigned int maxJpegBufferSize() const { return maxJpegBufferSize_; }\n> > > >\n> > > > std::unique_ptr requestTemplateManual() const;\n> > > > @@ -77,6 +78,7 @@ private:\n> > > >\n> > > > std::vector streamConfigurations_;\n> > > > std::map formatsMap_;\n> > > > + std::map revFormatsMap_;\n> > > > std::unique_ptr staticMetadata_;\n> > > > unsigned int maxJpegBufferSize_;\n> > > >\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 0E8E9BE080\n\tfor ;\n\tThu, 6 Jan 2022 05:29:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 549706091E;\n\tThu, 6 Jan 2022 06:29:11 +0100 (CET)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 255F960216\n\tfor ;\n\tThu, 6 Jan 2022 06:29:10 +0100 (CET)","by mail-ed1-x536.google.com with SMTP id u25so5280306edf.1\n\tfor ;\n\tWed, 05 Jan 2022 21:29:10 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"MQDvp+Wi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=6sHfkCbSrJAsdW70edEWE8GoOTMDtuqgSHz1R14BFhQ=;\n\tb=MQDvp+Wij0cd2Fu8szsg4SOxAgnf0TbaoWm+PH409/5ANvHEYpz+0OHqMm6cKHrcgG\n\tO8zbRxS2feNnAzVSJyVns+fS+ZTRy9w17S2RfIvH4K4F/4gByFhzStEfX7MpHmHvqMjN\n\tP90t/NxZK0FOej9iAHYRmJZyGRkLNlchSGAys=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=6sHfkCbSrJAsdW70edEWE8GoOTMDtuqgSHz1R14BFhQ=;\n\tb=ATXR5h2dQOWsSBWCOqZNsovp4loGxcHjJCm/dWTcZYgEXYHtRLM9sI9XFYvYPyjCJ3\n\tbqYukwGidjUGemCm3q0K83Xs6eNaIY6hWnQkXKdsuoq9lekQbNmAQovTzgFAb1n7L81T\n\teYGpJwORFKwocMh57YojRdPmw9+pl2m42RrPVxjkT5RLWJgeUMO6ZvduNJyEXs8C3rCZ\n\tDqcvaePx719Tuys2muu3TUFbm+3v89I+sw6siI8+YssYYSFduVGksHY68hwjywS27aRI\n\t5Qx8cWXHZ4ZOAeo/dg991Co+jYYgpfrBBoafPM7P8YmpDJr8yiyIWyaDOJScrdT1NZWx\n\tpyAQ==","X-Gm-Message-State":"AOAM530+vftD4dQCkHXPpkxPz1nuhLjjM/tf9vHFqINEprcX77WXpbE9\n\tePGUHCI+vfR/jCgaA2iMrJqdG2/Tfob0PhrvQhV0Hjr/0po=","X-Google-Smtp-Source":"ABdhPJycitRAWOBp4SKogwsc+zbUFoLl2OVl455jOXhguCAfskyfaJVwOJsl2Qo7xzmah2MFzt1ZYGyDHtDDToNKxd4=","X-Received":"by 2002:a05:6402:5202:: with SMTP id\n\ts2mr49421849edd.206.1641446949654; \n\tWed, 05 Jan 2022 21:29:09 -0800 (PST)","MIME-Version":"1.0","References":"<20211130124428.2163669-1-hiroh@chromium.org>\n\t<20211130124428.2163669-2-hiroh@chromium.org>\n\t\n\t\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Thu, 6 Jan 2022 14:28:59 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/2] android: camera_capabilities: Add\n\tfunction to convert PixelFormat to android format","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\" "}}]