[{"id":26442,"web_url":"https://patchwork.libcamera.org/comment/26442/","msgid":"","date":"2023-02-09T21:55:21","subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Harvey,\n\nThank you for the patch.\n\nOn Wed, Feb 08, 2023 at 03:33:19AM +0000, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang \n> \n> This patch adds JEA implementation to replace libjpeg in CrOS platform,\n> where hardware accelerator is available.\n> \n> Signed-off-by: Harvey Yang \n> ---\n> src/android/cros/camera3_hal.cpp | 4 +-\n> src/android/cros_mojo_token.h | 12 +++++\n> src/android/jpeg/encoder_jea.cpp | 56 ++++++++++++++++++++++++\n> src/android/jpeg/encoder_jea.h | 31 +++++++++++++\n> src/android/jpeg/meson.build | 10 ++++-\n> src/android/jpeg/post_processor_jpeg.cpp | 8 ++++\n> 6 files changed, 118 insertions(+), 3 deletions(-)\n> create mode 100644 src/android/cros_mojo_token.h\n> create mode 100644 src/android/jpeg/encoder_jea.cpp\n> create mode 100644 src/android/jpeg/encoder_jea.h\n> \n> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp\n> index fb863b5f..71acb441 100644\n> --- a/src/android/cros/camera3_hal.cpp\n> +++ b/src/android/cros/camera3_hal.cpp\n> @@ -8,9 +8,11 @@\n> #include \n> \n> #include \"../camera_hal_manager.h\"\n> +#include \"../cros_mojo_token.h\"\n> \n> -static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)\n> +static void set_up(cros::CameraMojoChannelManagerToken *token)\n> {\n> +\tgCrosMojoToken = token;\n> }\n> \n> static void tear_down()\n> diff --git a/src/android/cros_mojo_token.h b/src/android/cros_mojo_token.h\n> new file mode 100644\n> index 00000000..043c752a\n> --- /dev/null\n> +++ b/src/android/cros_mojo_token.h\n> @@ -0,0 +1,12 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * cros_mojo_token.h - cros-specific mojo token\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +inline cros::CameraMojoChannelManagerToken *gCrosMojoToken = nullptr;\n> diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp\n> new file mode 100644\n> index 00000000..7880a6bd\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.cpp\n> @@ -0,0 +1,56 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * encoder_jea.cpp - JPEG encoding using CrOS JEA\n> + */\n> +\n> +#include \"encoder_jea.h\"\n> +\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +\n> +#include \n> +\n> +#include \"../cros_mojo_token.h\"\n> +#include \"../hal_framebuffer.h\"\n> +\n> +EncoderJea::EncoderJea() = default;\n> +\n> +EncoderJea::~EncoderJea() = default;\n> +\n> +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)\n> +{\n> +\tsize_ = cfg.size;\n> +\n> +\tif (jpegCompressor_)\n> +\t\treturn 0;\n> +\n> +\tif (gCrosMojoToken == nullptr)\n> +\t\treturn -ENOTSUP;\n> +\n> +\tjpegCompressor_ = cros::JpegCompressor::GetInstance(gCrosMojoToken);\n> +\n> +\treturn 0;\n> +}\n> +\n> +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> +\t\t libcamera::Span exifData,\n> +\t\t unsigned int quality)\n> +{\n> +\tif (!jpegCompressor_)\n> +\t\treturn -ENOTSUP;\n> +\n> +\tuint32_t outDataSize = 0;\n> +\tconst HALFrameBuffer *fb =\n> +\t\tdynamic_cast(buffer->srcBuffer);\n> +\n> +\tif (!jpegCompressor_->CompressImageFromHandle(fb->handle(),\n> +\t\t\t\t\t\t *buffer->camera3Buffer,\n> +\t\t\t\t\t\t size_.width, size_.height,\n> +\t\t\t\t\t\t quality, exifData.data(),\n> +\t\t\t\t\t\t exifData.size(),\n> +\t\t\t\t\t\t &outDataSize))\n> +\t\treturn -EBUSY;\n> +\n> +\treturn outDataSize;\n> +}\n> diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h\n> new file mode 100644\n> index 00000000..ffe9df27\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.h\n> @@ -0,0 +1,31 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * encoder_jea.h - JPEG encoding using CrOS JEA\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"encoder.h\"\n> +\n> +class EncoderJea : public Encoder\n> +{\n> +public:\n> +\tEncoderJea();\n> +\t~EncoderJea();\n> +\n> +\tint configure(const libcamera::StreamConfiguration &cfg) override;\n> +\tint encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> +\t\t libcamera::Span exifData,\n> +\t\t unsigned int quality) override;\n> +\n> +private:\n> +\tlibcamera::Size size_;\n> +\n> +\tstd::unique_ptr jpegCompressor_;\n> +};\n> diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build\n> index 08397a87..9162576b 100644\n> --- a/src/android/jpeg/meson.build\n> +++ b/src/android/jpeg/meson.build\n> @@ -1,8 +1,14 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> android_hal_sources += files([\n> - 'encoder_libjpeg.cpp',\n> 'exif.cpp',\n> + 'encoder_libjpeg.cpp',\n> + 'thumbnailer.cpp',\n> 'post_processor_jpeg.cpp',\n> - 'thumbnailer.cpp'\n> ])\n\nThis doesn't seem right, or needed. I'll fix this locally and push the\nseries after running it through CTS.\n\n> +\n> +platform = get_option('android_platform')\n> +if platform == 'cros'\n> + android_hal_sources += files(['encoder_jea.cpp'])\n> + android_deps += [dependency('libcros_camera')]\n> +endif\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 3e676eb4..40261652 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -12,7 +12,11 @@\n> #include \"../camera_device.h\"\n> #include \"../camera_metadata.h\"\n> #include \"../camera_request.h\"\n> +#if defined(OS_CHROMEOS)\n> +#include \"encoder_jea.h\"\n> +#else /* !defined(OS_CHROMEOS) */\n> #include \"encoder_libjpeg.h\"\n> +#endif\n> #include \"exif.h\"\n> \n> #include \n> @@ -46,7 +50,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> \n> \tthumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> \n> +#if defined(OS_CHROMEOS)\n> +\tencoder_ = std::make_unique();\n> +#else /* !defined(OS_CHROMEOS) */\n> \tencoder_ = std::make_unique();\n> +#endif\n> \n> \treturn encoder_->configure(inCfg);\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 B1FD9BDB13\n\tfor ;\n\tThu, 9 Feb 2023 21:55:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08F21625F4;\n\tThu, 9 Feb 2023 22:55:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 775E4603BD\n\tfor ;\n\tThu, 9 Feb 2023 22:55:24 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BDA109CA;\n\tThu, 9 Feb 2023 22:55:23 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675979726;\n\tbh=yg+qpn0XpB7qxzzeb3nCxEngv/a0hH0xq4cqLh7y/5s=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DAl7/PM0gLqfeYLOfoeoSkZzcs287Gl+T1da3R1msLrCVTC5YLw6IlvaL8i8mUGl2\n\tNtMFkG0oekm99U1RTCXAkclqdoBI4SbGYbuNQWK2KJG+gwLDvo1GMxQZOZq0dvYEpQ\n\txYOTrMkQ7q5vg2OOhDzPp/hlWb40+5m1GzJSIcGrFhTlRhKu05OohaJYAaDuEeiLob\n\tt2tj4TW0dreOQHikZVB/daJNQVURVLFbLvchmz1R/rjBSr88hyJV9hYtcgR0MctDOG\n\tZXv2J28G3yR+s08wmNRYHfT5qj+tYVTUUw/ShIdLk4tQSX8+DOPJMvPWMQvqe0/zi0\n\t+v4VRq6i4GAeQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675979724;\n\tbh=yg+qpn0XpB7qxzzeb3nCxEngv/a0hH0xq4cqLh7y/5s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vVzcLz9IZqYSDyZKYkcuBoYvp9B/ngf9vrI9m122QMrMAx9BlQV2uBaR1CF0KqOIH\n\thR7xjn2Vd1+AjnI/gaWXbnxXEXyr9TmTY5Dh8uc5325fRx4ix90Sdq6Dmg4+F25HQk\n\tmHv0Qo7t542+EivGMxSXqq4Q3uaAvCQiLouWVzh8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vVzcLz9I\"; dkim-atps=neutral","Date":"Thu, 9 Feb 2023 23:55:21 +0200","To":"Harvey Yang ","Message-ID":"","References":"<20230208033319.1361109-1-chenghaoyang@google.com>\n\t<20230208033319.1361109-6-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230208033319.1361109-6-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26443,"web_url":"https://patchwork.libcamera.org/comment/26443/","msgid":"<167598286822.1526433.9971437652814373951@Monstersaurus>","date":"2023-02-09T22:47:48","subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2023-02-09 21:55:21)\n> Hi Harvey,\n> \n> Thank you for the patch.\n> \n> On Wed, Feb 08, 2023 at 03:33:19AM +0000, Harvey Yang via libcamera-devel wrote:\n> > From: Harvey Yang \n> > \n> > This patch adds JEA implementation to replace libjpeg in CrOS platform,\n> > where hardware accelerator is available.\n> > \n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/android/cros/camera3_hal.cpp | 4 +-\n> > src/android/cros_mojo_token.h | 12 +++++\n> > src/android/jpeg/encoder_jea.cpp | 56 ++++++++++++++++++++++++\n> > src/android/jpeg/encoder_jea.h | 31 +++++++++++++\n> > src/android/jpeg/meson.build | 10 ++++-\n> > src/android/jpeg/post_processor_jpeg.cpp | 8 ++++\n> > 6 files changed, 118 insertions(+), 3 deletions(-)\n> > create mode 100644 src/android/cros_mojo_token.h\n> > create mode 100644 src/android/jpeg/encoder_jea.cpp\n> > create mode 100644 src/android/jpeg/encoder_jea.h\n> > \n> > diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp\n> > index fb863b5f..71acb441 100644\n> > --- a/src/android/cros/camera3_hal.cpp\n> > +++ b/src/android/cros/camera3_hal.cpp\n> > @@ -8,9 +8,11 @@\n> > #include \n> > \n> > #include \"../camera_hal_manager.h\"\n> > +#include \"../cros_mojo_token.h\"\n> > \n> > -static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)\n> > +static void set_up(cros::CameraMojoChannelManagerToken *token)\n> > {\n> > + gCrosMojoToken = token;\n> > }\n> > \n> > static void tear_down()\n> > diff --git a/src/android/cros_mojo_token.h b/src/android/cros_mojo_token.h\n> > new file mode 100644\n> > index 00000000..043c752a\n> > --- /dev/null\n> > +++ b/src/android/cros_mojo_token.h\n> > @@ -0,0 +1,12 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * cros_mojo_token.h - cros-specific mojo token\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +inline cros::CameraMojoChannelManagerToken *gCrosMojoToken = nullptr;\n> > diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp\n> > new file mode 100644\n> > index 00000000..7880a6bd\n> > --- /dev/null\n> > +++ b/src/android/jpeg/encoder_jea.cpp\n> > @@ -0,0 +1,56 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * encoder_jea.cpp - JPEG encoding using CrOS JEA\n> > + */\n> > +\n> > +#include \"encoder_jea.h\"\n> > +\n> > +#include \"libcamera/internal/mapped_framebuffer.h\"\n> > +\n> > +#include \n> > +\n> > +#include \"../cros_mojo_token.h\"\n> > +#include \"../hal_framebuffer.h\"\n> > +\n> > +EncoderJea::EncoderJea() = default;\n> > +\n> > +EncoderJea::~EncoderJea() = default;\n> > +\n> > +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)\n> > +{\n> > + size_ = cfg.size;\n> > +\n> > + if (jpegCompressor_)\n> > + return 0;\n> > +\n> > + if (gCrosMojoToken == nullptr)\n> > + return -ENOTSUP;\n> > +\n> > + jpegCompressor_ = cros::JpegCompressor::GetInstance(gCrosMojoToken);\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> > + libcamera::Span exifData,\n> > + unsigned int quality)\n> > +{\n> > + if (!jpegCompressor_)\n> > + return -ENOTSUP;\n> > +\n> > + uint32_t outDataSize = 0;\n> > + const HALFrameBuffer *fb =\n> > + dynamic_cast(buffer->srcBuffer);\n> > +\n> > + if (!jpegCompressor_->CompressImageFromHandle(fb->handle(),\n> > + *buffer->camera3Buffer,\n> > + size_.width, size_.height,\n> > + quality, exifData.data(),\n> > + exifData.size(),\n> > + &outDataSize))\n> > + return -EBUSY;\n> > +\n> > + return outDataSize;\n> > +}\n> > diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h\n> > new file mode 100644\n> > index 00000000..ffe9df27\n> > --- /dev/null\n> > +++ b/src/android/jpeg/encoder_jea.h\n> > @@ -0,0 +1,31 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * encoder_jea.h - JPEG encoding using CrOS JEA\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"encoder.h\"\n> > +\n> > +class EncoderJea : public Encoder\n> > +{\n> > +public:\n> > + EncoderJea();\n> > + ~EncoderJea();\n> > +\n> > + int configure(const libcamera::StreamConfiguration &cfg) override;\n> > + int encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> > + libcamera::Span exifData,\n> > + unsigned int quality) override;\n> > +\n> > +private:\n> > + libcamera::Size size_;\n> > +\n> > + std::unique_ptr jpegCompressor_;\n> > +};\n> > diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build\n> > index 08397a87..9162576b 100644\n> > --- a/src/android/jpeg/meson.build\n> > +++ b/src/android/jpeg/meson.build\n> > @@ -1,8 +1,14 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> > \n> > android_hal_sources += files([\n> > - 'encoder_libjpeg.cpp',\n> > 'exif.cpp',\n> > + 'encoder_libjpeg.cpp',\n> > + 'thumbnailer.cpp',\n> > 'post_processor_jpeg.cpp',\n> > - 'thumbnailer.cpp'\n> > ])\n> \n> This doesn't seem right, or needed. I'll fix this locally and push the\n> series after running it through CTS.\n\nFeel free to run again, but I ran this through CTS earlier:\n\n- https://results.uk.libcamera.org/0.0.4+10-53502641/2023.02.09_15.09.10/test_result.xml\n\n8 failures, but not a regression - so this is OK with me.\n\n--\nKieran\n\n\n> \n> > +\n> > +platform = get_option('android_platform')\n> > +if platform == 'cros'\n> > + android_hal_sources += files(['encoder_jea.cpp'])\n> > + android_deps += [dependency('libcros_camera')]\n> > +endif\n> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> > index 3e676eb4..40261652 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -12,7 +12,11 @@\n> > #include \"../camera_device.h\"\n> > #include \"../camera_metadata.h\"\n> > #include \"../camera_request.h\"\n> > +#if defined(OS_CHROMEOS)\n> > +#include \"encoder_jea.h\"\n> > +#else /* !defined(OS_CHROMEOS) */\n> > #include \"encoder_libjpeg.h\"\n> > +#endif\n> > #include \"exif.h\"\n> > \n> > #include \n> > @@ -46,7 +50,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> > \n> > thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> > \n> > +#if defined(OS_CHROMEOS)\n> > + encoder_ = std::make_unique();\n> > +#else /* !defined(OS_CHROMEOS) */\n> > encoder_ = std::make_unique();\n> > +#endif\n> > \n> > return encoder_->configure(inCfg);\n> > }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EFA16BDB1C\n\tfor ;\n\tThu, 9 Feb 2023 22:47:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70A08625E3;\n\tThu, 9 Feb 2023 23:47:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 51566603BD\n\tfor ;\n\tThu, 9 Feb 2023 23:47:51 +0100 (CET)","from pendragon.ideasonboard.com\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 9AB519CA;\n\tThu, 9 Feb 2023 23:47:50 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675982873;\n\tbh=CECqLYmCQM6DDW15qAuLUETMFzAzdvFdXnAoT3wOD7g=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Gw/E+rRpN0tkdUWLIqaOFPiisqI6bwEjI3KyE1Q3KdW9JMRRg1XLkxVAr8vSzjZ2C\n\tfpsGVyIcpLg/4tsWFddGaZOpnYGJA2+QEv8Q3UnU83rcWnmquRatocavtIQRDpZv0Y\n\tYvN24t737Cq6gBY7aTYfRNEdwKLZYUO3v4obYwXBQykIQA+ezE+1UsZIbkxFBjV7ou\n\tHp1XtCxJh82x/XDbfnKlsZp9eJq0cqxiG1MwPf/2dHsliSRkB/MlYQ06kWFLMnVrmg\n\tCQ4ZmnEKeddDGieoZsC/mDFLiHD4QVeZlj/2uzKCwIpaiUkckpHzQ8EHvHebrF5x5L\n\taHERdHyOyGnUg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675982870;\n\tbh=CECqLYmCQM6DDW15qAuLUETMFzAzdvFdXnAoT3wOD7g=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ALhYNO92soEXjnRvsR781WiJwuqb2vy2b0RQ8a7NYmonBRWqgYmIOHJ4dLqSStcxY\n\t5FOx4jOpLkps4aJYMvmdVuljrkYLh1xsKikTh3aY16g04e05BHMvWXD8/tqfhtxm76\n\tFnpskaem+0XQIWbm0NCx3P8qKCl0zOs8UoCBunRA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ALhYNO92\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230208033319.1361109-1-chenghaoyang@google.com>\n\t<20230208033319.1361109-6-chenghaoyang@google.com>\n\t","To":"Harvey Yang ,\n\tLaurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t","Date":"Thu, 09 Feb 2023 22:47:48 +0000","Message-ID":"<167598286822.1526433.9971437652814373951@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26454,"web_url":"https://patchwork.libcamera.org/comment/26454/","msgid":"","date":"2023-02-14T09:45:44","subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thank you Laurent, Kieran, and all for reviewing and helping land the\nseries of patches!\n\nOn Fri, Feb 10, 2023 at 6:47 AM Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Laurent Pinchart via libcamera-devel (2023-02-09 21:55:21)\n> > Hi Harvey,\n> >\n> > Thank you for the patch.\n> >\n> > On Wed, Feb 08, 2023 at 03:33:19AM +0000, Harvey Yang via\n> libcamera-devel wrote:\n> > > From: Harvey Yang \n> > >\n> > > This patch adds JEA implementation to replace libjpeg in CrOS platform,\n> > > where hardware accelerator is available.\n> > >\n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > src/android/cros/camera3_hal.cpp | 4 +-\n> > > src/android/cros_mojo_token.h | 12 +++++\n> > > src/android/jpeg/encoder_jea.cpp | 56 ++++++++++++++++++++++++\n> > > src/android/jpeg/encoder_jea.h | 31 +++++++++++++\n> > > src/android/jpeg/meson.build | 10 ++++-\n> > > src/android/jpeg/post_processor_jpeg.cpp | 8 ++++\n> > > 6 files changed, 118 insertions(+), 3 deletions(-)\n> > > create mode 100644 src/android/cros_mojo_token.h\n> > > create mode 100644 src/android/jpeg/encoder_jea.cpp\n> > > create mode 100644 src/android/jpeg/encoder_jea.h\n> > >\n> > > diff --git a/src/android/cros/camera3_hal.cpp\n> b/src/android/cros/camera3_hal.cpp\n> > > index fb863b5f..71acb441 100644\n> > > --- a/src/android/cros/camera3_hal.cpp\n> > > +++ b/src/android/cros/camera3_hal.cpp\n> > > @@ -8,9 +8,11 @@\n> > > #include \n> > >\n> > > #include \"../camera_hal_manager.h\"\n> > > +#include \"../cros_mojo_token.h\"\n> > >\n> > > -static void set_up([[maybe_unused]]\n> cros::CameraMojoChannelManagerToken *token)\n> > > +static void set_up(cros::CameraMojoChannelManagerToken *token)\n> > > {\n> > > + gCrosMojoToken = token;\n> > > }\n> > >\n> > > static void tear_down()\n> > > diff --git a/src/android/cros_mojo_token.h\n> b/src/android/cros_mojo_token.h\n> > > new file mode 100644\n> > > index 00000000..043c752a\n> > > --- /dev/null\n> > > +++ b/src/android/cros_mojo_token.h\n> > > @@ -0,0 +1,12 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2022, Google Inc.\n> > > + *\n> > > + * cros_mojo_token.h - cros-specific mojo token\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +\n> > > +inline cros::CameraMojoChannelManagerToken *gCrosMojoToken = nullptr;\n> > > diff --git a/src/android/jpeg/encoder_jea.cpp\n> b/src/android/jpeg/encoder_jea.cpp\n> > > new file mode 100644\n> > > index 00000000..7880a6bd\n> > > --- /dev/null\n> > > +++ b/src/android/jpeg/encoder_jea.cpp\n> > > @@ -0,0 +1,56 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2022, Google Inc.\n> > > + *\n> > > + * encoder_jea.cpp - JPEG encoding using CrOS JEA\n> > > + */\n> > > +\n> > > +#include \"encoder_jea.h\"\n> > > +\n> > > +#include \"libcamera/internal/mapped_framebuffer.h\"\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"../cros_mojo_token.h\"\n> > > +#include \"../hal_framebuffer.h\"\n> > > +\n> > > +EncoderJea::EncoderJea() = default;\n> > > +\n> > > +EncoderJea::~EncoderJea() = default;\n> > > +\n> > > +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)\n> > > +{\n> > > + size_ = cfg.size;\n> > > +\n> > > + if (jpegCompressor_)\n> > > + return 0;\n> > > +\n> > > + if (gCrosMojoToken == nullptr)\n> > > + return -ENOTSUP;\n> > > +\n> > > + jpegCompressor_ =\n> cros::JpegCompressor::GetInstance(gCrosMojoToken);\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> > > + libcamera::Span exifData,\n> > > + unsigned int quality)\n> > > +{\n> > > + if (!jpegCompressor_)\n> > > + return -ENOTSUP;\n> > > +\n> > > + uint32_t outDataSize = 0;\n> > > + const HALFrameBuffer *fb =\n> > > + dynamic_cast(buffer->srcBuffer);\n> > > +\n> > > + if (!jpegCompressor_->CompressImageFromHandle(fb->handle(),\n> > > +\n> *buffer->camera3Buffer,\n> > > + size_.width,\n> size_.height,\n> > > + quality,\n> exifData.data(),\n> > > + exifData.size(),\n> > > + &outDataSize))\n> > > + return -EBUSY;\n> > > +\n> > > + return outDataSize;\n> > > +}\n> > > diff --git a/src/android/jpeg/encoder_jea.h\n> b/src/android/jpeg/encoder_jea.h\n> > > new file mode 100644\n> > > index 00000000..ffe9df27\n> > > --- /dev/null\n> > > +++ b/src/android/jpeg/encoder_jea.h\n> > > @@ -0,0 +1,31 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2022, Google Inc.\n> > > + *\n> > > + * encoder_jea.h - JPEG encoding using CrOS JEA\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \"encoder.h\"\n> > > +\n> > > +class EncoderJea : public Encoder\n> > > +{\n> > > +public:\n> > > + EncoderJea();\n> > > + ~EncoderJea();\n> > > +\n> > > + int configure(const libcamera::StreamConfiguration &cfg)\n> override;\n> > > + int encode(Camera3RequestDescriptor::StreamBuffer *buffer,\n> > > + libcamera::Span exifData,\n> > > + unsigned int quality) override;\n> > > +\n> > > +private:\n> > > + libcamera::Size size_;\n> > > +\n> > > + std::unique_ptr jpegCompressor_;\n> > > +};\n> > > diff --git a/src/android/jpeg/meson.build\n> b/src/android/jpeg/meson.build\n> > > index 08397a87..9162576b 100644\n> > > --- a/src/android/jpeg/meson.build\n> > > +++ b/src/android/jpeg/meson.build\n> > > @@ -1,8 +1,14 @@\n> > > # SPDX-License-Identifier: CC0-1.0\n> > >\n> > > android_hal_sources += files([\n> > > - 'encoder_libjpeg.cpp',\n> > > 'exif.cpp',\n> > > + 'encoder_libjpeg.cpp',\n> > > + 'thumbnailer.cpp',\n> > > 'post_processor_jpeg.cpp',\n> > > - 'thumbnailer.cpp'\n> > > ])\n> >\n> > This doesn't seem right, or needed. I'll fix this locally and push the\n> > series after running it through CTS.\n>\n> Feel free to run again, but I ran this through CTS earlier:\n>\n> -\n> https://results.uk.libcamera.org/0.0.4+10-53502641/2023.02.09_15.09.10/test_result.xml\n>\n> 8 failures, but not a regression - so this is OK with me.\n>\n> --\n> Kieran\n>\n>\n> >\n> > > +\n> > > +platform = get_option('android_platform')\n> > > +if platform == 'cros'\n> > > + android_hal_sources += files(['encoder_jea.cpp'])\n> > > + android_deps += [dependency('libcros_camera')]\n> > > +endif\n> > > diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n> b/src/android/jpeg/post_processor_jpeg.cpp\n> > > index 3e676eb4..40261652 100644\n> > > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > > @@ -12,7 +12,11 @@\n> > > #include \"../camera_device.h\"\n> > > #include \"../camera_metadata.h\"\n> > > #include \"../camera_request.h\"\n> > > +#if defined(OS_CHROMEOS)\n> > > +#include \"encoder_jea.h\"\n> > > +#else /* !defined(OS_CHROMEOS) */\n> > > #include \"encoder_libjpeg.h\"\n> > > +#endif\n> > > #include \"exif.h\"\n> > >\n> > > #include \n> > > @@ -46,7 +50,11 @@ int PostProcessorJpeg::configure(const\n> StreamConfiguration &inCfg,\n> > >\n> > > thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> > >\n> > > +#if defined(OS_CHROMEOS)\n> > > + encoder_ = std::make_unique();\n> > > +#else /* !defined(OS_CHROMEOS) */\n> > > encoder_ = std::make_unique();\n> > > +#endif\n> > >\n> > > return encoder_->configure(inCfg);\n> > > }\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\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 84392BDB13\n\tfor ;\n\tTue, 14 Feb 2023 09:45:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CCAEA625F4;\n\tTue, 14 Feb 2023 10:45:57 +0100 (CET)","from mail-vs1-xe36.google.com (mail-vs1-xe36.google.com\n\t[IPv6:2607:f8b0:4864:20::e36])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 770E261EEC\n\tfor ;\n\tTue, 14 Feb 2023 10:45:56 +0100 (CET)","by mail-vs1-xe36.google.com with SMTP id h19so15872749vsv.13\n\tfor ;\n\tTue, 14 Feb 2023 01:45:56 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1676367957;\n\tbh=ha3iG0LRI4ghgPuXtmf22cQg+I0BQHxhHESC4WZDGXE=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=HyXnFThtgI2l1Y6pZZaY0WWHh+g6MNhidJtnBTSFfDlKhsFWgMSOZWtJuCjUIgP5V\n\tr+Fd4ZJulAdhCU/AUqHOcAfKIv8eJjB76x85opj7quW3ahDzc28IUnSTFk0aXgMYss\n\tNz6pYap3DEjctRrrZro7VtXliQPi4u0iiS+xyTZL+9iLtKhOAk97Y9fEW1co6TLaw0\n\tyHw8YSTivhbIlcR2aI2gUjLxIwVceyNmkWf98xDXeKSdZCr+UnBrtH9psN1ydqarzH\n\t/1+k2fL+bXG8yjLrMK572VYIre4GTtV0JeR7MwrzRn3q8Ju5jb79O3TuufFgx/S3UG\n\ty48NKIrOMBQmA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1676367955;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=DslSLZFSLHReb6SP2wOYGNyjhxpeY9JTAY5eQXCEVlQ=;\n\tb=K+mlFITiKSjXueQbd5kiQb+U4LpYaFVFnAxRx+qIESHILwL2Bwd+EF4pjSi6UymVvQ\n\tFt6FjaTYR53HAs08PLO7RS4bAi8EKwYd5GwtdkLFWKPHcpS0Gl7DXATb7cwWB1qx5C63\n\tWvVcvhhn64pdpzgR15ldCShODzMKPvuvARR9M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"K+mlFITi\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1676367955;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=DslSLZFSLHReb6SP2wOYGNyjhxpeY9JTAY5eQXCEVlQ=;\n\tb=V7n0b5uHRK/tGN3cF/QWO6zLQxP3dZ9KQWQ3KPnID7jDAIWvXsQG9JkxGKGaTf3T0A\n\tv4Mzl1FQDJvhwRwkKpb9fCrOup0cj1beaAEE6WndaUbBUD+lDGJw+2yUpC7MY7NLAAKU\n\tAGDZJawPwWTpjLnF3afkxMq890WhdcR9mYbWjxfwSI12W8/OATlZPFgxZAk1VlCvzNUH\n\tHfAkPg6zYGBhVspCXa6UEL9lQK5XB7z2eNMcXYEkKVkj+KD+IArdXnYtqiKdZN2rm9h/\n\tqXI9B7/rltavlmoMJKMW1uQk8c6aN0Ftwr/BzNlTqEN9ZrL56dkqWKFG2T8ts79ye+iS\n\tTZNQ==","X-Gm-Message-State":"AO0yUKWtfJ0Ik8KomSgvEz94dFIvU9azxQK1sl8j3OZnGOrasoYyl0pX\n\tChhs8D2iltzRxw+8qSfRvtVrXY4FKKaXa8MVGQW2aRrY6WxruA==","X-Google-Smtp-Source":"AK7set96MRHYXbAGIi6DfHFyz4R/448HnQ51KhTdr2P2CVAiFySP9SeeDeQ+oNopVe411EJ+aIl3nDdqxivkgEiObSw=","X-Received":"by 2002:a67:e15c:0:b0:3fc:5b61:e42e with SMTP id\n\to28-20020a67e15c000000b003fc5b61e42emr227741vsl.5.1676367955176;\n\tTue, 14 Feb 2023 01:45:55 -0800 (PST)","MIME-Version":"1.0","References":"<20230208033319.1361109-1-chenghaoyang@google.com>\n\t<20230208033319.1361109-6-chenghaoyang@google.com>\n\t\n\t<167598286822.1526433.9971437652814373951@Monstersaurus>","In-Reply-To":"<167598286822.1526433.9971437652814373951@Monstersaurus>","Date":"Tue, 14 Feb 2023 17:45:44 +0800","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"0000000000007b4a9605f4a5d4d9\"","Subject":"Re: [libcamera-devel] [PATCH v10 5/5] android: jpeg: Add JEA\n\timplementation","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","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]