[{"id":12251,"web_url":"https://patchwork.libcamera.org/comment/12251/","msgid":"<9a58dc7c-17df-f0e7-4abb-9c229f692c8c@ideasonboard.com>","date":"2020-09-02T12:55:49","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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> When producing the list of image resolution to claim as supported by the\n> camera HAL, the JPEG stream was assumed to be 'always valid' as, at the\n> time, there was no JPEG support in place at all.\n> \n> With the introduction of support for JPEG compression, reporting\n> non-valid sizes as supported obviously causes troubles.\n> \n> In order to avoid reporting non-supported resolutions as supported,\n> produce the list of available JPEG sizes by using the ones supported\n> by the YCbCr_420_888 format, from which the JPEG stream is encoded.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 43 +++++++++++++++++++++++------------\n> 1 file changed, 28 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index ad0d7fd15d90..8a8072123961 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -363,17 +363,27 @@ int CameraDevice::initializeStreamConfigurations()\n> \t\tconst std::vector &libcameraFormats =\n> \t\t\tcamera3Format.libcameraFormats;\n> \n> +\t\t/*\n> +\t\t * Fixed format mapping for JPEG.\n> +\t\t *\n> +\t\t * The list of supported JPEG resolutions is generated\n> +\t\t * from the list of resolutions supported by\n> +\t\t * HAL_PIXEL_FORMAT_YCbCr_420_888 from which JPEG is produced.\n> +\t\t *\n> +\t\t * \\todo Wire the JPEG encoder interface to query the list\n> +\t\t * of supported resolutions.\n\nAs we require NV12 for android, I think this is fine.\nThe encoder 'could' encode other formats, but it only needs to encode\nNV12 - as there must always be a frame of that format right?\n\n\n(I envisage we'll have to add a pixel-convertor to take YUYV webcams to\nNV12, so even then, we'll still have an NV12 frame to encode).\n\n\n> +\t\t */\n> +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> +\t\t\tformatsMap_[androidFormat] = formats::MJPEG;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\t/*\n> \t\t * Test the libcamera formats that can produce images\n> \t\t * compatible with the format defined by Android.\n> \t\t */\n> \t\tPixelFormat mappedFormat;\n> \t\tfor (const PixelFormat &pixelFormat : libcameraFormats) {\n> -\t\t\t/* \\todo Fixed mapping for JPEG. */\n> -\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> -\t\t\t\tmappedFormat = formats::MJPEG;\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> \n> \t\t\t/*\n> \t\t\t * The stream configuration size can be adjusted,\n> @@ -416,19 +426,22 @@ int CameraDevice::initializeStreamConfigurations()\n> \t\t\tcfg.size = res;\n> \n> \t\t\tCameraConfiguration::Status status = cameraConfig->validate();\n> -\t\t\t/*\n> -\t\t\t * Unconditionally report we can produce JPEG.\n> -\t\t\t *\n> -\t\t\t * \\todo The JPEG stream will be implemented as an\n> -\t\t\t * HAL-only stream, but some cameras can produce it\n> -\t\t\t * directly. As of now, claim support for JPEG without\n> -\t\t\t * inspecting where the JPEG stream is produced.\n> -\t\t\t */\n> -\t\t\tif (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> -\t\t\t status != CameraConfiguration::Valid)\n> +\t\t\tif (status != CameraConfiguration::Valid)\n> \t\t\t\tcontinue;\n> \n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> +\n> +\t\t\t/*\n> +\t\t\t * If the format is HAL_PIXEL_FORMAT_YCbCr_420_888\n> +\t\t\t * from which JPEG is produced, add an entry for\n> +\t\t\t * the JPEG stream.\n> +\t\t\t *\n> +\t\t\t * \\todo Wire the JPEG encoder to query the supported\n> +\t\t\t * sizes provided a list of formats it can encode.\n\nI don't understand this comment.\n\nWhat do you need from the JPEG encoder?\n\nThe supported sizes are whatever it's given.... It shouldn't change the\nsize.\n\n> +\t\t\t */\n> +\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888)\n> +\t\t\t\tstreamConfigurations_.push_back(\n> +\t\t\t\t\t{ res, HAL_PIXEL_FORMAT_BLOB });\n> \t\t}\n> \t}\n> \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 33078BF019\n\tfor ;\n\tWed, 2 Sep 2020 12:55:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B604262984;\n\tWed, 2 Sep 2020 14:55: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 6B64F60374\n\tfor ;\n\tWed, 2 Sep 2020 14:55: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 AFC549CC;\n\tWed, 2 Sep 2020 14:55:52 +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=\"fumrP7R2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599051353;\n\tbh=Fb2HggdZKyHowIpVK/cVdlhokz7QoSkNGNoO5DxsPwU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=fumrP7R2B/Ecyuig2z41ivLDgwIVC+X+laXVzW14WDLghCgR9FFR6u3VKwPw4O93Z\n\tG2Y9IrlLLMCgg8aiGBbqzWbqz0NIrpofBMKU2i1vMncnkAQR67jQy8E3x0lCBQH+XL\n\tX88uhAJSd5nGwnwJtn3dYQ5buiJJIXM21qrRcItE=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-3-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":"<9a58dc7c-17df-f0e7-4abb-9c229f692c8c@ideasonboard.com>","Date":"Wed, 2 Sep 2020 13:55:49 +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-3-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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":12255,"web_url":"https://patchwork.libcamera.org/comment/12255/","msgid":"<20200902131535.ubvc6rrngj4wy4pj@uno.localdomain>","date":"2020-09-02T13:15:35","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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 01:55:49PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 02/09/2020 11:47, Jacopo Mondi wrote:\n> > When producing the list of image resolution to claim as supported by the\n> > camera HAL, the JPEG stream was assumed to be 'always valid' as, at the\n> > time, there was no JPEG support in place at all.\n> >\n> > With the introduction of support for JPEG compression, reporting\n> > non-valid sizes as supported obviously causes troubles.\n> >\n> > In order to avoid reporting non-supported resolutions as supported,\n> > produce the list of available JPEG sizes by using the ones supported\n> > by the YCbCr_420_888 format, from which the JPEG stream is encoded.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 43 +++++++++++++++++++++++------------\n> > 1 file changed, 28 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index ad0d7fd15d90..8a8072123961 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -363,17 +363,27 @@ int CameraDevice::initializeStreamConfigurations()\n> > \t\tconst std::vector &libcameraFormats =\n> > \t\t\tcamera3Format.libcameraFormats;\n> >\n> > +\t\t/*\n> > +\t\t * Fixed format mapping for JPEG.\n> > +\t\t *\n> > +\t\t * The list of supported JPEG resolutions is generated\n> > +\t\t * from the list of resolutions supported by\n> > +\t\t * HAL_PIXEL_FORMAT_YCbCr_420_888 from which JPEG is produced.\n> > +\t\t *\n> > +\t\t * \\todo Wire the JPEG encoder interface to query the list\n> > +\t\t * of supported resolutions.\n>\n> As we require NV12 for android, I think this is fine.\n> The encoder 'could' encode other formats, but it only needs to encode\n> NV12 - as there must always be a frame of that format right?\n>\n>\n> (I envisage we'll have to add a pixel-convertor to take YUYV webcams to\n> NV12, so even then, we'll still have an NV12 frame to encode).\n>\n>\n> > +\t\t */\n> > +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> > +\t\t\tformatsMap_[androidFormat] = formats::MJPEG;\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > \t\t/*\n> > \t\t * Test the libcamera formats that can produce images\n> > \t\t * compatible with the format defined by Android.\n> > \t\t */\n> > \t\tPixelFormat mappedFormat;\n> > \t\tfor (const PixelFormat &pixelFormat : libcameraFormats) {\n> > -\t\t\t/* \\todo Fixed mapping for JPEG. */\n> > -\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n> > -\t\t\t\tmappedFormat = formats::MJPEG;\n> > -\t\t\t\tbreak;\n> > -\t\t\t}\n> >\n> > \t\t\t/*\n> > \t\t\t * The stream configuration size can be adjusted,\n> > @@ -416,19 +426,22 @@ int CameraDevice::initializeStreamConfigurations()\n> > \t\t\tcfg.size = res;\n> >\n> > \t\t\tCameraConfiguration::Status status = cameraConfig->validate();\n> > -\t\t\t/*\n> > -\t\t\t * Unconditionally report we can produce JPEG.\n> > -\t\t\t *\n> > -\t\t\t * \\todo The JPEG stream will be implemented as an\n> > -\t\t\t * HAL-only stream, but some cameras can produce it\n> > -\t\t\t * directly. As of now, claim support for JPEG without\n> > -\t\t\t * inspecting where the JPEG stream is produced.\n> > -\t\t\t */\n> > -\t\t\tif (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> > -\t\t\t status != CameraConfiguration::Valid)\n> > +\t\t\tif (status != CameraConfiguration::Valid)\n> > \t\t\t\tcontinue;\n> >\n> > \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> > +\n> > +\t\t\t/*\n> > +\t\t\t * If the format is HAL_PIXEL_FORMAT_YCbCr_420_888\n> > +\t\t\t * from which JPEG is produced, add an entry for\n> > +\t\t\t * the JPEG stream.\n> > +\t\t\t *\n> > +\t\t\t * \\todo Wire the JPEG encoder to query the supported\n> > +\t\t\t * sizes provided a list of formats it can encode.\n>\n> I don't understand this comment.\n>\n> What do you need from the JPEG encoder?\n>\n> The supported sizes are whatever it's given.... It shouldn't change the\n> size.\n\nCurrently we create the encoder at configureStream() time, as we only\nhave one. Going forward I suppose we'll have multiple classes\nimplementing the (currently minimal) Encoder interface, as on some\nplatforms we could have some HW accelerated implementations.\n\nI also assumed other implementation might have requirements in terms\nof alignment, maximum and minimum supported sizes and so on, and\nsupported formats the JPEG images can be produced from.\n\nThe idea here is to be able to instantiate Encoders early, or at least\nidentify the one in use earlier, and query it to learn about the above\nmentioned constraints, which should be reflected in the available\nformats reported to the framework.\n\nDoes this make any sense to you ?\n\nThanks\n j\n\n>\n> > +\t\t\t */\n> > +\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888)\n> > +\t\t\t\tstreamConfigurations_.push_back(\n> > +\t\t\t\t\t{ res, HAL_PIXEL_FORMAT_BLOB });\n> > \t\t}\n> > \t}\n> >\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 ACEA0BE174\n\tfor ;\n\tWed, 2 Sep 2020 13:11:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BC9C62984;\n\tWed, 2 Sep 2020 15:11:51 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 84A5060374\n\tfor ;\n\tWed, 2 Sep 2020 15:11:50 +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 relay2-d.mail.gandi.net (Postfix) with ESMTPSA id A05E44000E;\n\tWed, 2 Sep 2020 13:11:49 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 2 Sep 2020 15:15:35 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20200902131535.ubvc6rrngj4wy4pj@uno.localdomain>","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-3-jacopo@jmondi.org>\n\t<9a58dc7c-17df-f0e7-4abb-9c229f692c8c@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<9a58dc7c-17df-f0e7-4abb-9c229f692c8c@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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":12257,"web_url":"https://patchwork.libcamera.org/comment/12257/","msgid":"<619f3baf-1868-c73b-fb1b-3a7841d03f34@ideasonboard.com>","date":"2020-09-02T13:18:34","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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 14:15, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Wed, Sep 02, 2020 at 01:55:49PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 02/09/2020 11:47, Jacopo Mondi wrote:\n>>> When producing the list of image resolution to claim as supported by the\n>>> camera HAL, the JPEG stream was assumed to be 'always valid' as, at the\n>>> time, there was no JPEG support in place at all.\n>>>\n>>> With the introduction of support for JPEG compression, reporting\n>>> non-valid sizes as supported obviously causes troubles.\n>>>\n>>> In order to avoid reporting non-supported resolutions as supported,\n>>> produce the list of available JPEG sizes by using the ones supported\n>>> by the YCbCr_420_888 format, from which the JPEG stream is encoded.\n>>>\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/android/camera_device.cpp | 43 +++++++++++++++++++++++------------\n>>> 1 file changed, 28 insertions(+), 15 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index ad0d7fd15d90..8a8072123961 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -363,17 +363,27 @@ int CameraDevice::initializeStreamConfigurations()\n>>> \t\tconst std::vector &libcameraFormats =\n>>> \t\t\tcamera3Format.libcameraFormats;\n>>>\n>>> +\t\t/*\n>>> +\t\t * Fixed format mapping for JPEG.\n>>> +\t\t *\n>>> +\t\t * The list of supported JPEG resolutions is generated\n>>> +\t\t * from the list of resolutions supported by\n>>> +\t\t * HAL_PIXEL_FORMAT_YCbCr_420_888 from which JPEG is produced.\n>>> +\t\t *\n>>> +\t\t * \\todo Wire the JPEG encoder interface to query the list\n>>> +\t\t * of supported resolutions.\n>>\n>> As we require NV12 for android, I think this is fine.\n>> The encoder 'could' encode other formats, but it only needs to encode\n>> NV12 - as there must always be a frame of that format right?\n>>\n>>\n>> (I envisage we'll have to add a pixel-convertor to take YUYV webcams to\n>> NV12, so even then, we'll still have an NV12 frame to encode).\n>>\n>>\n>>> +\t\t */\n>>> +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n>>> +\t\t\tformatsMap_[androidFormat] = formats::MJPEG;\n>>> +\t\t\tcontinue;\n>>> +\t\t}\n>>> +\n>>> \t\t/*\n>>> \t\t * Test the libcamera formats that can produce images\n>>> \t\t * compatible with the format defined by Android.\n>>> \t\t */\n>>> \t\tPixelFormat mappedFormat;\n>>> \t\tfor (const PixelFormat &pixelFormat : libcameraFormats) {\n>>> -\t\t\t/* \\todo Fixed mapping for JPEG. */\n>>> -\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB) {\n>>> -\t\t\t\tmappedFormat = formats::MJPEG;\n>>> -\t\t\t\tbreak;\n>>> -\t\t\t}\n>>>\n>>> \t\t\t/*\n>>> \t\t\t * The stream configuration size can be adjusted,\n>>> @@ -416,19 +426,22 @@ int CameraDevice::initializeStreamConfigurations()\n>>> \t\t\tcfg.size = res;\n>>>\n>>> \t\t\tCameraConfiguration::Status status = cameraConfig->validate();\n>>> -\t\t\t/*\n>>> -\t\t\t * Unconditionally report we can produce JPEG.\n>>> -\t\t\t *\n>>> -\t\t\t * \\todo The JPEG stream will be implemented as an\n>>> -\t\t\t * HAL-only stream, but some cameras can produce it\n>>> -\t\t\t * directly. As of now, claim support for JPEG without\n>>> -\t\t\t * inspecting where the JPEG stream is produced.\n>>> -\t\t\t */\n>>> -\t\t\tif (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n>>> -\t\t\t status != CameraConfiguration::Valid)\n>>> +\t\t\tif (status != CameraConfiguration::Valid)\n>>> \t\t\t\tcontinue;\n>>>\n>>> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n>>> +\n>>> +\t\t\t/*\n>>> +\t\t\t * If the format is HAL_PIXEL_FORMAT_YCbCr_420_888\n>>> +\t\t\t * from which JPEG is produced, add an entry for\n>>> +\t\t\t * the JPEG stream.\n>>> +\t\t\t *\n>>> +\t\t\t * \\todo Wire the JPEG encoder to query the supported\n>>> +\t\t\t * sizes provided a list of formats it can encode.\n>>\n>> I don't understand this comment.\n>>\n>> What do you need from the JPEG encoder?\n>>\n>> The supported sizes are whatever it's given.... It shouldn't change the\n>> size.\n> \n> Currently we create the encoder at configureStream() time, as we only\n> have one. Going forward I suppose we'll have multiple classes\n> implementing the (currently minimal) Encoder interface, as on some\n> platforms we could have some HW accelerated implementations.\n> \n> I also assumed other implementation might have requirements in terms\n> of alignment, maximum and minimum supported sizes and so on, and\n> supported formats the JPEG images can be produced from.\n> \n> The idea here is to be able to instantiate Encoders early, or at least\n> identify the one in use earlier, and query it to learn about the above\n> mentioned constraints, which should be reflected in the available\n> formats reported to the framework.\n> \n> Does this make any sense to you ?\n\n\nYes, thanks that helps a lot - I was in the wrong mindset - of only\nthinking about the existing software jpeg encoder.\n\nSeems I missed this:\n\nReviewed-by: Kieran Bingham \n\n> \n> Thanks\n> j\n> \n>>\n>>> +\t\t\t */\n>>> +\t\t\tif (androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888)\n>>> +\t\t\t\tstreamConfigurations_.push_back(\n>>> +\t\t\t\t\t{ res, HAL_PIXEL_FORMAT_BLOB });\n>>> \t\t}\n>>> \t}\n>>>\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 09D77BF019\n\tfor ;\n\tWed, 2 Sep 2020 13:18:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99F6C628EE;\n\tWed, 2 Sep 2020 15:18:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 236ED60374\n\tfor ;\n\tWed, 2 Sep 2020 15:18:38 +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 BF7119CC;\n\tWed, 2 Sep 2020 15:18:36 +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=\"fuQjf3K7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599052717;\n\tbh=SWJgT5xBts7zIu8TAcgIlzKRUUMVaEJetwFul8CCHe8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=fuQjf3K7QMGnkDsylgQdWU4tTc7aAQHg7AnF3SrmXnMuqdfiiv3Urdke91IeCYqYL\n\t9wKLgtry+hUZY5G/OC5OkfemhZjasznkFb+iIHV8FFoAiw6CdTERvT1OzwBGWL93/j\n\tuFEYYslnh+pz8bl9LN213ajfJea7eW1wBsLpEfa4=","To":"Jacopo Mondi ","References":"<20200902104730.43451-1-jacopo@jmondi.org>\n\t<20200902104730.43451-3-jacopo@jmondi.org>\n\t<9a58dc7c-17df-f0e7-4abb-9c229f692c8c@ideasonboard.com>\n\t<20200902131535.ubvc6rrngj4wy4pj@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":"<619f3baf-1868-c73b-fb1b-3a7841d03f34@ideasonboard.com>","Date":"Wed, 2 Sep 2020 14:18:34 +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":"<20200902131535.ubvc6rrngj4wy4pj@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Generate\n\tJPEG sizes","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\" "}}]