[{"id":12256,"web_url":"https://patchwork.libcamera.org/comment/12256/","msgid":"<9b7ec30a-0f43-866e-ec38-95740960d8cd@ideasonboard.com>","date":"2020-09-02T13:15:44","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 02/09/2020 11:47, Jacopo Mondi wrote:\n> The resolutions supported for the RAW formats cannot be tested from\n> a list of known sizes like the processed ones. This is mainly due to the\n> fact RAW streams are produced by capturing frames at the CSI-2 receiver\n> output and their size corresponds to the sensor's native sizes.\n> \n> In order to obtain the RAW frame size generate a temporary\n> CameraConfiguration for the Role::StillCaptureRAW role and inspect the\n> map of StreamFormats returned by the pipeline handler.\n\nPresumably, it will also be heavily dependant upon the actual chosen\nstreams from the other requested streams ... and only a subset might\nreally be available when a full configuration is used?\n\n\n\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 21 +++++++++++++++++----\n> src/android/camera_device.h | 2 ++\n> 2 files changed, 19 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 765c3292e4f3..181fca83988d 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -314,6 +314,17 @@ std::vector CameraDevice::listProcessedResolutions(CameraConfiguration *ca\n> \treturn supportedResolutions;\n> }\n> \n> +std::vector CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat)\n> +{\n\nsame comment on the function name, that we're filtering (rather than\n'listing' valid resolutions. Though maybe this one is subtly\ndifferent... but I'd keep both namings consistent either way.\n\n> +\tstd::unique_ptr cameraConfig =\n> +\t\tcamera_->generateConfiguration({ StillCaptureRaw });\n> +\tStreamConfiguration &cfg = cameraConfig->at(0);\n> +\tconst StreamFormats &formats = cfg.formats();\n> +\tstd::vector supportedResolutions = formats.sizes(pixelFormat);\n\nI'm a bit weary of this.\n\nIf you apply this function to a UVC camera for example, it would simply\nreturn a list of sizes which represent the available frame sizes...\n\nScratch that, I see it's filtered by pixelFormat ... ok - so I guess\nthat works.\n\n\nI wonder if this couldn't also be handled by the other filter fucntion\nbut I assume not easily, as you've done this ;-)\n\nReviewed-by: Kieran Bingham \n\n\n> +\n> +\treturn supportedResolutions;\n> +}\n> +\n> /*\n> * Initialize the format conversion map to translate from Android format\n> * identifier to libcamera pixel formats and fill in the list of supported\n> @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations()\n> \t\t\t\t<< camera3Format.name << \" to: \"\n> \t\t\t\t<< mappedFormat.toString();\n> \n> +\t\tstd::vector resolutions;\n> \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> -\t\t\tcontinue;\n> +\t\t\tresolutions = listRawResolutions(mappedFormat);\n> +\t\telse\n> +\t\t\tresolutions = listProcessedResolutions(cameraConfig.get(),\n> +\t\t\t\t\t\t\t mappedFormat,\n> +\t\t\t\t\t\t\t cameraResolutions);\n> \n> -\t\tstd::vector resolutions = listProcessedResolutions(cameraConfig.get(),\n> -\t\t\t\t\t\t\t\t\t mappedFormat,\n> -\t\t\t\t\t\t\t\t\t cameraResolutions);\n> \t\tfor (const Size &res : resolutions) {\n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> \n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index 18fd5ff03cde..230e89b011e6 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -97,6 +97,8 @@ private:\n> \tlistProcessedResolutions(libcamera::CameraConfiguration *cameraConfig,\n> \t\t\t\t const libcamera::PixelFormat &pixelFormat,\n> \t\t\t\t const std::vector &resolutions);\n> +\tstd::vector\n> +\tlistRawResolutions(const libcamera::PixelFormat &pixelFormat);\n> \n> \tstd::tuple calculateStaticMetadataSize();\n> \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\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 5FDEBBF019\n\tfor ;\n\tWed, 2 Sep 2020 13:15:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C72E9629B2;\n\tWed, 2 Sep 2020 15:15:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35C0760374\n\tfor ;\n\tWed, 2 Sep 2020 15:15:53 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65C049CC;\n\tWed, 2 Sep 2020 15:15:47 +0200 (CEST)"],"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=\"UueSIQ3X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599052547;\n\tbh=ed1X3ETf73ytogbqjw+pHgIQT/yrHc5PfQWdzqtC3Y0=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=UueSIQ3XRFf4EvOGTpyHVCVyJgaQuc29Ka+epN3+KBzsXhhLtcbH1P3vaogHq9gpj\n\tBPw6hPBsXM6Y180yql7oaMmNM5zjpsak89tezpZ77idLpGnWHP1bwRst+zsaeVBsxD\n\tlkbnk8urRmkqHqvSJLxceZSxEZLrD4dwg1n1uCL4=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-6-jacopo@jmondi.org>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<9b7ec30a-0f43-866e-ec38-95740960d8cd@ideasonboard.com>","Date":"Wed, 2 Sep 2020 14:15:44 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200902104730.43451-6-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"tfiga@google.com, hiroh@google.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12262,"web_url":"https://patchwork.libcamera.org/comment/12262/","msgid":"<20200902133617.nzbgyirl5lxnrr3z@uno.localdomain>","date":"2020-09-02T13:36:17","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Wed, Sep 02, 2020 at 02:15:44PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 02/09/2020 11:47, Jacopo Mondi wrote:\n> > The resolutions supported for the RAW formats cannot be tested from\n> > a list of known sizes like the processed ones. This is mainly due to the\n> > fact RAW streams are produced by capturing frames at the CSI-2 receiver\n> > output and their size corresponds to the sensor's native sizes.\n> >\n> > In order to obtain the RAW frame size generate a temporary\n> > CameraConfiguration for the Role::StillCaptureRAW role and inspect the\n> > map of StreamFormats returned by the pipeline handler.\n>\n> Presumably, it will also be heavily dependant upon the actual chosen\n> streams from the other requested streams ... and only a subset might\n> really be available when a full configuration is used?\n\nNot sure I got you here :/\n>\n>\n>\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 21 +++++++++++++++++----\n> > src/android/camera_device.h | 2 ++\n> > 2 files changed, 19 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 765c3292e4f3..181fca83988d 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -314,6 +314,17 @@ std::vector CameraDevice::listProcessedResolutions(CameraConfiguration *ca\n> > \treturn supportedResolutions;\n> > }\n> >\n> > +std::vector CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat)\n> > +{\n>\n> same comment on the function name, that we're filtering (rather than\n> 'listing' valid resolutions. Though maybe this one is subtly\n> different... but I'd keep both namings consistent either way.\n\nWell, we're actually listing them here :D\nAnd anyway, there's not much filtering going on here, maybe a bit more\nin the corresponding non-RAW implementation as we actually test a list\nof sizes and report only the supported ones.\n\nIn any case, I would keep the naming of the two functions consistent\nwhatever we chose.\n\n>\n> > +\tstd::unique_ptr cameraConfig =\n> > +\t\tcamera_->generateConfiguration({ StillCaptureRaw });\n> > +\tStreamConfiguration &cfg = cameraConfig->at(0);\n> > +\tconst StreamFormats &formats = cfg.formats();\n> > +\tstd::vector supportedResolutions = formats.sizes(pixelFormat);\n>\n> I'm a bit weary of this.\n>\n> If you apply this function to a UVC camera for example, it would simply\n> return a list of sizes which represent the available frame sizes...\n>\n\nPardon my ignorance, does uvc support RAW ?\n\n> Scratch that, I see it's filtered by pixelFormat ... ok - so I guess\n> that works.\n>\n>\n> I wonder if this couldn't also be handled by the other filter fucntion\n> but I assume not easily, as you've done this ;-)\n\nWell, they do two different things. I actually considered doing what\nI'm doing here (building on top of StreamFormats) for non-RAW stream,\nbut it turned out to be a whack-a-mole game of finding the right Role\nto request to have the sizes for the format we are interested in. And\nthat gets pretty pipeline-implementation-dependent as there's no\nstrict rule on what formats corresponds to a role. So I might be\nlooking for supported NV12 sizes and my best bet is to ask for\nViewfinder and see what StreamFormats are returned, but it's a bet and\nsome pipeline handler might only support RGB for viewfinder and NV12\nfor StillCapture (it's an example, not sure it makes any sense) so I\ncould end up testing all roles, filtering double results, re-ordering\netc... Seems like a no-go to me.\n\nThe other way around is possible as well: list RAW sizes by using the\nsame method as it's used for non-RAW ones, with the difference that we\naccept adjusted sizes, as the pipeline handler will adjust to the\nclosest available sensor frame size for each tested image resolution.\nAgain, it seemd sub-optimal and requires filtering doubles and testing\nan un-necessary number of times, so I created two separate functions\nin the end.\n\nBe aware that in the back of my mind there's also a listJPEGSizes()\nthat queries the encoder for the reasons explained in my other reply,\nif that makes any sense :)\n\nThanks\n j\n\n\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n> > +\n> > +\treturn supportedResolutions;\n> > +}\n> > +\n> > /*\n> > * Initialize the format conversion map to translate from Android format\n> > * identifier to libcamera pixel formats and fill in the list of supported\n> > @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations()\n> > \t\t\t\t<< camera3Format.name << \" to: \"\n> > \t\t\t\t<< mappedFormat.toString();\n> >\n> > +\t\tstd::vector resolutions;\n> > \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n> > \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> > -\t\t\tcontinue;\n> > +\t\t\tresolutions = listRawResolutions(mappedFormat);\n> > +\t\telse\n> > +\t\t\tresolutions = listProcessedResolutions(cameraConfig.get(),\n> > +\t\t\t\t\t\t\t mappedFormat,\n> > +\t\t\t\t\t\t\t cameraResolutions);\n> >\n> > -\t\tstd::vector resolutions = listProcessedResolutions(cameraConfig.get(),\n> > -\t\t\t\t\t\t\t\t\t mappedFormat,\n> > -\t\t\t\t\t\t\t\t\t cameraResolutions);\n> > \t\tfor (const Size &res : resolutions) {\n> > \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> >\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index 18fd5ff03cde..230e89b011e6 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -97,6 +97,8 @@ private:\n> > \tlistProcessedResolutions(libcamera::CameraConfiguration *cameraConfig,\n> > \t\t\t\t const libcamera::PixelFormat &pixelFormat,\n> > \t\t\t\t const std::vector &resolutions);\n> > +\tstd::vector\n> > +\tlistRawResolutions(const libcamera::PixelFormat &pixelFormat);\n> >\n> > \tstd::tuple calculateStaticMetadataSize();\n> > \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 65C5ABE174\n\tfor ;\n\tWed, 2 Sep 2020 13:32:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C435A62984;\n\tWed, 2 Sep 2020 15:32:34 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CFC7060374\n\tfor ;\n\tWed, 2 Sep 2020 15:32:33 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 0A416E0010;\n\tWed, 2 Sep 2020 13:32:31 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 2 Sep 2020 15:36:17 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20200902133617.nzbgyirl5lxnrr3z@uno.localdomain>","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-6-jacopo@jmondi.org>\n\t<9b7ec30a-0f43-866e-ec38-95740960d8cd@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<9b7ec30a-0f43-866e-ec38-95740960d8cd@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","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":"tfiga@google.com, libcamera-devel@lists.libcamera.org, hiroh@google.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12271,"web_url":"https://patchwork.libcamera.org/comment/12271/","msgid":"","date":"2020-09-02T14:42:11","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 02/09/2020 14:36, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Wed, Sep 02, 2020 at 02:15:44PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 02/09/2020 11:47, Jacopo Mondi wrote:\n>>> The resolutions supported for the RAW formats cannot be tested from\n>>> a list of known sizes like the processed ones. This is mainly due to the\n>>> fact RAW streams are produced by capturing frames at the CSI-2 receiver\n>>> output and their size corresponds to the sensor's native sizes.\n>>>\n>>> In order to obtain the RAW frame size generate a temporary\n>>> CameraConfiguration for the Role::StillCaptureRAW role and inspect the\n>>> map of StreamFormats returned by the pipeline handler.\n>>\n>> Presumably, it will also be heavily dependant upon the actual chosen\n>> streams from the other requested streams ... and only a subset might\n>> really be available when a full configuration is used?\n> \n> Not sure I got you here :/\n\nI was worried about the fact that the pipeline handlers would configure\nthe sensor in a specific format to handle the other streams, which might\nnot match what android requests as the raw size ... but I don't think it\nwould happen.\n\nAsking for a RAW stream should always be the 'size' of the main image I\nexpect... (or the 'rawest' form thereof...)\n\n>>\n>>\n>>\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/android/camera_device.cpp | 21 +++++++++++++++++----\n>>> src/android/camera_device.h | 2 ++\n>>> 2 files changed, 19 insertions(+), 4 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index 765c3292e4f3..181fca83988d 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -314,6 +314,17 @@ std::vector CameraDevice::listProcessedResolutions(CameraConfiguration *ca\n>>> \treturn supportedResolutions;\n>>> }\n>>>\n>>> +std::vector CameraDevice::listRawResolutions(const libcamera::PixelFormat &pixelFormat)\n>>> +{\n>>\n>> same comment on the function name, that we're filtering (rather than\n>> 'listing' valid resolutions. Though maybe this one is subtly\n>> different... but I'd keep both namings consistent either way.\n> \n> Well, we're actually listing them here :D\n> And anyway, there's not much filtering going on here, maybe a bit more\n> in the corresponding non-RAW implementation as we actually test a list\n> of sizes and report only the supported ones.\n> \n> In any case, I would keep the naming of the two functions consistent\n> whatever we chose.\n> \n>>\n>>> +\tstd::unique_ptr cameraConfig =\n>>> +\t\tcamera_->generateConfiguration({ StillCaptureRaw });\n>>> +\tStreamConfiguration &cfg = cameraConfig->at(0);\n>>> +\tconst StreamFormats &formats = cfg.formats();\n>>> +\tstd::vector supportedResolutions = formats.sizes(pixelFormat);\n>>\n>> I'm a bit weary of this.\n>>\n>> If you apply this function to a UVC camera for example, it would simply\n>> return a list of sizes which represent the available frame sizes...\n>>\n> \n> Pardon my ignorance, does uvc support RAW ?\n\nprobably not...\n\n\n>> Scratch that, I see it's filtered by pixelFormat ... ok - so I guess\n>> that works.\n\nSorry - I left my working/thoughts in place. That sentence voided the\nprevious one.\n\n\n>>\n>>\n>> I wonder if this couldn't also be handled by the other filter fucntion\n>> but I assume not easily, as you've done this ;-)\n> \n> Well, they do two different things. I actually considered doing what\n> I'm doing here (building on top of StreamFormats) for non-RAW stream,\n> but it turned out to be a whack-a-mole game of finding the right Role\n> to request to have the sizes for the format we are interested in. And\n> that gets pretty pipeline-implementation-dependent as there's no\n> strict rule on what formats corresponds to a role. So I might be\n> looking for supported NV12 sizes and my best bet is to ask for\n> Viewfinder and see what StreamFormats are returned, but it's a bet and\n> some pipeline handler might only support RGB for viewfinder and NV12\n> for StillCapture (it's an example, not sure it makes any sense) so I\n> could end up testing all roles, filtering double results, re-ordering\n> etc... Seems like a no-go to me.\n> \n> The other way around is possible as well: list RAW sizes by using the\n> same method as it's used for non-RAW ones, with the difference that we\n> accept adjusted sizes, as the pipeline handler will adjust to the\n> closest available sensor frame size for each tested image resolution.\n> Again, it seemd sub-optimal and requires filtering doubles and testing\n> an un-necessary number of times, so I created two separate functions\n> in the end.\n> \n> Be aware that in the back of my mind there's also a listJPEGSizes()\n> that queries the encoder for the reasons explained in my other reply,\n> if that makes any sense :)\n\n\nWell, we'll see what happens when we get our next encoder ...\n\n\n> Thanks\n> j\n> \n> \n>>\n>> Reviewed-by: Kieran Bingham \n>>\n>>\n>>> +\n>>> +\treturn supportedResolutions;\n>>> +}\n>>> +\n>>> /*\n>>> * Initialize the format conversion map to translate from Android format\n>>> * identifier to libcamera pixel formats and fill in the list of supported\n>>> @@ -466,13 +477,15 @@ int CameraDevice::initializeStreamConfigurations()\n>>> \t\t\t\t<< camera3Format.name << \" to: \"\n>>> \t\t\t\t<< mappedFormat.toString();\n>>>\n>>> +\t\tstd::vector resolutions;\n>>> \t\tconst PixelFormatInfo &info = PixelFormatInfo::info(mappedFormat);\n>>> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n>>> -\t\t\tcontinue;\n>>> +\t\t\tresolutions = listRawResolutions(mappedFormat);\n>>> +\t\telse\n>>> +\t\t\tresolutions = listProcessedResolutions(cameraConfig.get(),\n>>> +\t\t\t\t\t\t\t mappedFormat,\n>>> +\t\t\t\t\t\t\t cameraResolutions);\n>>>\n>>> -\t\tstd::vector resolutions = listProcessedResolutions(cameraConfig.get(),\n>>> -\t\t\t\t\t\t\t\t\t mappedFormat,\n>>> -\t\t\t\t\t\t\t\t\t cameraResolutions);\n>>> \t\tfor (const Size &res : resolutions) {\n>>> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n>>>\n>>> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n>>> index 18fd5ff03cde..230e89b011e6 100644\n>>> --- a/src/android/camera_device.h\n>>> +++ b/src/android/camera_device.h\n>>> @@ -97,6 +97,8 @@ private:\n>>> \tlistProcessedResolutions(libcamera::CameraConfiguration *cameraConfig,\n>>> \t\t\t\t const libcamera::PixelFormat &pixelFormat,\n>>> \t\t\t\t const std::vector &resolutions);\n>>> +\tstd::vector\n>>> +\tlistRawResolutions(const libcamera::PixelFormat &pixelFormat);\n>>>\n>>> \tstd::tuple calculateStaticMetadataSize();\n>>> \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","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 BAC8FBF019\n\tfor ;\n\tWed, 2 Sep 2020 14:42:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 538E762984;\n\tWed, 2 Sep 2020 16:42:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC15260374\n\tfor ;\n\tWed, 2 Sep 2020 16:42:16 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A7E656E;\n\tWed, 2 Sep 2020 16:42:14 +0200 (CEST)"],"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=\"J+oSrgIY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599057734;\n\tbh=J8eARBa2vt3kBVzSVvQb6lR+IMv/270ykeM/i/FL0r4=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=J+oSrgIYXZ94U99Kwwl7VFCywk7peebCIRdkfkqT43TOsfyPjSD7JVU2i9ZTrkbBg\n\tAnJ6/QVtFlQIVtrWFWEhARM2m6FyJcbXxoMNHtW0Kjxb3LXF7+kAAaSljWiStGSeVe\n\tO7jnQ0C+4FDpEUZqhLcGYCMUpvb8XOGRCugGe+7Y=","To":"Jacopo Mondi ","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-6-jacopo@jmondi.org>\n\t<9b7ec30a-0f43-866e-ec38-95740960d8cd@ideasonboard.com>\n\t<20200902133617.nzbgyirl5lxnrr3z@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 2 Sep 2020 15:42:11 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200902133617.nzbgyirl5lxnrr3z@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: List RAW\n\tresolutions","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"tfiga@google.com, libcamera-devel@lists.libcamera.org, hiroh@google.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]