[{"id":26223,"web_url":"https://patchwork.libcamera.org/comment/26223/","msgid":"","date":"2023-01-16T00:08:46","subject":"Re: [libcamera-devel] [PATCH v8 7/7] Add JEA implementation","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, Dec 14, 2022 at 09:33:30AM +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\ns/LibJpeg/libjpeg/\n\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 | 105 +++++++++++++++++++++++\n> src/android/jpeg/encoder_jea.h | 35 ++++++++\n> src/android/jpeg/meson.build | 13 ++-\n> src/android/jpeg/post_processor_jpeg.cpp | 8 ++\n> 6 files changed, 174 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..139bde93\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.cpp\n> @@ -0,0 +1,105 @@\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 *streamBuffer,\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 = dynamic_cast(\n> +\t\tstreamBuffer->srcBuffer);\n\n\tconst HALFrameBuffer *fb =\n\t\tdynamic_cast(streamBuffer->srcBuffer);\n\n> +\n> +\tif (!jpegCompressor_->CompressImageFromHandle(fb->handle(),\n> +\t\t\t\t\t\t *streamBuffer->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\nIs this the right error code ? What can cause CompressImageFromHandle to\nfail ?\n\n> +\n> +\treturn outDataSize;\n> +}\n> +\n> +void EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,\n> +\t\t\t\t const libcamera::Size &targetSize,\n> +\t\t\t\t unsigned int quality,\n> +\t\t\t\t std::vector *thumbnail)\n\ncheckstyle.py indicates an incorrect alignment here.\n\n> +{\n> +\tif (!jpegCompressor_)\n> +\t\treturn;\n> +\n> +\tlibcamera::MappedFrameBuffer frame(&source,\n> +\t\t\t\t\t libcamera::MappedFrameBuffer::MapFlag::Read);\n> +\n> +\tif (frame.planes().empty())\n> +\t\treturn;\n> +\n> +\t// JEA needs consecutive memory.\n\nC-style comment:\n\n\t/* JEA needs consecutive memory. */\n\nSame in two locations below.\n\n> +\tunsigned long size = 0, index = 0;\n> +\tfor (const auto& plane : frame.planes())\n\ncheckstyle.py indicates that this should be\n\n\tfor (const auto &plane : frame.planes())\n\n> +\t\tsize += plane.size();\n> +\n> +\tstd::vector data(size);\n> +\tfor (const auto& plane : frame.planes()) {\n\nSame here.\n\n> +\t\tmemcpy(&data[index], plane.data(), plane.size());\n> +\t\tindex += plane.size();\n> +\t}\n\nThat's awful :-( A full-frame memcpy is very costly. Could the JEA API\nbe fixed to handle multi-planar formats better ? That's of course not a\nblocker for this series.\n\nI'm curious if memcpy() + JEA is actually faster than using the CPU to\ngenerate the thumbnail.\n\n> +\n> +\tuint32_t outDataSize = 0;\n> +\n> +\t/*\n> +\t * Since the structure of the App1 segment is like:\n> +\t * 0xFF [1 byte marker] [2 bytes size] [data]\n> +\t * And it should not be larger than 64K.\n> +\t */\n> +\tconstexpr int kApp1MaxDataSize = 65532;\n> +\tthumbnail->resize(kApp1MaxDataSize);\n> +\n> +\tif (!jpegCompressor_->GenerateThumbnail(data.data(),\n> +\t\t\t\t\t\tsize_.width, size_.height,\n> +\t\t\t\t\t\ttargetSize.width,\n> +\t\t\t\t\t\ttargetSize.height, quality,\n> +\t\t\t\t\t\tthumbnail->size(),\n> +\t\t\t\t\t\tthumbnail->data(),\n> +\t\t\t\t\t\t&outDataSize)) {\n\nHow does the JEA compressor know what pixel format the source image uses\n?\n\n> +\t\tthumbnail->clear();\n> +\t\treturn;\n> +\t}\n> +\n> +\tthumbnail->resize(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..817b3202\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.h\n> @@ -0,0 +1,35 @@\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 *streamBuffer,\n> +\t\t libcamera::Span exifData,\n> +\t\t unsigned int quality) override;\n> +\tvoid generateThumbnail(const libcamera::FrameBuffer &source,\n> +\t\t\t const libcamera::Size &targetSize,\n> +\t\t\t unsigned int quality,\n> +\t\t\t std::vector *thumbnail) 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..2b68f54c 100644\n> --- a/src/android/jpeg/meson.build\n> +++ b/src/android/jpeg/meson.build\n> @@ -1,8 +1,17 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> android_hal_sources += files([\n> - 'encoder_libjpeg.cpp',\n> 'exif.cpp',\n> 'post_processor_jpeg.cpp',\n> - 'thumbnailer.cpp'\n> ])\n> +\n> +platform = get_option('android_platform')\n> +if platform == 'generic'\n> + android_hal_sources += files([\n> + 'encoder_libjpeg.cpp',\n> + 'thumbnailer.cpp'\n> + ])\n> +elif 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 918ab492..3424f8e7 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> @@ -44,7 +48,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> \n> \tstreamSize_ = outCfg.size;\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 A9C4AC3240\n\tfor ;\n\tMon, 16 Jan 2023 00:08:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E1A7D625E4;\n\tMon, 16 Jan 2023 01:08:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AF63625D0\n\tfor ;\n\tMon, 16 Jan 2023 01:08:47 +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 72665997;\n\tMon, 16 Jan 2023 01:08:46 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673827728;\n\tbh=XA45fkO2FFwoaFBE5OR1rzZODGiYaHbOzFazVwTn+tU=;\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=UrPKSKP+m3RNzvVCSB27WzPtuVKpXa8VivxOYeeR7CdNrFNc5ugXaTYOw+DA52qSi\n\tEd1/n2hiBugnX9CoUAbsokqulrGJ/Hg40gq0hb/1sAaiJaVpyxv0tS8/ifR7+/t5lD\n\tuPkmcC2zxNyck2n42HwBKg+FOYwhu2TWwfwvlE20+aWoof9Nc8UheV1VQYNwQ0VHwv\n\t4LueqsdyPm90AUFSvaPyYw5HFja6p77oXG2ZGOFIBbG0ZheOPyWq7T7Jk0qa5g/I2q\n\tcrU2D5mOIy8D+sZxfRsiI/3oD3xJOeamvKCFGIvNYLy4G1iYivh3dmFViZ6izLOcVi\n\t1hqqvzVu2dyNg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1673827726;\n\tbh=XA45fkO2FFwoaFBE5OR1rzZODGiYaHbOzFazVwTn+tU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CQLfWlulQEkcVl8cY5fCVAi722V+RjBfKQXZeXsdYrT43DY2ZYPeAIxKvO7xM/3KQ\n\tEf5LQ8LQ1vujJ6EcJuqOOwQaMZsirTf3w5jeIjswXobRzbIrYq06u1HjNqJxxg3vBe\n\t9sWkhQ9DTd9ZqyJRLJyiKsR+talK94dN9LWltWR4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CQLfWlul\"; dkim-atps=neutral","Date":"Mon, 16 Jan 2023 02:08:46 +0200","To":"Harvey Yang ","Message-ID":"","References":"<20221214093330.3345421-1-chenghaoyang@google.com>\n\t<20221214093330.3345421-8-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221214093330.3345421-8-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v8 7/7] Add JEA implementation","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":26253,"web_url":"https://patchwork.libcamera.org/comment/26253/","msgid":"","date":"2023-01-17T10:31:36","subject":"Re: [libcamera-devel] [PATCH v8 7/7] Add JEA implementation","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Laurent,\n\nThanks for your review and the updated patches!\n\nOn Mon, Jan 16, 2023 at 8:08 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Harvey,\n>\n> Thank you for the patch.\n>\n> On Wed, Dec 14, 2022 at 09:33:30AM +0000, Harvey Yang via libcamera-devel\n> wrote:\n> > From: Harvey Yang \n> >\n> > This patch adds JEA implementation to replace LibJpeg in CrOS platform,\n>\n> s/LibJpeg/libjpeg/\n>\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 | 105 +++++++++++++++++++++++\n> > src/android/jpeg/encoder_jea.h | 35 ++++++++\n> > src/android/jpeg/meson.build | 13 ++-\n> > src/android/jpeg/post_processor_jpeg.cpp | 8 ++\n> > 6 files changed, 174 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]] cros::CameraMojoChannelManagerToken\n> *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..139bde93\n> > --- /dev/null\n> > +++ b/src/android/jpeg/encoder_jea.cpp\n> > @@ -0,0 +1,105 @@\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\n> *streamBuffer,\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 = dynamic_cast(\n> > + streamBuffer->srcBuffer);\n>\n> const HALFrameBuffer *fb =\n> dynamic_cast *>(streamBuffer->srcBuffer);\n>\n> > +\n> > + if (!jpegCompressor_->CompressImageFromHandle(fb->handle(),\n> > +\n> *streamBuffer->camera3Buffer,\n> > + size_.width,\n> size_.height,\n> > + quality,\n> exifData.data(),\n> > + exifData.size(),\n> > + &outDataSize))\n> > + return -EBUSY;\n>\n> Is this the right error code ? What can cause CompressImageFromHandle to\n> fail ?\n>\n>\nThere might be multiple different possibilities, according to CrOS'\nimplementation\n[1]. Maybe |-EINVAL| is more accurate?\n\n[1]:\nhttps://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/camera/common/jpeg_compressor_impl.cc#141\n\n\n> > +\n> > + return outDataSize;\n> > +}\n> > +\n> > +void EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,\n> > + const libcamera::Size &targetSize,\n> > + unsigned int quality,\n> > + std::vector *thumbnail)\n>\n> checkstyle.py indicates an incorrect alignment here.\n>\n> > +{\n> > + if (!jpegCompressor_)\n> > + return;\n> > +\n> > + libcamera::MappedFrameBuffer frame(&source,\n> > +\n> libcamera::MappedFrameBuffer::MapFlag::Read);\n> > +\n> > + if (frame.planes().empty())\n> > + return;\n> > +\n> > + // JEA needs consecutive memory.\n>\n> C-style comment:\n>\n> /* JEA needs consecutive memory. */\n>\n> Same in two locations below.\n>\n> > + unsigned long size = 0, index = 0;\n> > + for (const auto& plane : frame.planes())\n>\n> checkstyle.py indicates that this should be\n>\n> for (const auto &plane : frame.planes())\n>\n> > + size += plane.size();\n> > +\n> > + std::vector data(size);\n> > + for (const auto& plane : frame.planes()) {\n>\n> Same here.\n>\n> > + memcpy(&data[index], plane.data(), plane.size());\n> > + index += plane.size();\n> > + }\n>\n> That's awful :-( A full-frame memcpy is very costly. Could the JEA API\n> be fixed to handle multi-planar formats better ? That's of course not a\n> blocker for this series.\n>\n>\nIt would actually fail here [2] if we don't make the memory consecutive.\nI don't know if it could be fixed properly though...\n\n[2]:\nhttps://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/camera/common/jpeg_compressor_impl.cc#310\n\n\n> I'm curious if memcpy() + JEA is actually faster than using the CPU to\n> generate the thumbnail.\n>\n>\nI reckon the answer is no... JEA actually generates the thumbnail in SW\ninstead of HW [3], as the size of the image would be small.\nWe could also fall back to using EncoderLibJpeg::Encoder for\ngenerateThumbnail (maybe just implement it in\nEncoder::generateThumbnail). WDYT?\n\n[3]:\nhttps://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/camera/common/jpeg_compressor_impl.cc#320\n\n> +\n> > + uint32_t outDataSize = 0;\n> > +\n> > + /*\n> > + * Since the structure of the App1 segment is like:\n> > + * 0xFF [1 byte marker] [2 bytes size] [data]\n> > + * And it should not be larger than 64K.\n> > + */\n> > + constexpr int kApp1MaxDataSize = 65532;\n> > + thumbnail->resize(kApp1MaxDataSize);\n> > +\n> > + if (!jpegCompressor_->GenerateThumbnail(data.data(),\n> > + size_.width, size_.height,\n> > + targetSize.width,\n> > + targetSize.height, quality,\n> > + thumbnail->size(),\n> > + thumbnail->data(),\n> > + &outDataSize)) {\n>\n> How does the JEA compressor know what pixel format the source image uses\n> ?\n>\n>\nI honestly don't know... Maybe it assumes the format to be NV12 [4]?\n\n[4]:\nhttps://chromium.googlesource.com/chromiumos/platform2/+/refs/heads/main/camera/common/jpeg_compressor_impl.cc#619\n\n\n> > + thumbnail->clear();\n> > + return;\n> > + }\n> > +\n> > + thumbnail->resize(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..817b3202\n> > --- /dev/null\n> > +++ b/src/android/jpeg/encoder_jea.h\n> > @@ -0,0 +1,35 @@\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 *streamBuffer,\n> > + libcamera::Span exifData,\n> > + unsigned int quality) override;\n> > + void generateThumbnail(const libcamera::FrameBuffer &source,\n> > + const libcamera::Size &targetSize,\n> > + unsigned int quality,\n> > + std::vector *thumbnail)\n> 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..2b68f54c 100644\n> > --- a/src/android/jpeg/meson.build\n> > +++ b/src/android/jpeg/meson.build\n> > @@ -1,8 +1,17 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > android_hal_sources += files([\n> > - 'encoder_libjpeg.cpp',\n> > 'exif.cpp',\n> > 'post_processor_jpeg.cpp',\n> > - 'thumbnailer.cpp'\n> > ])\n> > +\n> > +platform = get_option('android_platform')\n> > +if platform == 'generic'\n> > + android_hal_sources += files([\n> > + 'encoder_libjpeg.cpp',\n> > + 'thumbnailer.cpp'\n> > + ])\n> > +elif 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 918ab492..3424f8e7 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> > @@ -44,7 +48,11 @@ int PostProcessorJpeg::configure(const\n> StreamConfiguration &inCfg,\n> >\n> > streamSize_ = outCfg.size;\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 1D23FBD87C\n\tfor ;\n\tTue, 17 Jan 2023 10:31:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72D5C625E4;\n\tTue, 17 Jan 2023 11:31:50 +0100 (CET)","from mail-vk1-xa30.google.com (mail-vk1-xa30.google.com\n\t[IPv6:2607:f8b0:4864:20::a30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B509661EFE\n\tfor ;\n\tTue, 17 Jan 2023 11:31:48 +0100 (CET)","by mail-vk1-xa30.google.com with SMTP id l185so11368332vke.2\n\tfor ;\n\tTue, 17 Jan 2023 02:31:48 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673951510;\n\tbh=eSxFnf/mI7Gt8bmFW8EuV/skWVfQWF4oBl3tV1CzSO8=;\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=JY7Z2L943eLDvNi351htQNKf9lvODfIF1VFaQGJQB6cDIi4bCsM0G/MKqeQS7vhF/\n\tJN27OsgFNImLwgry+KWvYnbFb9xmHNYWSc4kEjhfRejLWrcDTcAVrxsypo5g02iDcL\n\tyI1O4h3pvorcoy40O33D/7T+1oFOKaoWaJeC923mykpuQQOyDKcOXVnG9UKifsVf4M\n\tZZssmefGIugEwQDTgQoIrjrATLFFJadDkraar0pJxoq5Wbh35I9qZTiePh862uMHmg\n\th+ydcwJdTPVNqI169Nyl19wFikodUgGGpzofzJGTmPmp2rhdua5u5Qe57JgxXrXAIy\n\tZOfF7TjDvO9mQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \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=8QNQNj773ZtIf39qLO6x4gz89eZM0PQIOODarF+E0IM=;\n\tb=SraKfpMmfJ0hN3I3o+gZTgoeN1Dt6AWSKTN78aPnCQG+cjQJBSJIo8Bvr+Opw9suNT\n\tOchrlC5TS2ihRyL+8q15ccUwR+z39zfgNxMOQ9+xxFbKpkYwaGRbOSOMc7+pkIPuVcGA\n\tg90m/Gw/coWKueop+pNTCn/1J9jZGmQQUkdQ4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"SraKfpMm\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=8QNQNj773ZtIf39qLO6x4gz89eZM0PQIOODarF+E0IM=;\n\tb=VaWduGvZRnIPmWvQ6UiNmdd/csK1x6+JXIYSJlU13eW//SyrnV91SWwleKJUtokqTM\n\t6Igb6ZibzFi+nVLSk9zjtbNktTgIZXJB/ZHp5oLvQK/mv+CaCAI7U8kvF+WdsMJ4alZd\n\tzk9n82axzNFcBHIhk/73F26zINo62dsdYs1QhMQ/ZIYXFqzQXszuyfMRH2eeu0TUF+zB\n\tKzekDqGBDmAQw3k13NNylpkROCK1IA+UgQ6QjQ7pLuHzRtpIlhrZ/x7py/UAZqSWyoer\n\t75nxXBoAsnZv8ziinAN7gkfaTo07ry5Bm3xF/zx7nUCszQ6v1mEIkQR61h9vhctjN2CF\n\tOwTA==","X-Gm-Message-State":"AFqh2krJQpLTUPH7pyMFAyCwsRihHG1vAEK9gs4qLT+BPXfeFdIYTC0E\n\tLbxrTrVWfZ0Kw0P4W9tjT3C4RYeynmXkQajooY3mteBYLMxVBscV","X-Google-Smtp-Source":"AMrXdXu9iWZhQUkXwzENVEvaxhR20NRRqb+n6m6fokh/FPzYn37u6LIJortHY98ZPURd7GrHy5gWfaThZ7J2wCYMvoM=","X-Received":"by 2002:ac5:c8a4:0:b0:3da:d34f:f82 with SMTP id\n\to4-20020ac5c8a4000000b003dad34f0f82mr342285vkl.33.1673951507438;\n\tTue, 17 Jan 2023 02:31:47 -0800 (PST)","MIME-Version":"1.0","References":"<20221214093330.3345421-1-chenghaoyang@google.com>\n\t<20221214093330.3345421-8-chenghaoyang@google.com>\n\t","In-Reply-To":"","Date":"Tue, 17 Jan 2023 18:31:36 +0800","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000f8f33e05f27334e0\"","Subject":"Re: [libcamera-devel] [PATCH v8 7/7] Add JEA implementation","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]