[{"id":26255,"web_url":"https://patchwork.libcamera.org/comment/26255/","msgid":"","date":"2023-01-18T08:00:04","subject":"Re: [libcamera-devel] [PATCH v9 8/8] 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":"Thanks Laurent!\nSome nits below. After these are addressed, the patches work fine on\nChromebook soraka.\n\nOn Mon, Jan 16, 2023 at 8:28 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\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 | 103 +++++++++++++++++++++++\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, 172 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 fb863b5f9aa9..71acb441b0d4 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 b/src/android/cros_mojo_token.h\n> new file mode 100644\n> index 000000000000..043c752a3997\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 000000000000..a7076039d0b7\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.cpp\n> @@ -0,0 +1,103 @@\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>\n\nPerhaps use |buffer| instead of |streamBuffer|, to be aligned with the base\nclass.\n\n\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 *>(streamBuffer->srcBuffer);\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> + return outDataSize;\n> +}\n> +\n> +int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,\n> + const libcamera::Size &targetSize,\n> + unsigned int quality,\n> + std::vector *thumbnail)\n> +{\n> + if (!jpegCompressor_)\n> + return -ENOTSUPP;\n> +\n>\n\nShould be `-ENOTSUP`.\n\n\n> + libcamera::MappedFrameBuffer frame(&source,\n> +\n> libcamera::MappedFrameBuffer::MapFlag::Read);\n> +\n> + if (frame.planes().empty())\n> + return -EINVAL;\n> +\n> + /* JEA needs consecutive memory. */\n> + unsigned long size = 0, index = 0;\n> + for (const auto &plane : frame.planes())\n> + size += plane.size();\n> +\n> + std::vector data(size);\n> + for (const auto &plane : frame.planes()) {\n> + memcpy(&data[index], plane.data(), plane.size());\n> + index += plane.size();\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> + return -EBUSY;\n> +\n> + thumbnail->resize(outDataSize);\n>\n\nAdd `return 0;` here\n\n\n> +}\n\ndiff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h\n> new file mode 100644\n> index 000000000000..2eba31c2f73c\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> + int 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 08397a87bc46..2b68f54c4228 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 2a22b4a88f4a..f7cc70de1ef1 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> Regards,\n>\n> Laurent Pinchart\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D6379BD87C\n\tfor ;\n\tWed, 18 Jan 2023 08:00:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D8B4625E4;\n\tWed, 18 Jan 2023 09:00:19 +0100 (CET)","from mail-vs1-xe34.google.com (mail-vs1-xe34.google.com\n\t[IPv6:2607:f8b0:4864:20::e34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CC1461EFD\n\tfor ;\n\tWed, 18 Jan 2023 09:00:16 +0100 (CET)","by mail-vs1-xe34.google.com with SMTP id l125so15524881vsc.2\n\tfor ;\n\tWed, 18 Jan 2023 00:00:16 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674028819;\n\tbh=/JjsoqnzO7xIinH0GIJTJl//I32/lb4m9j6XlfzV/Pc=;\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=dq3XcrdNS2VmfW5GcUczy5zlV8xQ0OLogy7nw6SWs6GY8lWFqqW2EOkyFkLKJSBqa\n\tBugxDIXolfns79OCQ58KYBjpf/1DAkSt5rwsDiCb+WsySrEb1h+yKR1DVrcsWlFSqw\n\tTvQkuv5EdQrhP3dOAPy1bpw7WVEkUQcDuxGvcTZ269jJ6rM8O7sjgv/JB6FONAzebK\n\tv1vBPQaCtmslmro4TOxydp6rzlRwqb8wgnk4vNZA3yb9c9sDm8Cp4fizsprpVxOjSx\n\tBliIvbp2TCJTnsaJHnExjIy+mhUXtHkRv6dRQKd1ihWW/txw30gCOFJMtf/sWXydNM\n\txr5QQ0xlHoK8w==","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=//nFsYQie5arsp+A6KKLjdJ0Rrib5NBHVVPq/pk39RQ=;\n\tb=aMUJGDtzFoFyf74PQW+YYT5hbATsJunwdhge/6Jajho3keaa3SZBF9+PQx6nxByoC2\n\t6PiZXk7ebjiyeOCufLKgeJI8E9/a0jI2/V+qauhZAAI+o8QH90qcoMmtBV6K967jnwxH\n\tXtpeSDIR7k4MrKa+2pDZJmlmvvotapJyEShKQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"aMUJGDtz\"; \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=//nFsYQie5arsp+A6KKLjdJ0Rrib5NBHVVPq/pk39RQ=;\n\tb=Fuamt8BNyDT2zyIZcIWQVnptezY2yWVFq0j7lpjZyR1B9D/84aN/r0Br8O4hUwe3sv\n\th5aTuHO8CjZx1rvd3ZT9bEKYntLvfpqK0mqYxlpgu0wBcrX4GutTA9AKIn6kWXjQQdwk\n\tROyyLdNsSo7f+9+4kg8w6ZERSmbcdMgFedV8+SomBZ0D8P4GfJC1u4ShmcczSi/1SBXx\n\th7yQIf3x76Tc7qbEb79Rivfm5MK7G45ajUtAHp7TTSes0T97AuEV1t8Uq3hnnjfVP2gc\n\ttsJGTM6dICMe0ffz39Z0iGv54C6Gc9QiUmAZw554l37efk83MDiqf3TKWdoGPI3wOwyB\n\tQieg==","X-Gm-Message-State":"AFqh2krO1gFT8iebkJ9kCOsuOIv+2EwUCFQw4TEz3cBoNrim+DGJPgcz\n\tHkN4Cc7EJmbCybrHConrXZHwXdVcKjJvRH7iOFS4rbJUCojN+aLA","X-Google-Smtp-Source":"AMrXdXtKywenxBoVdZFJeA4EcQtz4cHCRjYcyqViY814FhwMkA5rGWw7aAJSEyKR3fop5APmMgiLT4434SeIGIfXTRM=","X-Received":"by 2002:a67:f441:0:b0:3d3:e619:3c9f with SMTP id\n\tr1-20020a67f441000000b003d3e6193c9fmr709299vsn.6.1674028815278;\n\tWed, 18 Jan 2023 00:00:15 -0800 (PST)","MIME-Version":"1.0","References":"<20230116002808.16014-1-laurent.pinchart@ideasonboard.com>\n\t<20230116002808.16014-9-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20230116002808.16014-9-laurent.pinchart@ideasonboard.com>","Date":"Wed, 18 Jan 2023 16:00:04 +0800","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000e109b705f28534e0\"","Subject":"Re: [libcamera-devel] [PATCH v9 8/8] 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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]