[{"id":11894,"web_url":"https://patchwork.libcamera.org/comment/11894/","msgid":"<20200805165341.GK6751@pendragon.ideasonboard.com>","date":"2020-08-05T16:53:41","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 05, 2020 at 05:37:45PM +0200, 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> \n> Patch to be applied on top of Kieran's JPEG work.\n> ---\n> src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n> 1 file changed, 27 insertions(+), 11 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index ec8ca934842a..6a9a038a2b53 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n> \t\t */\n> \t\tformatsMap_[androidFormat] = mappedFormat;\n> \n> +\t\t/*\n> +\t\t * Stop here for JPEG streams: the JPEG supported sizes will\n> +\t\t * be tested later using the here recorded non-blob stream sizes.\n> +\t\t */\n> +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n> +\t\t\tcontinue;\n> +\n> \t\tfor (const Size &res : cameraResolutions) {\n> \t\t\tcfg.pixelFormat = mappedFormat;\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 (cameraConfig->validate() != CameraConfiguration::Valid)\n> \t\t\t\tcontinue;\n> \n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> \t\t}\n> \t}\n> \n> +\t/*\n> +\t * Insert the JPEG sizes by using the ones recorded for YUV streams\n> +\t * from which JPEG is produced.\n> +\t */\n> +\tstd::vector jpegConfigurations;\n> +\tjpegConfigurations.reserve(cameraResolutions.size());\n> +\n> +\tfor (const auto &config : streamConfigurations_) {\n> +\t\t/* \\todo JPEG can be produced from other formats too! */\n\nAnother todo item, the android.scaler.availableStreamConfigurations\ndocumentation lists required resolutions (see\nhttps://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\nwhich very annoyingly googlesource.com can't display as html in a web browser).\n\nJPEG \tandroid.sensor.info.activeArraySize \tAny \t\nJPEG \t1920x1080 (1080p) \t\t\tAny \tif 1080p <= activeArraySize\nJPEG \t1280x720 (720) \t\t\t\tAny \tif 720p <= activeArraySize\nJPEG \t640x480 (480p) \t\t\t\tAny \tif 480p <= activeArraySize\nJPEG \t320x240 (240p) \t\t\t\tAny \tif 240p <= activeArraySize\n\n> +\t\tif (config.androidFormat != HAL_PIXEL_FORMAT_YCbCr_420_888)\n> +\t\t\tcontinue;\n> +\n> +\t\tjpegConfigurations.push_back({ config.resolution,\n> +\t\t\t\t\t HAL_PIXEL_FORMAT_BLOB });\n> +\t}\n> +\n> +\tfor (auto const jpegConfig : jpegConfigurations)\n> +\t\tstreamConfigurations_.push_back(jpegConfig);\n> +\n> \tLOG(HAL, Debug) << \"Collected stream configuration map: \";\n> \tfor (const auto &entry : streamConfigurations_)\n> \t\tLOG(HAL, Debug) << \"{ \" << entry.resolution.toString() << \" - \"","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 E7002BD87A\n\tfor ;\n\tWed, 5 Aug 2020 16:53:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BF6B6059F;\n\tWed, 5 Aug 2020 18:53:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E2D960599\n\tfor ;\n\tWed, 5 Aug 2020 18:53:53 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D72EC2C0;\n\tWed, 5 Aug 2020 18:53: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=\"gk7YsAfZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596646433;\n\tbh=yPiTJ723DiPQag/1A6OnGRAbT9eLEP9lZ/56bImr/uk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gk7YsAfZin16QE0wJyN2FZLXjYy2JqTqgzMW1kTppU5nVw93+UjJxPHZjV9/Tu9cE\n\tfy3CM1lpcMi0yaTJMsJShRnXfk72HIeIzfevnPtfTQ3NxiIAtOUVhsHemtZsFuwBIN\n\tXy86YdzJEZfaUvZV7DtjckvutiW2XPm/D/+BfPWc=","Date":"Wed, 5 Aug 2020 19:53:41 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200805165341.GK6751@pendragon.ideasonboard.com>","References":"<20200805153745.30378-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805153745.30378-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11900,"web_url":"https://patchwork.libcamera.org/comment/11900/","msgid":"<20200805213355.6llvit6znf5uuozc@uno.localdomain>","date":"2020-08-05T21:33:55","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)\n +Kieran for the Compressor interface part\n\nOn Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, Aug 05, 2020 at 05:37:45PM +0200, 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> >\n> > Patch to be applied on top of Kieran's JPEG work.\n> > ---\n> > src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n> > 1 file changed, 27 insertions(+), 11 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index ec8ca934842a..6a9a038a2b53 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n> > \t\t */\n> > \t\tformatsMap_[androidFormat] = mappedFormat;\n> >\n> > +\t\t/*\n> > +\t\t * Stop here for JPEG streams: the JPEG supported sizes will\n> > +\t\t * be tested later using the here recorded non-blob stream sizes.\n> > +\t\t */\n> > +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n> > +\t\t\tcontinue;\n> > +\n> > \t\tfor (const Size &res : cameraResolutions) {\n> > \t\t\tcfg.pixelFormat = mappedFormat;\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 (cameraConfig->validate() != CameraConfiguration::Valid)\n> > \t\t\t\tcontinue;\n> >\n> > \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> > \t\t}\n> > \t}\n> >\n> > +\t/*\n> > +\t * Insert the JPEG sizes by using the ones recorded for YUV streams\n> > +\t * from which JPEG is produced.\n> > +\t */\n> > +\tstd::vector jpegConfigurations;\n> > +\tjpegConfigurations.reserve(cameraResolutions.size());\n> > +\n> > +\tfor (const auto &config : streamConfigurations_) {\n> > +\t\t/* \\todo JPEG can be produced from other formats too! */\n>\n> Another todo item, the android.scaler.availableStreamConfigurations\n> documentation lists required resolutions (see\n> https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\n> which very annoyingly googlesource.com can't display as html in a web browser).\n>\n> JPEG \tandroid.sensor.info.activeArraySize \tAny\n> JPEG \t1920x1080 (1080p) \t\t\tAny \tif 1080p <= activeArraySize\n> JPEG \t1280x720 (720) \t\t\t\tAny \tif 720p <= activeArraySize\n> JPEG \t640x480 (480p) \t\t\t\tAny \tif 480p <= activeArraySize\n> JPEG \t320x240 (240p) \t\t\t\tAny \tif 240p <= activeArraySize\n>\n\nThose are already mandatory to be supported if I'm not mistaken.\n\nwe feed cameraResolutions for all the non-jpeg formats here:\n\n\t\tfor (const Size &res : cameraResolutions) {\n\t\t\tcfg.pixelFormat = mappedFormat;\n\t\t\tcfg.size = res;\n\n\t\t\tif (cameraConfig->validate() != CameraConfiguration::Valid)\n\t\t\t\tcontinue;\n\n\t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n\t\t}\n\nThen we use the sizes associated with the mandatory\nHAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed\nsomething on this part ?\n\nLooking at the code now, we could create an initJpeg() functions, as\nall the jpeg streams are basically just ignored now and\ninitialized at the end of the function.\n\nMaybe it's really time to break this file apart, it's already quite\nbig and terse. What if we delegate the jpeg initalization part to the\nCompressor interface (looking at it now, we could have gone for\nEncoder as a name maybe).\n\nAs of now it would basically require two\nmore methods: a way to get the a metadata pack with the values of\nANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE\ninitalized, and a vector of supported sizes given the vector of the\nones so far collected.\n\nThen we'll probably need a factory to create the right Compressor, but\nthat's for later and we should pobably first discuss how to decide\n\"which\" encoder to initialize before getting to the \"how\".\n\nIn example, how does it work today in CrOS and what are the\nrequirements going forward ?\n\nThanks\n j\n\n> > +\t\tif (config.androidFormat != HAL_PIXEL_FORMAT_YCbCr_420_888)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tjpegConfigurations.push_back({ config.resolution,\n> > +\t\t\t\t\t HAL_PIXEL_FORMAT_BLOB });\n> > +\t}\n> > +\n> > +\tfor (auto const jpegConfig : jpegConfigurations)\n> > +\t\tstreamConfigurations_.push_back(jpegConfig);\n> > +\n> > \tLOG(HAL, Debug) << \"Collected stream configuration map: \";\n> > \tfor (const auto &entry : streamConfigurations_)\n> > \t\tLOG(HAL, Debug) << \"{ \" << entry.resolution.toString() << \" - \"\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 DCD2DBD87A\n\tfor ;\n\tWed, 5 Aug 2020 21:30:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A70C6071B;\n\tWed, 5 Aug 2020 23:30:18 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2B1C36039D\n\tfor ;\n\tWed, 5 Aug 2020 23:30:17 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 7E4EEFF806;\n\tWed, 5 Aug 2020 21:30:15 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 5 Aug 2020 23:33:55 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200805213355.6llvit6znf5uuozc@uno.localdomain>","References":"<20200805153745.30378-1-jacopo@jmondi.org>\n\t<20200805165341.GK6751@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200805165341.GK6751@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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, \n\tkieran bingham ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11917,"web_url":"https://patchwork.libcamera.org/comment/11917/","msgid":"","date":"2020-08-06T11:04:47","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":48,"url":"https://patchwork.libcamera.org/api/people/48/","name":"Tomasz Figa","email":"tfiga@google.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 5, 2020 at 11:30 PM Jacopo Mondi wrote:\n>\n> Hi Laurent,\n> +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)\n> +Kieran for the Compressor interface part\n>\n> On Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > On Wed, Aug 05, 2020 at 05:37:45PM +0200, 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> > >\n> > > Patch to be applied on top of Kieran's JPEG work.\n> > > ---\n> > > src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n> > > 1 file changed, 27 insertions(+), 11 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index ec8ca934842a..6a9a038a2b53 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n> > > */\n> > > formatsMap_[androidFormat] = mappedFormat;\n> > >\n> > > + /*\n> > > + * Stop here for JPEG streams: the JPEG supported sizes will\n> > > + * be tested later using the here recorded non-blob stream sizes.\n> > > + */\n> > > + if (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n> > > + continue;\n> > > +\n> > > for (const Size &res : cameraResolutions) {\n> > > cfg.pixelFormat = mappedFormat;\n> > > cfg.size = res;\n> > >\n> > > - CameraConfiguration::Status status = cameraConfig->validate();\n> > > - /*\n> > > - * Unconditionally report we can produce JPEG.\n> > > - *\n> > > - * \\todo The JPEG stream will be implemented as an\n> > > - * HAL-only stream, but some cameras can produce it\n> > > - * directly. As of now, claim support for JPEG without\n> > > - * inspecting where the JPEG stream is produced.\n> > > - */\n> > > - if (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> > > - status != CameraConfiguration::Valid)\n> > > + if (cameraConfig->validate() != CameraConfiguration::Valid)\n> > > continue;\n> > >\n> > > streamConfigurations_.push_back({ res, androidFormat });\n> > > }\n> > > }\n> > >\n> > > + /*\n> > > + * Insert the JPEG sizes by using the ones recorded for YUV streams\n> > > + * from which JPEG is produced.\n> > > + */\n> > > + std::vector jpegConfigurations;\n> > > + jpegConfigurations.reserve(cameraResolutions.size());\n> > > +\n> > > + for (const auto &config : streamConfigurations_) {\n> > > + /* \\todo JPEG can be produced from other formats too! */\n> >\n> > Another todo item, the android.scaler.availableStreamConfigurations\n> > documentation lists required resolutions (see\n> > https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\n> > which very annoyingly googlesource.com can't display as html in a web browser).\n> >\n> > JPEG android.sensor.info.activeArraySize Any\n> > JPEG 1920x1080 (1080p) Any if 1080p <= activeArraySize\n> > JPEG 1280x720 (720) Any if 720p <= activeArraySize\n> > JPEG 640x480 (480p) Any if 480p <= activeArraySize\n> > JPEG 320x240 (240p) Any if 240p <= activeArraySize\n> >\n>\n> Those are already mandatory to be supported if I'm not mistaken.\n>\n> we feed cameraResolutions for all the non-jpeg formats here:\n>\n> for (const Size &res : cameraResolutions) {\n> cfg.pixelFormat = mappedFormat;\n> cfg.size = res;\n>\n> if (cameraConfig->validate() != CameraConfiguration::Valid)\n> continue;\n>\n> streamConfigurations_.push_back({ res, androidFormat });\n> }\n>\n> Then we use the sizes associated with the mandatory\n> HAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed\n> something on this part ?\n>\n> Looking at the code now, we could create an initJpeg() functions, as\n> all the jpeg streams are basically just ignored now and\n> initialized at the end of the function.\n>\n> Maybe it's really time to break this file apart, it's already quite\n> big and terse. What if we delegate the jpeg initalization part to the\n> Compressor interface (looking at it now, we could have gone for\n> Encoder as a name maybe).\n>\n> As of now it would basically require two\n> more methods: a way to get the a metadata pack with the values of\n> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE\n> initalized, and a vector of supported sizes given the vector of the\n> ones so far collected.\n>\n> Then we'll probably need a factory to create the right Compressor, but\n> that's for later and we should pobably first discuss how to decide\n> \"which\" encoder to initialize before getting to the \"how\".\n>\n> In example, how does it work today in CrOS and what are the\n> requirements going forward ?\n\nIf you mean JPEG resolution advertisement, I think it depends on the\ncamera hardware.\n\nIf it's a UVC camera, it often has higher resolution capture\ncapabilities with MJPEG rather than YUV. At the same time, Android\nrequires YUV capture at \"RECORD\" resolutions, which are typically\nhigher (e.g. 1920x1080) than what a UVC camera can output at\nreasonable framerate without resorting to MJPEG, so we have to\nadvertise any resolution supported by MJPEG as supported for YUV\nstreams and decode the MJPEG frames in the HAL to provide YUV frames\nto the user.\n\nFor RAW cameras, the JPEG streams are usually provided from YUV\nstreams anyway, so I believe reporting all the resolutions supported\nby YUV as supported by JPEG should be correct.\n\nIf you mean JPEG encoder selection, we have the encoding abstracted\nusing a generic userspace interface exposed from Chrome (called JEA -\nJpegEncodeAccelerator), which automatically uses whatever available in\nthe system. Currently it provides 3 implementations - V4L2, VAAPI and\nsoftware.\n\nBest regards,\nTomasz","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 170C5BD86F\n\tfor ;\n\tThu, 6 Aug 2020 12:27:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DF4360730;\n\tThu, 6 Aug 2020 14:27:35 +0200 (CEST)","from mail-wm1-x344.google.com (mail-wm1-x344.google.com\n\t[IPv6:2a00:1450:4864:20::344])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 62DDF60554\n\tfor ;\n\tThu, 6 Aug 2020 13:04:59 +0200 (CEST)","by mail-wm1-x344.google.com with SMTP id k20so9249791wmi.5\n\tfor ;\n\tThu, 06 Aug 2020 04:04:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"vYsPwod/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=cgv0g7nFJFtl7HcSJn82i/DYqBpHwpQRguhfjO3OAKc=;\n\tb=vYsPwod/bl44UpMVCDaocUu9qZmwTCRNErUGIpyFbcxsHkm+HEIlW+YZkb1eajXzTz\n\taL0TAyXb6SlxcgxPvJ466m7Ex3p4AzZu8y79ubrBxyEuxp0l/bKPafMJU8t7gyEUJtS7\n\t9ZF2ne+Eyee7/vWlkqE+3gteGSjTojVf05IrgUJugSTIBi7fbZ0oNsp1vQCI3+gF4xtK\n\taJ5Q2pAAvvGpVSWC8FgsLNGbftp7vbelgM7uZ+9pkpiu9cqGshIGl2onuv2D5bABQThw\n\tLxu+lyK3BphoeXs29bUVA8YDch5DXnEshesnZ5ncpBTovEiPYOdMLPZTF1NQ+lt9QPSq\n\tQ5sQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=cgv0g7nFJFtl7HcSJn82i/DYqBpHwpQRguhfjO3OAKc=;\n\tb=m2rGepclrIThL6TuHqSxQcCBLfV4OHHDxMMrq/Lr90ye8MOBJblXB6udpOyxKglfvP\n\thq8r4w9WRPdoi/h38iYrXDedXHp7MBM4oihoDpDpWl2NugjyxR4rS9gzeh3elfbUxVsC\n\tgCRxpRLJjYvwq3caqxbZtj9Pm/rbNj8NiJ2bCKYODjiG6qILenU0iKnusF/Gdk+M5aBE\n\tzwHPpGfVnFLIREBLU702sADXXjwfbcZ/qobnl2iN+PgZTXVscAN0tTg0ops4COgbTHvJ\n\tI/vlppyUHcnODUbiyQsh5S69QApKOqNlXO1xM7x1jt+I0LdApyOP99vr0N+6DemZH28n\n\tOhLg==","X-Gm-Message-State":"AOAM532hFNfXmqDJqeq9jBpVMuJbcbbh78Ui5WDNc7yd4EQwmeALgzTJ\n\tCAwhbruGCodO/0hBOGfIJWxDcQJePsuRIzsxUB0L0w==","X-Google-Smtp-Source":"ABdhPJw/QZzocLhlj3OyBo16dCzxMAWLsUgNq75ui6k/bD0aBj4tbOIiOYSsaeUUFIDUi/+9Ng3M8wFRiE9AF31uzic=","X-Received":"by 2002:a1c:9a81:: with SMTP id\n\tc123mr7241495wme.46.1596711898687; \n\tThu, 06 Aug 2020 04:04:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20200805153745.30378-1-jacopo@jmondi.org>\n\t<20200805165341.GK6751@pendragon.ideasonboard.com>\n\t<20200805213355.6llvit6znf5uuozc@uno.localdomain>","In-Reply-To":"<20200805213355.6llvit6znf5uuozc@uno.localdomain>","From":"Tomasz Figa ","Date":"Thu, 6 Aug 2020 13:04:47 +0200","Message-ID":"","To":"Jacopo Mondi ","X-Mailman-Approved-At":"Thu, 06 Aug 2020 14:27:33 +0200","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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":"libcamera devel ,\n\tHirokazu Honda ,\n\tkieran bingham ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11924,"web_url":"https://patchwork.libcamera.org/comment/11924/","msgid":"<20200806132306.mp24hwdjzdietgld@uno.localdomain>","date":"2020-08-06T13:23:06","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Tomasz,\n\nOn Thu, Aug 06, 2020 at 01:04:47PM +0200, Tomasz Figa wrote:\n> Hi Jacopo,\n>\n> On Wed, Aug 5, 2020 at 11:30 PM Jacopo Mondi wrote:\n> >\n> > Hi Laurent,\n> > +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)\n> > +Kieran for the Compressor interface part\n> >\n> > On Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:\n> > > Hi Jacopo,\n> > >\n> > > On Wed, Aug 05, 2020 at 05:37:45PM +0200, 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> > > >\n> > > > Patch to be applied on top of Kieran's JPEG work.\n> > > > ---\n> > > > src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n> > > > 1 file changed, 27 insertions(+), 11 deletions(-)\n> > > >\n> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > index ec8ca934842a..6a9a038a2b53 100644\n> > > > --- a/src/android/camera_device.cpp\n> > > > +++ b/src/android/camera_device.cpp\n> > > > @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n> > > > */\n> > > > formatsMap_[androidFormat] = mappedFormat;\n> > > >\n> > > > + /*\n> > > > + * Stop here for JPEG streams: the JPEG supported sizes will\n> > > > + * be tested later using the here recorded non-blob stream sizes.\n> > > > + */\n> > > > + if (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n> > > > + continue;\n> > > > +\n> > > > for (const Size &res : cameraResolutions) {\n> > > > cfg.pixelFormat = mappedFormat;\n> > > > cfg.size = res;\n> > > >\n> > > > - CameraConfiguration::Status status = cameraConfig->validate();\n> > > > - /*\n> > > > - * Unconditionally report we can produce JPEG.\n> > > > - *\n> > > > - * \\todo The JPEG stream will be implemented as an\n> > > > - * HAL-only stream, but some cameras can produce it\n> > > > - * directly. As of now, claim support for JPEG without\n> > > > - * inspecting where the JPEG stream is produced.\n> > > > - */\n> > > > - if (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> > > > - status != CameraConfiguration::Valid)\n> > > > + if (cameraConfig->validate() != CameraConfiguration::Valid)\n> > > > continue;\n> > > >\n> > > > streamConfigurations_.push_back({ res, androidFormat });\n> > > > }\n> > > > }\n> > > >\n> > > > + /*\n> > > > + * Insert the JPEG sizes by using the ones recorded for YUV streams\n> > > > + * from which JPEG is produced.\n> > > > + */\n> > > > + std::vector jpegConfigurations;\n> > > > + jpegConfigurations.reserve(cameraResolutions.size());\n> > > > +\n> > > > + for (const auto &config : streamConfigurations_) {\n> > > > + /* \\todo JPEG can be produced from other formats too! */\n> > >\n> > > Another todo item, the android.scaler.availableStreamConfigurations\n> > > documentation lists required resolutions (see\n> > > https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\n> > > which very annoyingly googlesource.com can't display as html in a web browser).\n> > >\n> > > JPEG android.sensor.info.activeArraySize Any\n> > > JPEG 1920x1080 (1080p) Any if 1080p <= activeArraySize\n> > > JPEG 1280x720 (720) Any if 720p <= activeArraySize\n> > > JPEG 640x480 (480p) Any if 480p <= activeArraySize\n> > > JPEG 320x240 (240p) Any if 240p <= activeArraySize\n> > >\n> >\n> > Those are already mandatory to be supported if I'm not mistaken.\n> >\n> > we feed cameraResolutions for all the non-jpeg formats here:\n> >\n> > for (const Size &res : cameraResolutions) {\n> > cfg.pixelFormat = mappedFormat;\n> > cfg.size = res;\n> >\n> > if (cameraConfig->validate() != CameraConfiguration::Valid)\n> > continue;\n> >\n> > streamConfigurations_.push_back({ res, androidFormat });\n> > }\n> >\n> > Then we use the sizes associated with the mandatory\n> > HAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed\n> > something on this part ?\n> >\n> > Looking at the code now, we could create an initJpeg() functions, as\n> > all the jpeg streams are basically just ignored now and\n> > initialized at the end of the function.\n> >\n> > Maybe it's really time to break this file apart, it's already quite\n> > big and terse. What if we delegate the jpeg initalization part to the\n> > Compressor interface (looking at it now, we could have gone for\n> > Encoder as a name maybe).\n> >\n> > As of now it would basically require two\n> > more methods: a way to get the a metadata pack with the values of\n> > ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE\n> > initalized, and a vector of supported sizes given the vector of the\n> > ones so far collected.\n> >\n> > Then we'll probably need a factory to create the right Compressor, but\n> > that's for later and we should pobably first discuss how to decide\n> > \"which\" encoder to initialize before getting to the \"how\".\n> >\n> > In example, how does it work today in CrOS and what are the\n> > requirements going forward ?\n>\n> If you mean JPEG resolution advertisement, I think it depends on the\n> camera hardware.\n>\n> If it's a UVC camera, it often has higher resolution capture\n> capabilities with MJPEG rather than YUV. At the same time, Android\n> requires YUV capture at \"RECORD\" resolutions, which are typically\n> higher (e.g. 1920x1080) than what a UVC camera can output at\n> reasonable framerate without resorting to MJPEG, so we have to\n> advertise any resolution supported by MJPEG as supported for YUV\n> streams and decode the MJPEG frames in the HAL to provide YUV frames\n> to the user.\n>\n> For RAW cameras, the JPEG streams are usually provided from YUV\n> streams anyway, so I believe reporting all the resolutions supported\n> by YUV as supported by JPEG should be correct.\n\nThanks, that's usefull to know.\n\n>\n> If you mean JPEG encoder selection, we have the encoding abstracted\n> using a generic userspace interface exposed from Chrome (called JEA -\n> JpegEncodeAccelerator), which automatically uses whatever available in\n> the system. Currently it provides 3 implementations - V4L2, VAAPI and\n> software.\n\nYep, I meant this part. My first thought was a factory in our Camera\nHAL, but the criteria to decide which encode to instantiate should be\ndefined first. Otherwise the alternative is for downstream to maintain\na version \"which does the right thing\" and I think we want to avoid\nit.\n\nThanks\n j\n\n>\n> Best regards,\n> Tomasz","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 EFB54BD86F\n\tfor ;\n\tThu, 6 Aug 2020 13:19:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 811BB60BF7;\n\tThu, 6 Aug 2020 15:19:29 +0200 (CEST)","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 97A456038F\n\tfor ;\n\tThu, 6 Aug 2020 15:19:27 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 551CC1BF207;\n\tThu, 6 Aug 2020 13:19:26 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 6 Aug 2020 15:23:06 +0200","From":"Jacopo Mondi ","To":"Tomasz Figa ","Message-ID":"<20200806132306.mp24hwdjzdietgld@uno.localdomain>","References":"<20200805153745.30378-1-jacopo@jmondi.org>\n\t<20200805165341.GK6751@pendragon.ideasonboard.com>\n\t<20200805213355.6llvit6znf5uuozc@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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":"libcamera devel ,\n\tHirokazu Honda ,\n\tkieran bingham ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11935,"web_url":"https://patchwork.libcamera.org/comment/11935/","msgid":"","date":"2020-08-06T13:25:48","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":65,"url":"https://patchwork.libcamera.org/api/people/65/","name":"Kieran Bingham","email":"kieran.bingham+renesas@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 05/08/2020 22:33, Jacopo Mondi wrote:\n> Hi Laurent,\n> +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)\n> +Kieran for the Compressor interface part\n> \n> On Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:\n>> Hi Jacopo,\n>>\n>> On Wed, Aug 05, 2020 at 05:37:45PM +0200, 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>>>\n>>> Patch to be applied on top of Kieran's JPEG work.\n>>> ---\n>>> src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n>>> 1 file changed, 27 insertions(+), 11 deletions(-)\n>>>\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index ec8ca934842a..6a9a038a2b53 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n>>> \t\t */\n>>> \t\tformatsMap_[androidFormat] = mappedFormat;\n>>>\n>>> +\t\t/*\n>>> +\t\t * Stop here for JPEG streams: the JPEG supported sizes will\n>>> +\t\t * be tested later using the here recorded non-blob stream sizes.\n>>> +\t\t */\n>>> +\t\tif (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n>>> +\t\t\tcontinue;\n>>> +\n>>> \t\tfor (const Size &res : cameraResolutions) {\n>>> \t\t\tcfg.pixelFormat = mappedFormat;\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 (cameraConfig->validate() != CameraConfiguration::Valid)\n>>> \t\t\t\tcontinue;\n>>>\n>>> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n>>> \t\t}\n>>> \t}\n>>>\n>>> +\t/*\n>>> +\t * Insert the JPEG sizes by using the ones recorded for YUV streams\n>>> +\t * from which JPEG is produced.\n>>> +\t */\n>>> +\tstd::vector jpegConfigurations;\n>>> +\tjpegConfigurations.reserve(cameraResolutions.size());\n>>> +\n>>> +\tfor (const auto &config : streamConfigurations_) {\n>>> +\t\t/* \\todo JPEG can be produced from other formats too! */\n>>\n>> Another todo item, the android.scaler.availableStreamConfigurations\n>> documentation lists required resolutions (see\n>> https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\n>> which very annoyingly googlesource.com can't display as html in a web browser).\n>>\n>> JPEG \tandroid.sensor.info.activeArraySize \tAny\n>> JPEG \t1920x1080 (1080p) \t\t\tAny \tif 1080p <= activeArraySize\n>> JPEG \t1280x720 (720) \t\t\t\tAny \tif 720p <= activeArraySize\n>> JPEG \t640x480 (480p) \t\t\t\tAny \tif 480p <= activeArraySize\n>> JPEG \t320x240 (240p) \t\t\t\tAny \tif 240p <= activeArraySize\n>>\n> \n> Those are already mandatory to be supported if I'm not mistaken.\n> \n> we feed cameraResolutions for all the non-jpeg formats here:\n> \n> \t\tfor (const Size &res : cameraResolutions) {\n> \t\t\tcfg.pixelFormat = mappedFormat;\n> \t\t\tcfg.size = res;\n> \n> \t\t\tif (cameraConfig->validate() != CameraConfiguration::Valid)\n> \t\t\t\tcontinue;\n> \n> \t\t\tstreamConfigurations_.push_back({ res, androidFormat });\n> \t\t}\n> \n> Then we use the sizes associated with the mandatory\n> HAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed\n> something on this part ?\n> \n> Looking at the code now, we could create an initJpeg() functions, as\n> all the jpeg streams are basically just ignored now and\n> initialized at the end of the function.\n> \n> Maybe it's really time to break this file apart, it's already quite\n> big and terse. What if we delegate the jpeg initalization part to the\n> Compressor interface (looking at it now, we could have gone for\n> Encoder as a name maybe).\n\nI certainly agree we need to split camera_device up.\n\nBut for me, I would instead consider that the compressor(/encoder)\nshouldn't know about anything except doing that job.\n\nManagement of the stream should be handled by a CameraStream interface\n(i.e. the struct CameraStream promoted to a full class, with it's own\ncamera_stream.cpp).\n\nThat's the layer that should be dealing with the interface between\nandroid streams and libcamera streams IMO. It should then be up to any\ninstance of a CameraStream to decide if and how to handle the\nrequirements for that stream, which may be to use a libcamera stream, or\nto use/implement a more configurable stream which might include\nscaler/convertor/compressors as required.\n\n\n\nAs for using the name Encoder, yes that's fine by me.\n\nI'll rename throughout my series now, I don't yet know if that will\nrequire a repost, but I had hoped to push already.\n\nI'll see if it warrants a v5 posting to the list or not depending on the\nchurn.\n\n--\nKieran\n\n\n\n> \n> As of now it would basically require two\n> more methods: a way to get the a metadata pack with the values of\n> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE\n> initalized, and a vector of supported sizes given the vector of the\n> ones so far collected.\n> \n> Then we'll probably need a factory to create the right Compressor, but\n> that's for later and we should pobably first discuss how to decide\n> \"which\" encoder to initialize before getting to the \"how\".\n> \n> In example, how does it work today in CrOS and what are the\n> requirements going forward ?\n> \n> Thanks\n> j\n> \n>>> +\t\tif (config.androidFormat != HAL_PIXEL_FORMAT_YCbCr_420_888)\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tjpegConfigurations.push_back({ config.resolution,\n>>> +\t\t\t\t\t HAL_PIXEL_FORMAT_BLOB });\n>>> +\t}\n>>> +\n>>> +\tfor (auto const jpegConfig : jpegConfigurations)\n>>> +\t\tstreamConfigurations_.push_back(jpegConfig);\n>>> +\n>>> \tLOG(HAL, Debug) << \"Collected stream configuration map: \";\n>>> \tfor (const auto &entry : streamConfigurations_)\n>>> \t\tLOG(HAL, Debug) << \"{ \" << entry.resolution.toString() << \" - \"\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 F0986BD87A\n\tfor ;\n\tThu, 6 Aug 2020 14:58:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86B8D60495;\n\tThu, 6 Aug 2020 16:58:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3CBC46038F\n\tfor ;\n\tThu, 6 Aug 2020 15:25:52 +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 88A1F50E;\n\tThu, 6 Aug 2020 15:25:51 +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=\"YRO+BKfB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596720351;\n\tbh=NO3edG0c9iSJZGEAQaBGU/rarHrVK3/sa+5rusJ3pDY=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=YRO+BKfBnXntceYWhzdVq8rzV9oI+5mAIbK4WTBv4OMGU6CnmfdCyqZcIvA+gBOSR\n\t440sF0TLaCZMyipG1tbP/7qC7yoN3aCMJnezUk5lI0Ug2O6RkeyxrNTQnoFw6FpAbg\n\t/og1ARU1SjIC/9B8CYgRKl9RJvsZGlsoaxPklerI=","To":"Jacopo Mondi ,\n\tLaurent Pinchart ","References":"<20200805153745.30378-1-jacopo@jmondi.org>\n\t<20200805165341.GK6751@pendragon.ideasonboard.com>\n\t<20200805213355.6llvit6znf5uuozc@uno.localdomain>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"","Date":"Thu, 6 Aug 2020 14:25:48 +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":"<20200805213355.6llvit6znf5uuozc@uno.localdomain>","Content-Language":"en-GB","X-Mailman-Approved-At":"Thu, 06 Aug 2020 16:58:02 +0200","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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+renesas@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\" "}},{"id":11937,"web_url":"https://patchwork.libcamera.org/comment/11937/","msgid":"","date":"2020-08-06T15:26:34","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG sizes","submitter":{"id":48,"url":"https://patchwork.libcamera.org/api/people/48/","name":"Tomasz Figa","email":"tfiga@google.com"},"content":"On Thu, Aug 6, 2020 at 3:19 PM Jacopo Mondi wrote:\n>\n> Hi Tomasz,\n>\n> On Thu, Aug 06, 2020 at 01:04:47PM +0200, Tomasz Figa wrote:\n> > Hi Jacopo,\n> >\n> > On Wed, Aug 5, 2020 at 11:30 PM Jacopo Mondi wrote:\n> > >\n> > > Hi Laurent,\n> > > +Tomasz, Hiro and Han-Lin as there's a cros question at the end :)\n> > > +Kieran for the Compressor interface part\n> > >\n> > > On Wed, Aug 05, 2020 at 07:53:41PM +0300, Laurent Pinchart wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > On Wed, Aug 05, 2020 at 05:37:45PM +0200, 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> > > > >\n> > > > > Patch to be applied on top of Kieran's JPEG work.\n> > > > > ---\n> > > > > src/android/camera_device.cpp | 38 +++++++++++++++++++++++++----------\n> > > > > 1 file changed, 27 insertions(+), 11 deletions(-)\n> > > > >\n> > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > index ec8ca934842a..6a9a038a2b53 100644\n> > > > > --- a/src/android/camera_device.cpp\n> > > > > +++ b/src/android/camera_device.cpp\n> > > > > @@ -398,27 +398,43 @@ int CameraDevice::initializeStreamConfigurations()\n> > > > > */\n> > > > > formatsMap_[androidFormat] = mappedFormat;\n> > > > >\n> > > > > + /*\n> > > > > + * Stop here for JPEG streams: the JPEG supported sizes will\n> > > > > + * be tested later using the here recorded non-blob stream sizes.\n> > > > > + */\n> > > > > + if (androidFormat == HAL_PIXEL_FORMAT_BLOB)\n> > > > > + continue;\n> > > > > +\n> > > > > for (const Size &res : cameraResolutions) {\n> > > > > cfg.pixelFormat = mappedFormat;\n> > > > > cfg.size = res;\n> > > > >\n> > > > > - CameraConfiguration::Status status = cameraConfig->validate();\n> > > > > - /*\n> > > > > - * Unconditionally report we can produce JPEG.\n> > > > > - *\n> > > > > - * \\todo The JPEG stream will be implemented as an\n> > > > > - * HAL-only stream, but some cameras can produce it\n> > > > > - * directly. As of now, claim support for JPEG without\n> > > > > - * inspecting where the JPEG stream is produced.\n> > > > > - */\n> > > > > - if (androidFormat != HAL_PIXEL_FORMAT_BLOB &&\n> > > > > - status != CameraConfiguration::Valid)\n> > > > > + if (cameraConfig->validate() != CameraConfiguration::Valid)\n> > > > > continue;\n> > > > >\n> > > > > streamConfigurations_.push_back({ res, androidFormat });\n> > > > > }\n> > > > > }\n> > > > >\n> > > > > + /*\n> > > > > + * Insert the JPEG sizes by using the ones recorded for YUV streams\n> > > > > + * from which JPEG is produced.\n> > > > > + */\n> > > > > + std::vector jpegConfigurations;\n> > > > > + jpegConfigurations.reserve(cameraResolutions.size());\n> > > > > +\n> > > > > + for (const auto &config : streamConfigurations_) {\n> > > > > + /* \\todo JPEG can be produced from other formats too! */\n> > > >\n> > > > Another todo item, the android.scaler.availableStreamConfigurations\n> > > > documentation lists required resolutions (see\n> > > > https://android.googlesource.com/platform/system/media/+/refs/heads/master/camera/docs/docs.html\n> > > > which very annoyingly googlesource.com can't display as html in a web browser).\n> > > >\n> > > > JPEG android.sensor.info.activeArraySize Any\n> > > > JPEG 1920x1080 (1080p) Any if 1080p <= activeArraySize\n> > > > JPEG 1280x720 (720) Any if 720p <= activeArraySize\n> > > > JPEG 640x480 (480p) Any if 480p <= activeArraySize\n> > > > JPEG 320x240 (240p) Any if 240p <= activeArraySize\n> > > >\n> > >\n> > > Those are already mandatory to be supported if I'm not mistaken.\n> > >\n> > > we feed cameraResolutions for all the non-jpeg formats here:\n> > >\n> > > for (const Size &res : cameraResolutions) {\n> > > cfg.pixelFormat = mappedFormat;\n> > > cfg.size = res;\n> > >\n> > > if (cameraConfig->validate() != CameraConfiguration::Valid)\n> > > continue;\n> > >\n> > > streamConfigurations_.push_back({ res, androidFormat });\n> > > }\n> > >\n> > > Then we use the sizes associated with the mandatory\n> > > HAL_PIXEL_FORMAT_YCbCr_420_888 to add entries for jpeg. Have I missed\n> > > something on this part ?\n> > >\n> > > Looking at the code now, we could create an initJpeg() functions, as\n> > > all the jpeg streams are basically just ignored now and\n> > > initialized at the end of the function.\n> > >\n> > > Maybe it's really time to break this file apart, it's already quite\n> > > big and terse. What if we delegate the jpeg initalization part to the\n> > > Compressor interface (looking at it now, we could have gone for\n> > > Encoder as a name maybe).\n> > >\n> > > As of now it would basically require two\n> > > more methods: a way to get the a metadata pack with the values of\n> > > ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES and ANDROID_JPEG_MAX_SIZE\n> > > initalized, and a vector of supported sizes given the vector of the\n> > > ones so far collected.\n> > >\n> > > Then we'll probably need a factory to create the right Compressor, but\n> > > that's for later and we should pobably first discuss how to decide\n> > > \"which\" encoder to initialize before getting to the \"how\".\n> > >\n> > > In example, how does it work today in CrOS and what are the\n> > > requirements going forward ?\n> >\n> > If you mean JPEG resolution advertisement, I think it depends on the\n> > camera hardware.\n> >\n> > If it's a UVC camera, it often has higher resolution capture\n> > capabilities with MJPEG rather than YUV. At the same time, Android\n> > requires YUV capture at \"RECORD\" resolutions, which are typically\n> > higher (e.g. 1920x1080) than what a UVC camera can output at\n> > reasonable framerate without resorting to MJPEG, so we have to\n> > advertise any resolution supported by MJPEG as supported for YUV\n> > streams and decode the MJPEG frames in the HAL to provide YUV frames\n> > to the user.\n> >\n> > For RAW cameras, the JPEG streams are usually provided from YUV\n> > streams anyway, so I believe reporting all the resolutions supported\n> > by YUV as supported by JPEG should be correct.\n>\n> Thanks, that's usefull to know.\n\nAlso note that for MJPEG output from UVC cameras, it would be a waste\nto re-encode the YUV back into JPEG, so we instead just perform a\nlightweight conversion of the MJPEG frame into a proper JPEG (AFAIR\njust some headers need to be added).\n\n>\n> >\n> > If you mean JPEG encoder selection, we have the encoding abstracted\n> > using a generic userspace interface exposed from Chrome (called JEA -\n> > JpegEncodeAccelerator), which automatically uses whatever available in\n> > the system. Currently it provides 3 implementations - V4L2, VAAPI and\n> > software.\n>\n> Yep, I meant this part. My first thought was a factory in our Camera\n> HAL, but the criteria to decide which encode to instantiate should be\n> defined first. Otherwise the alternative is for downstream to maintain\n> a version \"which does the right thing\" and I think we want to avoid\n> it.\n\nIn Chrome OS, the camera service may not have access to the JPEG\nencoder hardware (which could be exposed via VAAPI), so we would\nalways want to delegate the encoding to JEA.\n\nBest regards,\nTomasz","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 A647BBD87A\n\tfor ;\n\tThu, 6 Aug 2020 15:26:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3BE6B6038F;\n\tThu, 6 Aug 2020 17:26:50 +0200 (CEST)","from mail-wr1-x432.google.com (mail-wr1-x432.google.com\n\t[IPv6:2a00:1450:4864:20::432])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 470DE6038F\n\tfor ;\n\tThu, 6 Aug 2020 17:26:48 +0200 (CEST)","by mail-wr1-x432.google.com with SMTP id c15so8184020wrs.11\n\tfor ;\n\tThu, 06 Aug 2020 08:26:48 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"p/1atTue\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=A+uSPZueYWNYavOAQLbm/VT5kn+bFdGoCi+OaVdNYT8=;\n\tb=p/1atTuewCtfDsqxfax8s0ZUmD/NEpr69yp39zVgyfEloPt3QFMdJB/SruR3W8Dco5\n\tFbKpCCc3+1HwoxGdTguYvD8M4y6v7jtAs8fV7B1s7Y4kfQFY/y4JvjyjKR7id4b3KD5t\n\tO0H9/SDBjY/Xig+b6bY+49R7ECiLsHyFuAjjBZWckTPrhUMI4ZR4seJ81SYGe5vphYME\n\t8UNGeV0AzxJTUVgoU3ZT7sy1RnFQuCKnJ3GsBXfhGdEC//HEx9rpvVk65+U1RAD4ELxI\n\t2TrFCGdv5gR4J/kIf9lxDJzGHNTpFf3fUszTrSbnqWZG+f5a7VyfILDk0jL3lYAlUryW\n\twtFw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=A+uSPZueYWNYavOAQLbm/VT5kn+bFdGoCi+OaVdNYT8=;\n\tb=L/b8W/fiInw74U3FdohkTiNyKe9TqlprkT7SN65hV0tWtKvcihXrc1hixdNqYukCtN\n\tAxVN7qYakf9i/MylHQonJyvF97CR0ypkwwsVLoBk8wIGsbCksBEqyn2S7OwFZXfJT8Ld\n\ty2DW5/0B602yVcbTia64vpAUebwwDfXUHRAhujVe/M40oUF4HKp9K06gDsYL6iyTCpal\n\tCOc6rzysySi92Iu+/acol6knJcBXyCZScJ2NTQz7Jj+RKc1cwJfFmksPAn/qamuOtUHi\n\t1G3SHAeIy74tYFY/nnlXjTGR4IA8W1bt9+KqRtlbjd4k7Q1v3DIhhksTWxD5bPCeR9iz\n\t6Scg==","X-Gm-Message-State":"AOAM532QwSINU6GyAA8M2CxUM5Zb9tmrMFLaPjCOUzoABTw7kCkA/2rf\n\tjDq6QhS4sKx0k9jYS4FMF43WQxKZDlPRNohnKclL2A==","X-Google-Smtp-Source":"ABdhPJycznoAlwCHry33oMOHGqmT3ChyHax02G17S4JTrBcqrQgA9RE9NoLjcS18Cbtq03csJaQ4+TfKeA8RPztoSo0=","X-Received":"by 2002:adf:ec45:: with SMTP id w5mr8030313wrn.415.1596727607648;\n\tThu, 06 Aug 2020 08:26:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20200805153745.30378-1-jacopo@jmondi.org>\n\t<20200805165341.GK6751@pendragon.ideasonboard.com>\n\t<20200805213355.6llvit6znf5uuozc@uno.localdomain>\n\t\n\t<20200806132306.mp24hwdjzdietgld@uno.localdomain>","In-Reply-To":"<20200806132306.mp24hwdjzdietgld@uno.localdomain>","From":"Tomasz Figa ","Date":"Thu, 6 Aug 2020 17:26:34 +0200","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Report\n\tsupported JPEG 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":"libcamera devel ,\n\tHirokazu Honda ,\n\tkieran bingham ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]