[{"id":22791,"web_url":"https://patchwork.libcamera.org/comment/22791/","msgid":"<YmcjQHCsm9RoY5B+@pendragon.ideasonboard.com>","date":"2022-04-25T22:40:00","subject":"Re: [libcamera-devel] [PATCH 1/1] Add CrOS 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, and sorry for the late reply. Catching up with\ne-mail after travel is painful. Next time I'll try to get the whole\nworld to travel at the same time, maybe I'll get less e-mails :-)\n\nOn Wed, Apr 06, 2022 at 05:41:30PM +0800, Harvey Yang via libcamera-devel wrote:\n> This CL uses CrOS JpegCompressor with potential HW accelerator to do\n> JPEG encoding.\n> \n> As CrOS JpegCompressor might need file descriptors to get the source\n> data and pass the jpeg result, this CL extends FrameBuffer in the\n> android source code as Android_FrameBuffer, which stores the\n> buffer_handle_t when constructing the frame buffer, and adds a\n> getter function to access it.\n> \n> This CL also redefines src/android/jpeg/encoder interfaces and adds\n> Encoder::generateThumbnail, which might also be accelerated by CrOS\n> HW. It simplifies PostProcessorJpeg's logic when generating the\n> thumbnail. The original implementation is then moved into the\n> EncoderLibJpeg::generateThumbnail.\n\nThis is missing a Signed-off-by line, see\nDocumentation/contributing.rst.\n\n> ---\n>  include/libcamera/framebuffer.h               |  3 +-\n>  src/android/android_framebuffer.cpp           | 32 ++++++++\n>  src/android/android_framebuffer.h             | 28 +++++++\n>  src/android/camera_device.cpp                 |  3 +-\n>  src/android/cros/camera3_hal.cpp              |  3 +\n>  src/android/frame_buffer_allocator.h          | 37 +++++----\n>  src/android/jpeg/cros_post_processor_jpeg.cpp | 14 ++++\n>  src/android/jpeg/encoder.h                    |  9 +-\n>  src/android/jpeg/encoder_jea.cpp              | 82 +++++++++++++++++++\n>  src/android/jpeg/encoder_jea.h                | 35 ++++++++\n>  src/android/jpeg/encoder_libjpeg.cpp          | 70 ++++++++++++++++\n>  src/android/jpeg/encoder_libjpeg.h            | 21 ++++-\n>  .../jpeg/generic_post_processor_jpeg.cpp      | 14 ++++\n>  src/android/jpeg/meson.build                  | 16 ++++\n>  src/android/jpeg/post_processor_jpeg.cpp      | 60 ++------------\n>  src/android/jpeg/post_processor_jpeg.h        | 11 +--\n>  src/android/meson.build                       |  6 +-\n>  .../mm/cros_frame_buffer_allocator.cpp        | 13 +--\n>  .../mm/generic_frame_buffer_allocator.cpp     | 11 +--\n\nThere are lots of changes here, making this hard to review. Could you\nplease split this patch in pieces, with one logical change by patch, and\nbundle them as a series ? Candidates are\n\n- Drop the final keyword from FrameBuffer and make the destructor\n  virtual\n- Add AndroidFrameBuffer and use it in the HAL (you could even split\n  that in two if desired, but bundling a new class with its user(s) can\n  make review easier, if the result isn't too big)\n- Rework the JPEG encoder API and implementation to prepare for the\n  needs of JEA\n- Add the JEA implementation\n\n>  19 files changed, 367 insertions(+), 101 deletions(-)\n>  create mode 100644 src/android/android_framebuffer.cpp\n>  create mode 100644 src/android/android_framebuffer.h\n>  create mode 100644 src/android/jpeg/cros_post_processor_jpeg.cpp\n>  create mode 100644 src/android/jpeg/encoder_jea.cpp\n>  create mode 100644 src/android/jpeg/encoder_jea.h\n>  create mode 100644 src/android/jpeg/generic_post_processor_jpeg.cpp\n>  create mode 100644 src/android/jpeg/meson.build\n> \n> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\n> index de172d97..c902cc18 100644\n> --- a/include/libcamera/framebuffer.h\n> +++ b/include/libcamera/framebuffer.h\n> @@ -46,7 +46,7 @@ private:\n>  \tstd::vector<Plane> planes_;\n>  };\n>  \n> -class FrameBuffer final : public Extensible\n> +class FrameBuffer : public Extensible\n>  {\n>  \tLIBCAMERA_DECLARE_PRIVATE()\n>  \n> @@ -61,6 +61,7 @@ public:\n>  \tFrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0);\n>  \tFrameBuffer(std::unique_ptr<Private> d,\n>  \t\t    const std::vector<Plane> &planes, unsigned int cookie = 0);\n> +\tvirtual ~FrameBuffer() {}\n>  \n>  \tconst std::vector<Plane> &planes() const { return planes_; }\n>  \tRequest *request() const;\n> diff --git a/src/android/android_framebuffer.cpp b/src/android/android_framebuffer.cpp\n> new file mode 100644\n> index 00000000..1ff7018e\n> --- /dev/null\n> +++ b/src/android/android_framebuffer.cpp\n> @@ -0,0 +1,32 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * android_framebuffer.cpp - Android Frame buffer handling\n> + */\n> +\n> +#include \"android_framebuffer.h\"\n> +\n> +#include <hardware/camera3.h>\n> +\n> +AndroidFrameBuffer::AndroidFrameBuffer(\n> +\tbuffer_handle_t handle,\n> +\tstd::unique_ptr<Private> d,\n> +\tconst std::vector<Plane> &planes,\n> +\tunsigned int cookie)\n> +\t: FrameBuffer(std::move(d), planes, cookie), handle_(handle)\n> +{\n> +}\n> +\n> +AndroidFrameBuffer::AndroidFrameBuffer(\n> +\tbuffer_handle_t handle,\n> +\tconst std::vector<Plane> &planes,\n> +\tunsigned int cookie)\n> +\t: FrameBuffer(planes, cookie), handle_(handle)\n> +{\n> +}\n> +\n> +buffer_handle_t AndroidFrameBuffer::getHandle() const\n> +{\n> +\treturn handle_;\n> +}\n> diff --git a/src/android/android_framebuffer.h b/src/android/android_framebuffer.h\n> new file mode 100644\n> index 00000000..49df9756\n> --- /dev/null\n> +++ b/src/android/android_framebuffer.h\n> @@ -0,0 +1,28 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * android_framebuffer.h - Android Frame buffer handling\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"libcamera/internal/framebuffer.h\"\n> +\n> +#include <hardware/camera3.h>\n> +\n> +class AndroidFrameBuffer final : public libcamera::FrameBuffer\n> +{\n> +public:\n> +\tAndroidFrameBuffer(\n> +\t\tbuffer_handle_t handle, std::unique_ptr<Private> d,\n> +\t\tconst std::vector<Plane> &planes,\n> +\t\tunsigned int cookie = 0);\n> +\tAndroidFrameBuffer(buffer_handle_t handle,\n> +\t\t\t   const std::vector<Plane> &planes,\n> +\t\t\t   unsigned int cookie = 0);\n> +\tbuffer_handle_t getHandle() const;\n> +\n> +private:\n> +\tbuffer_handle_t handle_ = nullptr;\n> +};\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 00d48471..643b4dee 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -25,6 +25,7 @@\n>  \n>  #include \"system/graphics.h\"\n>  \n> +#include \"android_framebuffer.h\"\n>  #include \"camera_buffer.h\"\n>  #include \"camera_hal_config.h\"\n>  #include \"camera_ops.h\"\n> @@ -754,7 +755,7 @@ CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer,\n>  \t\tplanes[i].length = buf.size(i);\n>  \t}\n>  \n> -\treturn std::make_unique<FrameBuffer>(planes);\n> +\treturn std::make_unique<AndroidFrameBuffer>(camera3buffer, planes);\n>  }\n>  \n>  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp\n> index fb863b5f..ea5577f0 100644\n> --- a/src/android/cros/camera3_hal.cpp\n> +++ b/src/android/cros/camera3_hal.cpp\n> @@ -9,8 +9,11 @@\n>  \n>  #include \"../camera_hal_manager.h\"\n>  \n> +cros::CameraMojoChannelManagerToken *g_cros_camera_token = nullptr;\n> +\n>  static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)\n>  {\n> +\tg_cros_camera_token = token;\n>  }\n>  \n>  static void tear_down()\n> diff --git a/src/android/frame_buffer_allocator.h b/src/android/frame_buffer_allocator.h\n> index 5d2eeda1..e26422a3 100644\n> --- a/src/android/frame_buffer_allocator.h\n> +++ b/src/android/frame_buffer_allocator.h\n> @@ -13,9 +13,10 @@\n>  #include <libcamera/base/class.h>\n>  \n>  #include <libcamera/camera.h>\n> -#include <libcamera/framebuffer.h>\n>  #include <libcamera/geometry.h>\n>  \n> +#include \"android_framebuffer.h\"\n> +\n>  class CameraDevice;\n>  \n>  class PlatformFrameBufferAllocator : libcamera::Extensible\n> @@ -31,25 +32,25 @@ public:\n>  \t * Note: The returned FrameBuffer needs to be destroyed before\n>  \t * PlatformFrameBufferAllocator is destroyed.\n>  \t */\n> -\tstd::unique_ptr<libcamera::FrameBuffer> allocate(\n> +\tstd::unique_ptr<AndroidFrameBuffer> allocate(\n>  \t\tint halPixelFormat, const libcamera::Size &size, uint32_t usage);\n>  };\n>  \n> -#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION\t\t\t\\\n> -PlatformFrameBufferAllocator::PlatformFrameBufferAllocator(\t\t\\\n> -\tCameraDevice *const cameraDevice)\t\t\t\t\\\n> -\t: Extensible(std::make_unique<Private>(cameraDevice))\t\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()\t\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -}\t\t\t\t\t\t\t\t\t\\\n> -std::unique_ptr<libcamera::FrameBuffer>\t\t\t\t\t\\\n> -PlatformFrameBufferAllocator::allocate(int halPixelFormat,\t\t\\\n> -\t\t\t\t       const libcamera::Size &size,\t\\\n> -\t\t\t\t       uint32_t usage)\t\t\t\\\n> -{\t\t\t\t\t\t\t\t\t\\\n> -\treturn _d()->allocate(halPixelFormat, size, usage);\t\t\\\n> -}\n> +#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION                        \\\n> +\tPlatformFrameBufferAllocator::PlatformFrameBufferAllocator(         \\\n> +\t\tCameraDevice *const cameraDevice)                           \\\n> +\t\t: Extensible(std::make_unique<Private>(cameraDevice))       \\\n> +\t{                                                                   \\\n> +\t}                                                                   \\\n> +\tPlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()       \\\n> +\t{                                                                   \\\n> +\t}                                                                   \\\n> +\tstd::unique_ptr<AndroidFrameBuffer>                                 \\\n> +\tPlatformFrameBufferAllocator::allocate(int halPixelFormat,          \\\n> +\t\t\t\t\t       const libcamera::Size &size, \\\n> +\t\t\t\t\t       uint32_t usage)              \\\n> +\t{                                                                   \\\n> +\t\treturn _d()->allocate(halPixelFormat, size, usage);         \\\n> +\t}\n>  \n>  #endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */\n> diff --git a/src/android/jpeg/cros_post_processor_jpeg.cpp b/src/android/jpeg/cros_post_processor_jpeg.cpp\n> new file mode 100644\n> index 00000000..7020f0d0\n> --- /dev/null\n> +++ b/src/android/jpeg/cros_post_processor_jpeg.cpp\n> @@ -0,0 +1,14 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * cros_post_processor_jpeg.cpp - CrOS JPEG Post Processor\n> + */\n> +\n> +#include \"encoder_jea.h\"\n> +#include \"post_processor_jpeg.h\"\n> +\n> +void PostProcessorJpeg::SetEncoder()\n> +{\n> +\tencoder_ = std::make_unique<EncoderJea>();\n> +}\n> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h\n> index b974d367..6d527d91 100644\n> --- a/src/android/jpeg/encoder.h\n> +++ b/src/android/jpeg/encoder.h\n> @@ -12,14 +12,19 @@\n>  #include <libcamera/framebuffer.h>\n>  #include <libcamera/stream.h>\n>  \n> +#include \"../camera_request.h\"\n> +\n>  class Encoder\n>  {\n>  public:\n>  \tvirtual ~Encoder() = default;\n>  \n>  \tvirtual int configure(const libcamera::StreamConfiguration &cfg) = 0;\n> -\tvirtual int encode(const libcamera::FrameBuffer &source,\n> -\t\t\t   libcamera::Span<uint8_t> destination,\n> +\tvirtual int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n>  \t\t\t   libcamera::Span<const uint8_t> exifData,\n>  \t\t\t   unsigned int quality) = 0;\n> +\tvirtual int 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<unsigned char> *thumbnail) = 0;\n>  };\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..838e8647\n> --- /dev/null\n> +++ b/src/android/jpeg/encoder_jea.cpp\n> @@ -0,0 +1,82 @@\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/base/log.h>\n> +\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +\n> +#include <cros-camera/camera_mojo_channel_manager_token.h>\n> +\n> +#include \"../android_framebuffer.h\"\n> +\n> +extern cros::CameraMojoChannelManagerToken *g_cros_camera_token;\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 (jpeg_compressor_.get())\n> +\t\treturn 0;\n> +\n> +\tif (g_cros_camera_token == nullptr)\n> +\t\treturn -ENOTSUP;\n> +\n> +\tjpeg_compressor_ = cros::JpegCompressor::GetInstance(g_cros_camera_token);\n> +\n> +\treturn 0;\n> +}\n> +\n> +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> +\t\t       libcamera::Span<const uint8_t> exifData,\n> +\t\t       unsigned int quality)\n> +{\n> +\tif (!jpeg_compressor_.get())\n> +\t\treturn -1;\n> +\n> +\tuint32_t out_data_size = 0;\n> +\n> +\tif (!jpeg_compressor_->CompressImageFromHandle(\n> +\t\t    dynamic_cast<const AndroidFrameBuffer *>(\n> +\t\t\t    streamBuffer->srcBuffer)\n> +\t\t\t    ->getHandle(),\n> +\t\t    *streamBuffer->camera3Buffer, size_.width, size_.height, quality,\n> +\t\t    exifData.data(), exifData.size(), &out_data_size)) {\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn out_data_size;\n> +}\n> +\n> +int 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<unsigned char> *thumbnail)\n> +{\n> +\tif (!jpeg_compressor_.get())\n> +\t\treturn -1;\n> +\n> +\tlibcamera::MappedFrameBuffer frame(&source, libcamera::MappedFrameBuffer::MapFlag::Read);\n> +\n> +\tif (frame.planes().empty())\n> +\t\treturn 0;\n> +\n> +\tuint32_t out_data_size = 0;\n> +\n> +\tif (!jpeg_compressor_->GenerateThumbnail(frame.planes()[0].data(),\n> +\t\t\t\t\t\t size_.width, size_.height, targetSize.width, targetSize.height,\n> +\t\t\t\t\t\t quality, thumbnail->size(), thumbnail->data(), &out_data_size)) {\n> +\t\treturn -1;\n> +\t}\n> +\n> +\treturn out_data_size;\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..d5c9f1f7\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 <libcamera/geometry.h>\n> +\n> +#include <cros-camera/jpeg_compressor.h>\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<const uint8_t> exifData,\n> +\t\t   unsigned int quality) override;\n> +\tint 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<unsigned char> *thumbnail) override;\n> +\n> +private:\n> +\tlibcamera::Size size_;\n> +\n> +\tstd::unique_ptr<cros::JpegCompressor> jpeg_compressor_;\n> +};\n> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index 21a3b33d..b5591e33 100644\n> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> @@ -24,6 +24,8 @@\n>  #include \"libcamera/internal/formats.h\"\n>  #include \"libcamera/internal/mapped_framebuffer.h\"\n>  \n> +#include \"../camera_buffer.h\"\n> +\n>  using namespace libcamera;\n>  \n>  LOG_DECLARE_CATEGORY(JPEG)\n> @@ -82,8 +84,17 @@ EncoderLibJpeg::~EncoderLibJpeg()\n>  }\n>  \n>  int EncoderLibJpeg::configure(const StreamConfiguration &cfg)\n> +{\n> +\tthumbnailer_.configure(cfg.size, cfg.pixelFormat);\n> +\tcfg_ = cfg;\n> +\n> +\treturn internalConfigure(cfg);\n> +}\n> +\n> +int EncoderLibJpeg::internalConfigure(const StreamConfiguration &cfg)\n>  {\n>  \tconst struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat);\n> +\n>  \tif (info.colorSpace == JCS_UNKNOWN)\n>  \t\treturn -ENOTSUP;\n>  \n> @@ -178,6 +189,65 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)\n>  \t}\n>  }\n>  \n> +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> +\t\t\t   libcamera::Span<const uint8_t> exifData,\n> +\t\t\t   unsigned int quality)\n> +{\n> +\tinternalConfigure(cfg_);\n> +\treturn encode(*streamBuffer->srcBuffer, streamBuffer->dstBuffer.get()->plane(0), exifData, quality);\n> +}\n> +\n> +int EncoderLibJpeg::generateThumbnail(\n> +\tconst libcamera::FrameBuffer &source,\n> +\tconst libcamera::Size &targetSize,\n> +\tunsigned int quality,\n> +\tstd::vector<unsigned char> *thumbnail)\n> +{\n> +\t/* Stores the raw scaled-down thumbnail bytes. */\n> +\tstd::vector<unsigned char> rawThumbnail;\n> +\n> +\tthumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> +\n> +\tStreamConfiguration thCfg;\n> +\tthCfg.size = targetSize;\n> +\tthCfg.pixelFormat = thumbnailer_.pixelFormat();\n> +\tint ret = internalConfigure(thCfg);\n> +\n> +\tif (!rawThumbnail.empty() && !ret) {\n> +\t\t/*\n> +\t\t * \\todo Avoid value-initialization of all elements of the\n> +\t\t * vector.\n> +\t\t */\n> +\t\tthumbnail->resize(rawThumbnail.size());\n> +\n> +\t\t/*\n> +\t\t * Split planes manually as the encoder expects a vector of\n> +\t\t * planes.\n> +\t\t *\n> +\t\t * \\todo Pass a vector of planes directly to\n> +\t\t * Thumbnailer::createThumbnailer above and remove the manual\n> +\t\t * planes split from here.\n> +\t\t */\n> +\t\tstd::vector<Span<uint8_t>> thumbnailPlanes;\n> +\t\tconst PixelFormatInfo &formatNV12 = PixelFormatInfo::info(formats::NV12);\n> +\t\tsize_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> +\t\tsize_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> +\t\tthumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });\n> +\t\tthumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize, uvPlaneSize });\n> +\n> +\t\tint jpeg_size = encode(thumbnailPlanes, *thumbnail, {}, quality);\n> +\t\tthumbnail->resize(jpeg_size);\n> +\n> +\t\tLOG(JPEG, Debug)\n> +\t\t\t<< \"Thumbnail compress returned \"\n> +\t\t\t<< jpeg_size << \" bytes\";\n> +\n> +\t\treturn jpeg_size;\n> +\t}\n> +\n> +\treturn -1;\n> +}\n> +\n>  int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,\n>  \t\t\t   Span<const uint8_t> exifData, unsigned int quality)\n>  {\n> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h\n> index 1b3ac067..56b27bae 100644\n> --- a/src/android/jpeg/encoder_libjpeg.h\n> +++ b/src/android/jpeg/encoder_libjpeg.h\n> @@ -15,6 +15,8 @@\n>  \n>  #include <jpeglib.h>\n>  \n> +#include \"thumbnailer.h\"\n> +\n>  class EncoderLibJpeg : public Encoder\n>  {\n>  public:\n> @@ -22,19 +24,32 @@ public:\n>  \t~EncoderLibJpeg();\n>  \n>  \tint configure(const libcamera::StreamConfiguration &cfg) override;\n> +\tint encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> +\t\t   libcamera::Span<const uint8_t> exifData,\n> +\t\t   unsigned int quality) override;\n> +\tint generateThumbnail(\n> +\t\tconst libcamera::FrameBuffer &source,\n> +\t\tconst libcamera::Size &targetSize,\n> +\t\tunsigned int quality,\n> +\t\tstd::vector<unsigned char> *thumbnail) override;\n> +\n> +private:\n> +\tint internalConfigure(const libcamera::StreamConfiguration &cfg);\n> +\n>  \tint encode(const libcamera::FrameBuffer &source,\n>  \t\t   libcamera::Span<uint8_t> destination,\n>  \t\t   libcamera::Span<const uint8_t> exifData,\n> -\t\t   unsigned int quality) override;\n> +\t\t   unsigned int quality);\n>  \tint encode(const std::vector<libcamera::Span<uint8_t>> &planes,\n>  \t\t   libcamera::Span<uint8_t> destination,\n>  \t\t   libcamera::Span<const uint8_t> exifData,\n>  \t\t   unsigned int quality);\n>  \n> -private:\n>  \tvoid compressRGB(const std::vector<libcamera::Span<uint8_t>> &planes);\n>  \tvoid compressNV(const std::vector<libcamera::Span<uint8_t>> &planes);\n>  \n> +\tlibcamera::StreamConfiguration cfg_;\n> +\n>  \tstruct jpeg_compress_struct compress_;\n>  \tstruct jpeg_error_mgr jerr_;\n>  \n> @@ -42,4 +57,6 @@ private:\n>  \n>  \tbool nv_;\n>  \tbool nvSwap_;\n> +\n> +\tThumbnailer thumbnailer_;\n>  };\n> diff --git a/src/android/jpeg/generic_post_processor_jpeg.cpp b/src/android/jpeg/generic_post_processor_jpeg.cpp\n> new file mode 100644\n> index 00000000..890f6972\n> --- /dev/null\n> +++ b/src/android/jpeg/generic_post_processor_jpeg.cpp\n> @@ -0,0 +1,14 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * generic_post_processor_jpeg.cpp - Generic JPEG Post Processor\n> + */\n> +\n> +#include \"encoder_libjpeg.h\"\n> +#include \"post_processor_jpeg.h\"\n> +\n> +void PostProcessorJpeg::SetEncoder()\n> +{\n> +\tencoder_ = std::make_unique<EncoderLibJpeg>();\n> +}\n> diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build\n> new file mode 100644\n> index 00000000..8606acc4\n> --- /dev/null\n> +++ b/src/android/jpeg/meson.build\n> @@ -0,0 +1,16 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +android_hal_sources += files([\n> +    'exif.cpp',\n> +    'post_processor_jpeg.cpp'])\n> +\n> +platform = get_option('android_platform')\n> +if platform == 'generic'\n> +    android_hal_sources += files(['encoder_libjpeg.cpp',\n> +                                  'generic_post_processor_jpeg.cpp',\n> +                                  'thumbnailer.cpp'])\n> +elif platform == 'cros'\n> +    android_hal_sources += files(['cros_post_processor_jpeg.cpp',\n> +                                  '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 d72ebc3c..7ceba60e 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -9,10 +9,10 @@\n>  \n>  #include <chrono>\n>  \n> +#include \"../android_framebuffer.h\"\n>  #include \"../camera_device.h\"\n>  #include \"../camera_metadata.h\"\n>  #include \"../camera_request.h\"\n> -#include \"encoder_libjpeg.h\"\n>  #include \"exif.h\"\n>  \n>  #include <libcamera/base/log.h>\n> @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n>  \n>  \tstreamSize_ = outCfg.size;\n>  \n> -\tthumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> -\n> -\tencoder_ = std::make_unique<EncoderLibJpeg>();\n> +\tSetEncoder();\n>  \n>  \treturn encoder_->configure(inCfg);\n>  }\n>  \n> -void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> -\t\t\t\t\t  const Size &targetSize,\n> -\t\t\t\t\t  unsigned int quality,\n> -\t\t\t\t\t  std::vector<unsigned char> *thumbnail)\n> -{\n> -\t/* Stores the raw scaled-down thumbnail bytes. */\n> -\tstd::vector<unsigned char> rawThumbnail;\n> -\n> -\tthumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> -\n> -\tStreamConfiguration thCfg;\n> -\tthCfg.size = targetSize;\n> -\tthCfg.pixelFormat = thumbnailer_.pixelFormat();\n> -\tint ret = thumbnailEncoder_.configure(thCfg);\n> -\n> -\tif (!rawThumbnail.empty() && !ret) {\n> -\t\t/*\n> -\t\t * \\todo Avoid value-initialization of all elements of the\n> -\t\t * vector.\n> -\t\t */\n> -\t\tthumbnail->resize(rawThumbnail.size());\n> -\n> -\t\t/*\n> -\t\t * Split planes manually as the encoder expects a vector of\n> -\t\t * planes.\n> -\t\t *\n> -\t\t * \\todo Pass a vector of planes directly to\n> -\t\t * Thumbnailer::createThumbnailer above and remove the manual\n> -\t\t * planes split from here.\n> -\t\t */\n> -\t\tstd::vector<Span<uint8_t>> thumbnailPlanes;\n> -\t\tconst PixelFormatInfo &formatNV12 = PixelFormatInfo::info(formats::NV12);\n> -\t\tsize_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> -\t\tsize_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> -\t\tthumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });\n> -\t\tthumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize, uvPlaneSize });\n> -\n> -\t\tint jpeg_size = thumbnailEncoder_.encode(thumbnailPlanes,\n> -\t\t\t\t\t\t\t *thumbnail, {}, quality);\n> -\t\tthumbnail->resize(jpeg_size);\n> -\n> -\t\tLOG(JPEG, Debug)\n> -\t\t\t<< \"Thumbnail compress returned \"\n> -\t\t\t<< jpeg_size << \" bytes\";\n> -\t}\n> -}\n> -\n>  void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)\n>  {\n>  \tASSERT(encoder_);\n> @@ -164,8 +115,8 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu\n>  \n>  \t\tif (thumbnailSize != Size(0, 0)) {\n>  \t\t\tstd::vector<unsigned char> thumbnail;\n> -\t\t\tgenerateThumbnail(source, thumbnailSize, quality, &thumbnail);\n> -\t\t\tif (!thumbnail.empty())\n> +\t\t\tret = encoder_->generateThumbnail(source, thumbnailSize, quality, &thumbnail);\n> +\t\t\tif (ret > 0 && !thumbnail.empty())\n>  \t\t\t\texif.setThumbnail(thumbnail, Exif::Compression::JPEG);\n>  \t\t}\n>  \n> @@ -194,8 +145,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu\n>  \tconst uint8_t quality = ret ? *entry.data.u8 : 95;\n>  \tresultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);\n>  \n> -\tint jpeg_size = encoder_->encode(source, destination->plane(0),\n> -\t\t\t\t\t exif.data(), quality);\n> +\tint jpeg_size = encoder_->encode(streamBuffer, exif.data(), quality);\n>  \tif (jpeg_size < 0) {\n>  \t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n>  \t\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> index 98309b01..a09f8798 100644\n> --- a/src/android/jpeg/post_processor_jpeg.h\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -8,11 +8,11 @@\n>  #pragma once\n>  \n>  #include \"../post_processor.h\"\n> -#include \"encoder_libjpeg.h\"\n> -#include \"thumbnailer.h\"\n>  \n>  #include <libcamera/geometry.h>\n>  \n> +#include \"encoder.h\"\n> +\n>  class CameraDevice;\n>  \n>  class PostProcessorJpeg : public PostProcessor\n> @@ -25,14 +25,9 @@ public:\n>  \tvoid process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override;\n>  \n>  private:\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<unsigned char> *thumbnail);\n> +\tvoid SetEncoder();\n>  \n>  \tCameraDevice *const cameraDevice_;\n>  \tstd::unique_ptr<Encoder> encoder_;\n>  \tlibcamera::Size streamSize_;\n> -\tEncoderLibJpeg thumbnailEncoder_;\n> -\tThumbnailer thumbnailer_;\n>  };\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 75b4bf20..026b8b3c 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -38,6 +38,7 @@ endif\n>  android_deps += [libyuv_dep]\n>  \n>  android_hal_sources = files([\n> +    'android_framebuffer.cpp',\n>      'camera3_hal.cpp',\n>      'camera_capabilities.cpp',\n>      'camera_device.cpp',\n> @@ -47,10 +48,6 @@ android_hal_sources = files([\n>      'camera_ops.cpp',\n>      'camera_request.cpp',\n>      'camera_stream.cpp',\n> -    'jpeg/encoder_libjpeg.cpp',\n> -    'jpeg/exif.cpp',\n> -    'jpeg/post_processor_jpeg.cpp',\n> -    'jpeg/thumbnailer.cpp',\n>      'yuv/post_processor_yuv.cpp'\n>  ])\n>  \n> @@ -58,6 +55,7 @@ android_cpp_args = []\n>  \n>  subdir('cros')\n>  subdir('mm')\n> +subdir('jpeg')\n>  \n>  android_camera_metadata_sources = files([\n>      'metadata/camera_metadata.c',\n> diff --git a/src/android/mm/cros_frame_buffer_allocator.cpp b/src/android/mm/cros_frame_buffer_allocator.cpp\n> index 52e8c180..163c5d75 100644\n> --- a/src/android/mm/cros_frame_buffer_allocator.cpp\n> +++ b/src/android/mm/cros_frame_buffer_allocator.cpp\n> @@ -14,6 +14,7 @@\n>  \n>  #include \"libcamera/internal/framebuffer.h\"\n>  \n> +#include \"../android_framebuffer.h\"\n>  #include \"../camera_device.h\"\n>  #include \"../frame_buffer_allocator.h\"\n>  #include \"cros-camera/camera_buffer_manager.h\"\n> @@ -47,11 +48,11 @@ public:\n>  \t{\n>  \t}\n>  \n> -\tstd::unique_ptr<libcamera::FrameBuffer>\n> +\tstd::unique_ptr<AndroidFrameBuffer>\n>  \tallocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);\n>  };\n>  \n> -std::unique_ptr<libcamera::FrameBuffer>\n> +std::unique_ptr<AndroidFrameBuffer>\n>  PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n>  \t\t\t\t\t\tconst libcamera::Size &size,\n>  \t\t\t\t\t\tuint32_t usage)\n> @@ -80,9 +81,11 @@ PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n>  \t\tplane.length = cros::CameraBufferManager::GetPlaneSize(handle, i);\n>  \t}\n>  \n> -\treturn std::make_unique<FrameBuffer>(\n> -\t\tstd::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),\n> -\t\tplanes);\n> +\tauto fb = std::make_unique<AndroidFrameBuffer>(handle,\n> +\t\t\t\t\t\t       std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),\n> +\t\t\t\t\t\t       planes);\n> +\n> +\treturn fb;\n>  }\n>  \n>  PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION\n> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp\n> index acb2fa2b..c79b7b10 100644\n> --- a/src/android/mm/generic_frame_buffer_allocator.cpp\n> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp\n> @@ -18,6 +18,7 @@\n>  #include <hardware/gralloc.h>\n>  #include <hardware/hardware.h>\n>  \n> +#include \"../android_framebuffer.h\"\n>  #include \"../camera_device.h\"\n>  #include \"../frame_buffer_allocator.h\"\n>  \n> @@ -77,7 +78,7 @@ public:\n>  \n>  \t~Private() override;\n>  \n> -\tstd::unique_ptr<libcamera::FrameBuffer>\n> +\tstd::unique_ptr<AndroidFrameBuffer>\n>  \tallocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);\n>  \n>  private:\n> @@ -92,7 +93,7 @@ PlatformFrameBufferAllocator::Private::~Private()\n>  \t\tgralloc_close(allocDevice_);\n>  }\n>  \n> -std::unique_ptr<libcamera::FrameBuffer>\n> +std::unique_ptr<AndroidFrameBuffer>\n>  PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n>  \t\t\t\t\t\tconst libcamera::Size &size,\n>  \t\t\t\t\t\tuint32_t usage)\n> @@ -135,9 +136,9 @@ PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n>  \t\toffset += planeSize;\n>  \t}\n>  \n> -\treturn std::make_unique<FrameBuffer>(\n> -\t\tstd::make_unique<GenericFrameBufferData>(allocDevice_, handle),\n> -\t\tplanes);\n> +\treturn std::make_unique<AndroidFrameBuffer>(handle,\n> +\t\t\t\t\t\t    std::make_unique<GenericFrameBufferData>(allocDevice_, handle),\n> +\t\t\t\t\t\t    planes);\n>  }\n>  \n>  PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4EC53C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 25 Apr 2022 22:40:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81D4565647;\n\tTue, 26 Apr 2022 00:40:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EBD6604AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Apr 2022 00:40:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 04018487;\n\tTue, 26 Apr 2022 00:40:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1650926403;\n\tbh=yQvrMK10Gb1Vt1swCQQJHxtGD8nRogWVO4Ix2v5+pjg=;\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=o/nSuaOP77jInjRDfU4VcuqO8Ej4wOrXCwcdEeUmAdnHBq0p1c6cDqKdCFmpyC8G9\n\tSpkY62t3v+gP1MZA/xYjTrcgyf6ZIE8sOK0dqh3bBK8b3DbYGVA0tCUvE0QXptaZxU\n\tVRSFctuAbMsaE+BrwSu5VUatMpnqKFJW7/1NBa+m+I78PrWuMRa7qzdSsO/tPB+4ql\n\tNYxeONEK94cLutcJGtoNRrHsvQZnMeCgBo83pFPFWR1qOUMCwWQQekbn7Q+E3BKoEN\n\tvY6FRDMjemH26CckxJtlnWAje82dNdYBYo6aNyz4XjAQlxHivtYJHyGyiJrmb3n6X4\n\tdyemz96ioHv6Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1650926401;\n\tbh=yQvrMK10Gb1Vt1swCQQJHxtGD8nRogWVO4Ix2v5+pjg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YeXX/Wp59XiiCbt1zyYhmc7kUzRQZS01ccdtEfX3T+FtPOa2kTONH9DueOhGDf+Qm\n\tcAdoGmu+x62njIVRwk95UzeizsHDVgxlIzSIsrqKkpIejKB26K7o5Kgfi30X8pxl4L\n\tnDb4OUKCON/xzfldkh0HHu9CEBwvHRj9Ac+KsznM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YeXX/Wp5\"; dkim-atps=neutral","Date":"Tue, 26 Apr 2022 01:40:00 +0300","To":"Harvey Yang <chenghaoyang@chromium.org>","Message-ID":"<YmcjQHCsm9RoY5B+@pendragon.ideasonboard.com>","References":"<20220406094130.189862-1-chenghaoyang@chromium.org>\n\t<20220406094130.189862-2-chenghaoyang@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220406094130.189862-2-chenghaoyang@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH 1/1] Add CrOS JEA implementation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22793,"web_url":"https://patchwork.libcamera.org/comment/22793/","msgid":"<CAEB1ahu=m_OD-MtWz_xxD-nXdUR_DuK6oiv3=sp6ffv=VSkesg@mail.gmail.com>","date":"2022-04-26T09:18:29","subject":"Re: [libcamera-devel] [PATCH 1/1] Add CrOS JEA implementation","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Laurent!\nNo worries about the schedule. I'm not in a hurry :)\n\nI've splitted it into four CLs as you suggested in my PATCH v3 (Sorry for\nthe spam as I forgot to add the Signed-off-by line :'( )\nPlease check :)\n\nBR,\nHarvey\n\nOn Tue, Apr 26, 2022 at 6:40 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Harvey,\n>\n> Thank you for the patch, and sorry for the late reply. Catching up with\n> e-mail after travel is painful. Next time I'll try to get the whole\n> world to travel at the same time, maybe I'll get less e-mails :-)\n>\n> On Wed, Apr 06, 2022 at 05:41:30PM +0800, Harvey Yang via libcamera-devel\n> wrote:\n> > This CL uses CrOS JpegCompressor with potential HW accelerator to do\n> > JPEG encoding.\n> >\n> > As CrOS JpegCompressor might need file descriptors to get the source\n> > data and pass the jpeg result, this CL extends FrameBuffer in the\n> > android source code as Android_FrameBuffer, which stores the\n> > buffer_handle_t when constructing the frame buffer, and adds a\n> > getter function to access it.\n> >\n> > This CL also redefines src/android/jpeg/encoder interfaces and adds\n> > Encoder::generateThumbnail, which might also be accelerated by CrOS\n> > HW. It simplifies PostProcessorJpeg's logic when generating the\n> > thumbnail. The original implementation is then moved into the\n> > EncoderLibJpeg::generateThumbnail.\n>\n> This is missing a Signed-off-by line, see\n> Documentation/contributing.rst.\n>\n> > ---\n> >  include/libcamera/framebuffer.h               |  3 +-\n> >  src/android/android_framebuffer.cpp           | 32 ++++++++\n> >  src/android/android_framebuffer.h             | 28 +++++++\n> >  src/android/camera_device.cpp                 |  3 +-\n> >  src/android/cros/camera3_hal.cpp              |  3 +\n> >  src/android/frame_buffer_allocator.h          | 37 +++++----\n> >  src/android/jpeg/cros_post_processor_jpeg.cpp | 14 ++++\n> >  src/android/jpeg/encoder.h                    |  9 +-\n> >  src/android/jpeg/encoder_jea.cpp              | 82 +++++++++++++++++++\n> >  src/android/jpeg/encoder_jea.h                | 35 ++++++++\n> >  src/android/jpeg/encoder_libjpeg.cpp          | 70 ++++++++++++++++\n> >  src/android/jpeg/encoder_libjpeg.h            | 21 ++++-\n> >  .../jpeg/generic_post_processor_jpeg.cpp      | 14 ++++\n> >  src/android/jpeg/meson.build                  | 16 ++++\n> >  src/android/jpeg/post_processor_jpeg.cpp      | 60 ++------------\n> >  src/android/jpeg/post_processor_jpeg.h        | 11 +--\n> >  src/android/meson.build                       |  6 +-\n> >  .../mm/cros_frame_buffer_allocator.cpp        | 13 +--\n> >  .../mm/generic_frame_buffer_allocator.cpp     | 11 +--\n>\n> There are lots of changes here, making this hard to review. Could you\n> please split this patch in pieces, with one logical change by patch, and\n> bundle them as a series ? Candidates are\n>\n> - Drop the final keyword from FrameBuffer and make the destructor\n>   virtual\n> - Add AndroidFrameBuffer and use it in the HAL (you could even split\n>   that in two if desired, but bundling a new class with its user(s) can\n>   make review easier, if the result isn't too big)\n> - Rework the JPEG encoder API and implementation to prepare for the\n>   needs of JEA\n> - Add the JEA implementation\n>\n> >  19 files changed, 367 insertions(+), 101 deletions(-)\n> >  create mode 100644 src/android/android_framebuffer.cpp\n> >  create mode 100644 src/android/android_framebuffer.h\n> >  create mode 100644 src/android/jpeg/cros_post_processor_jpeg.cpp\n> >  create mode 100644 src/android/jpeg/encoder_jea.cpp\n> >  create mode 100644 src/android/jpeg/encoder_jea.h\n> >  create mode 100644 src/android/jpeg/generic_post_processor_jpeg.cpp\n> >  create mode 100644 src/android/jpeg/meson.build\n> >\n> > diff --git a/include/libcamera/framebuffer.h\n> b/include/libcamera/framebuffer.h\n> > index de172d97..c902cc18 100644\n> > --- a/include/libcamera/framebuffer.h\n> > +++ b/include/libcamera/framebuffer.h\n> > @@ -46,7 +46,7 @@ private:\n> >       std::vector<Plane> planes_;\n> >  };\n> >\n> > -class FrameBuffer final : public Extensible\n> > +class FrameBuffer : public Extensible\n> >  {\n> >       LIBCAMERA_DECLARE_PRIVATE()\n> >\n> > @@ -61,6 +61,7 @@ public:\n> >       FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie\n> = 0);\n> >       FrameBuffer(std::unique_ptr<Private> d,\n> >                   const std::vector<Plane> &planes, unsigned int cookie\n> = 0);\n> > +     virtual ~FrameBuffer() {}\n> >\n> >       const std::vector<Plane> &planes() const { return planes_; }\n> >       Request *request() const;\n> > diff --git a/src/android/android_framebuffer.cpp\n> b/src/android/android_framebuffer.cpp\n> > new file mode 100644\n> > index 00000000..1ff7018e\n> > --- /dev/null\n> > +++ b/src/android/android_framebuffer.cpp\n> > @@ -0,0 +1,32 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * android_framebuffer.cpp - Android Frame buffer handling\n> > + */\n> > +\n> > +#include \"android_framebuffer.h\"\n> > +\n> > +#include <hardware/camera3.h>\n> > +\n> > +AndroidFrameBuffer::AndroidFrameBuffer(\n> > +     buffer_handle_t handle,\n> > +     std::unique_ptr<Private> d,\n> > +     const std::vector<Plane> &planes,\n> > +     unsigned int cookie)\n> > +     : FrameBuffer(std::move(d), planes, cookie), handle_(handle)\n> > +{\n> > +}\n> > +\n> > +AndroidFrameBuffer::AndroidFrameBuffer(\n> > +     buffer_handle_t handle,\n> > +     const std::vector<Plane> &planes,\n> > +     unsigned int cookie)\n> > +     : FrameBuffer(planes, cookie), handle_(handle)\n> > +{\n> > +}\n> > +\n> > +buffer_handle_t AndroidFrameBuffer::getHandle() const\n> > +{\n> > +     return handle_;\n> > +}\n> > diff --git a/src/android/android_framebuffer.h\n> b/src/android/android_framebuffer.h\n> > new file mode 100644\n> > index 00000000..49df9756\n> > --- /dev/null\n> > +++ b/src/android/android_framebuffer.h\n> > @@ -0,0 +1,28 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * android_framebuffer.h - Android Frame buffer handling\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"libcamera/internal/framebuffer.h\"\n> > +\n> > +#include <hardware/camera3.h>\n> > +\n> > +class AndroidFrameBuffer final : public libcamera::FrameBuffer\n> > +{\n> > +public:\n> > +     AndroidFrameBuffer(\n> > +             buffer_handle_t handle, std::unique_ptr<Private> d,\n> > +             const std::vector<Plane> &planes,\n> > +             unsigned int cookie = 0);\n> > +     AndroidFrameBuffer(buffer_handle_t handle,\n> > +                        const std::vector<Plane> &planes,\n> > +                        unsigned int cookie = 0);\n> > +     buffer_handle_t getHandle() const;\n> > +\n> > +private:\n> > +     buffer_handle_t handle_ = nullptr;\n> > +};\n> > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > index 00d48471..643b4dee 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -25,6 +25,7 @@\n> >\n> >  #include \"system/graphics.h\"\n> >\n> > +#include \"android_framebuffer.h\"\n> >  #include \"camera_buffer.h\"\n> >  #include \"camera_hal_config.h\"\n> >  #include \"camera_ops.h\"\n> > @@ -754,7 +755,7 @@ CameraDevice::createFrameBuffer(const\n> buffer_handle_t camera3buffer,\n> >               planes[i].length = buf.size(i);\n> >       }\n> >\n> > -     return std::make_unique<FrameBuffer>(planes);\n> > +     return std::make_unique<AndroidFrameBuffer>(camera3buffer, planes);\n> >  }\n> >\n> >  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > diff --git a/src/android/cros/camera3_hal.cpp\n> b/src/android/cros/camera3_hal.cpp\n> > index fb863b5f..ea5577f0 100644\n> > --- a/src/android/cros/camera3_hal.cpp\n> > +++ b/src/android/cros/camera3_hal.cpp\n> > @@ -9,8 +9,11 @@\n> >\n> >  #include \"../camera_hal_manager.h\"\n> >\n> > +cros::CameraMojoChannelManagerToken *g_cros_camera_token = nullptr;\n> > +\n> >  static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken\n> *token)\n> >  {\n> > +     g_cros_camera_token = token;\n> >  }\n> >\n> >  static void tear_down()\n> > diff --git a/src/android/frame_buffer_allocator.h\n> b/src/android/frame_buffer_allocator.h\n> > index 5d2eeda1..e26422a3 100644\n> > --- a/src/android/frame_buffer_allocator.h\n> > +++ b/src/android/frame_buffer_allocator.h\n> > @@ -13,9 +13,10 @@\n> >  #include <libcamera/base/class.h>\n> >\n> >  #include <libcamera/camera.h>\n> > -#include <libcamera/framebuffer.h>\n> >  #include <libcamera/geometry.h>\n> >\n> > +#include \"android_framebuffer.h\"\n> > +\n> >  class CameraDevice;\n> >\n> >  class PlatformFrameBufferAllocator : libcamera::Extensible\n> > @@ -31,25 +32,25 @@ public:\n> >        * Note: The returned FrameBuffer needs to be destroyed before\n> >        * PlatformFrameBufferAllocator is destroyed.\n> >        */\n> > -     std::unique_ptr<libcamera::FrameBuffer> allocate(\n> > +     std::unique_ptr<AndroidFrameBuffer> allocate(\n> >               int halPixelFormat, const libcamera::Size &size, uint32_t\n> usage);\n> >  };\n> >\n> > -#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION                 \\\n> > -PlatformFrameBufferAllocator::PlatformFrameBufferAllocator(          \\\n> > -     CameraDevice *const cameraDevice)                               \\\n> > -     : Extensible(std::make_unique<Private>(cameraDevice))           \\\n> > -{                                                                    \\\n> > -}                                                                    \\\n> > -PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()\n>       \\\n> > -{                                                                    \\\n> > -}                                                                    \\\n> > -std::unique_ptr<libcamera::FrameBuffer>\n>       \\\n> > -PlatformFrameBufferAllocator::allocate(int halPixelFormat,           \\\n> > -                                    const libcamera::Size &size,     \\\n> > -                                    uint32_t usage)                  \\\n> > -{                                                                    \\\n> > -     return _d()->allocate(halPixelFormat, size, usage);             \\\n> > -}\n> > +#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION\n>     \\\n> > +     PlatformFrameBufferAllocator::PlatformFrameBufferAllocator(\n>  \\\n> > +             CameraDevice *const cameraDevice)\n>  \\\n> > +             : Extensible(std::make_unique<Private>(cameraDevice))\n>  \\\n> > +     {\n>  \\\n> > +     }\n>  \\\n> > +     PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator()\n>  \\\n> > +     {\n>  \\\n> > +     }\n>  \\\n> > +     std::unique_ptr<AndroidFrameBuffer>\n>  \\\n> > +     PlatformFrameBufferAllocator::allocate(int halPixelFormat,\n>   \\\n> > +                                            const libcamera::Size\n> &size, \\\n> > +                                            uint32_t usage)\n>   \\\n> > +     {\n>  \\\n> > +             return _d()->allocate(halPixelFormat, size, usage);\n>  \\\n> > +     }\n> >\n> >  #endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */\n> > diff --git a/src/android/jpeg/cros_post_processor_jpeg.cpp\n> b/src/android/jpeg/cros_post_processor_jpeg.cpp\n> > new file mode 100644\n> > index 00000000..7020f0d0\n> > --- /dev/null\n> > +++ b/src/android/jpeg/cros_post_processor_jpeg.cpp\n> > @@ -0,0 +1,14 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * cros_post_processor_jpeg.cpp - CrOS JPEG Post Processor\n> > + */\n> > +\n> > +#include \"encoder_jea.h\"\n> > +#include \"post_processor_jpeg.h\"\n> > +\n> > +void PostProcessorJpeg::SetEncoder()\n> > +{\n> > +     encoder_ = std::make_unique<EncoderJea>();\n> > +}\n> > diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h\n> > index b974d367..6d527d91 100644\n> > --- a/src/android/jpeg/encoder.h\n> > +++ b/src/android/jpeg/encoder.h\n> > @@ -12,14 +12,19 @@\n> >  #include <libcamera/framebuffer.h>\n> >  #include <libcamera/stream.h>\n> >\n> > +#include \"../camera_request.h\"\n> > +\n> >  class Encoder\n> >  {\n> >  public:\n> >       virtual ~Encoder() = default;\n> >\n> >       virtual int configure(const libcamera::StreamConfiguration &cfg) =\n> 0;\n> > -     virtual int encode(const libcamera::FrameBuffer &source,\n> > -                        libcamera::Span<uint8_t> destination,\n> > +     virtual int encode(Camera3RequestDescriptor::StreamBuffer\n> *streamBuffer,\n> >                          libcamera::Span<const uint8_t> exifData,\n> >                          unsigned int quality) = 0;\n> > +     virtual int generateThumbnail(const libcamera::FrameBuffer &source,\n> > +                                   const libcamera::Size &targetSize,\n> > +                                   unsigned int quality,\n> > +                                   std::vector<unsigned char>\n> *thumbnail) = 0;\n> >  };\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..838e8647\n> > --- /dev/null\n> > +++ b/src/android/jpeg/encoder_jea.cpp\n> > @@ -0,0 +1,82 @@\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/base/log.h>\n> > +\n> > +#include \"libcamera/internal/mapped_framebuffer.h\"\n> > +\n> > +#include <cros-camera/camera_mojo_channel_manager_token.h>\n> > +\n> > +#include \"../android_framebuffer.h\"\n> > +\n> > +extern cros::CameraMojoChannelManagerToken *g_cros_camera_token;\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 (jpeg_compressor_.get())\n> > +             return 0;\n> > +\n> > +     if (g_cros_camera_token == nullptr)\n> > +             return -ENOTSUP;\n> > +\n> > +     jpeg_compressor_ =\n> cros::JpegCompressor::GetInstance(g_cros_camera_token);\n> > +\n> > +     return 0;\n> > +}\n> > +\n> > +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer\n> *streamBuffer,\n> > +                    libcamera::Span<const uint8_t> exifData,\n> > +                    unsigned int quality)\n> > +{\n> > +     if (!jpeg_compressor_.get())\n> > +             return -1;\n> > +\n> > +     uint32_t out_data_size = 0;\n> > +\n> > +     if (!jpeg_compressor_->CompressImageFromHandle(\n> > +                 dynamic_cast<const AndroidFrameBuffer *>(\n> > +                         streamBuffer->srcBuffer)\n> > +                         ->getHandle(),\n> > +                 *streamBuffer->camera3Buffer, size_.width,\n> size_.height, quality,\n> > +                 exifData.data(), exifData.size(), &out_data_size)) {\n> > +             return -1;\n> > +     }\n> > +\n> > +     return out_data_size;\n> > +}\n> > +\n> > +int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,\n> > +                               const libcamera::Size &targetSize,\n> > +                               unsigned int quality,\n> > +                               std::vector<unsigned char> *thumbnail)\n> > +{\n> > +     if (!jpeg_compressor_.get())\n> > +             return -1;\n> > +\n> > +     libcamera::MappedFrameBuffer frame(&source,\n> libcamera::MappedFrameBuffer::MapFlag::Read);\n> > +\n> > +     if (frame.planes().empty())\n> > +             return 0;\n> > +\n> > +     uint32_t out_data_size = 0;\n> > +\n> > +     if (!jpeg_compressor_->GenerateThumbnail(frame.planes()[0].data(),\n> > +                                              size_.width,\n> size_.height, targetSize.width, targetSize.height,\n> > +                                              quality,\n> thumbnail->size(), thumbnail->data(), &out_data_size)) {\n> > +             return -1;\n> > +     }\n> > +\n> > +     return out_data_size;\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..d5c9f1f7\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 <libcamera/geometry.h>\n> > +\n> > +#include <cros-camera/jpeg_compressor.h>\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<const uint8_t> 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<unsigned char> *thumbnail)\n> override;\n> > +\n> > +private:\n> > +     libcamera::Size size_;\n> > +\n> > +     std::unique_ptr<cros::JpegCompressor> jpeg_compressor_;\n> > +};\n> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n> b/src/android/jpeg/encoder_libjpeg.cpp\n> > index 21a3b33d..b5591e33 100644\n> > --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > @@ -24,6 +24,8 @@\n> >  #include \"libcamera/internal/formats.h\"\n> >  #include \"libcamera/internal/mapped_framebuffer.h\"\n> >\n> > +#include \"../camera_buffer.h\"\n> > +\n> >  using namespace libcamera;\n> >\n> >  LOG_DECLARE_CATEGORY(JPEG)\n> > @@ -82,8 +84,17 @@ EncoderLibJpeg::~EncoderLibJpeg()\n> >  }\n> >\n> >  int EncoderLibJpeg::configure(const StreamConfiguration &cfg)\n> > +{\n> > +     thumbnailer_.configure(cfg.size, cfg.pixelFormat);\n> > +     cfg_ = cfg;\n> > +\n> > +     return internalConfigure(cfg);\n> > +}\n> > +\n> > +int EncoderLibJpeg::internalConfigure(const StreamConfiguration &cfg)\n> >  {\n> >       const struct JPEGPixelFormatInfo info =\n> findPixelInfo(cfg.pixelFormat);\n> > +\n> >       if (info.colorSpace == JCS_UNKNOWN)\n> >               return -ENOTSUP;\n> >\n> > @@ -178,6 +189,65 @@ void EncoderLibJpeg::compressNV(const\n> std::vector<Span<uint8_t>> &planes)\n> >       }\n> >  }\n> >\n> > +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer\n> *streamBuffer,\n> > +                        libcamera::Span<const uint8_t> exifData,\n> > +                        unsigned int quality)\n> > +{\n> > +     internalConfigure(cfg_);\n> > +     return encode(*streamBuffer->srcBuffer,\n> streamBuffer->dstBuffer.get()->plane(0), exifData, quality);\n> > +}\n> > +\n> > +int EncoderLibJpeg::generateThumbnail(\n> > +     const libcamera::FrameBuffer &source,\n> > +     const libcamera::Size &targetSize,\n> > +     unsigned int quality,\n> > +     std::vector<unsigned char> *thumbnail)\n> > +{\n> > +     /* Stores the raw scaled-down thumbnail bytes. */\n> > +     std::vector<unsigned char> rawThumbnail;\n> > +\n> > +     thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> > +\n> > +     StreamConfiguration thCfg;\n> > +     thCfg.size = targetSize;\n> > +     thCfg.pixelFormat = thumbnailer_.pixelFormat();\n> > +     int ret = internalConfigure(thCfg);\n> > +\n> > +     if (!rawThumbnail.empty() && !ret) {\n> > +             /*\n> > +              * \\todo Avoid value-initialization of all elements of the\n> > +              * vector.\n> > +              */\n> > +             thumbnail->resize(rawThumbnail.size());\n> > +\n> > +             /*\n> > +              * Split planes manually as the encoder expects a vector of\n> > +              * planes.\n> > +              *\n> > +              * \\todo Pass a vector of planes directly to\n> > +              * Thumbnailer::createThumbnailer above and remove the\n> manual\n> > +              * planes split from here.\n> > +              */\n> > +             std::vector<Span<uint8_t>> thumbnailPlanes;\n> > +             const PixelFormatInfo &formatNV12 =\n> PixelFormatInfo::info(formats::NV12);\n> > +             size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> > +             size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> > +             thumbnailPlanes.push_back({ rawThumbnail.data(),\n> yPlaneSize });\n> > +             thumbnailPlanes.push_back({ rawThumbnail.data() +\n> yPlaneSize, uvPlaneSize });\n> > +\n> > +             int jpeg_size = encode(thumbnailPlanes, *thumbnail, {},\n> quality);\n> > +             thumbnail->resize(jpeg_size);\n> > +\n> > +             LOG(JPEG, Debug)\n> > +                     << \"Thumbnail compress returned \"\n> > +                     << jpeg_size << \" bytes\";\n> > +\n> > +             return jpeg_size;\n> > +     }\n> > +\n> > +     return -1;\n> > +}\n> > +\n> >  int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t>\n> dest,\n> >                          Span<const uint8_t> exifData, unsigned int\n> quality)\n> >  {\n> > diff --git a/src/android/jpeg/encoder_libjpeg.h\n> b/src/android/jpeg/encoder_libjpeg.h\n> > index 1b3ac067..56b27bae 100644\n> > --- a/src/android/jpeg/encoder_libjpeg.h\n> > +++ b/src/android/jpeg/encoder_libjpeg.h\n> > @@ -15,6 +15,8 @@\n> >\n> >  #include <jpeglib.h>\n> >\n> > +#include \"thumbnailer.h\"\n> > +\n> >  class EncoderLibJpeg : public Encoder\n> >  {\n> >  public:\n> > @@ -22,19 +24,32 @@ public:\n> >       ~EncoderLibJpeg();\n> >\n> >       int configure(const libcamera::StreamConfiguration &cfg) override;\n> > +     int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> > +                libcamera::Span<const uint8_t> exifData,\n> > +                unsigned int quality) override;\n> > +     int generateThumbnail(\n> > +             const libcamera::FrameBuffer &source,\n> > +             const libcamera::Size &targetSize,\n> > +             unsigned int quality,\n> > +             std::vector<unsigned char> *thumbnail) override;\n> > +\n> > +private:\n> > +     int internalConfigure(const libcamera::StreamConfiguration &cfg);\n> > +\n> >       int encode(const libcamera::FrameBuffer &source,\n> >                  libcamera::Span<uint8_t> destination,\n> >                  libcamera::Span<const uint8_t> exifData,\n> > -                unsigned int quality) override;\n> > +                unsigned int quality);\n> >       int encode(const std::vector<libcamera::Span<uint8_t>> &planes,\n> >                  libcamera::Span<uint8_t> destination,\n> >                  libcamera::Span<const uint8_t> exifData,\n> >                  unsigned int quality);\n> >\n> > -private:\n> >       void compressRGB(const std::vector<libcamera::Span<uint8_t>>\n> &planes);\n> >       void compressNV(const std::vector<libcamera::Span<uint8_t>>\n> &planes);\n> >\n> > +     libcamera::StreamConfiguration cfg_;\n> > +\n> >       struct jpeg_compress_struct compress_;\n> >       struct jpeg_error_mgr jerr_;\n> >\n> > @@ -42,4 +57,6 @@ private:\n> >\n> >       bool nv_;\n> >       bool nvSwap_;\n> > +\n> > +     Thumbnailer thumbnailer_;\n> >  };\n> > diff --git a/src/android/jpeg/generic_post_processor_jpeg.cpp\n> b/src/android/jpeg/generic_post_processor_jpeg.cpp\n> > new file mode 100644\n> > index 00000000..890f6972\n> > --- /dev/null\n> > +++ b/src/android/jpeg/generic_post_processor_jpeg.cpp\n> > @@ -0,0 +1,14 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * generic_post_processor_jpeg.cpp - Generic JPEG Post Processor\n> > + */\n> > +\n> > +#include \"encoder_libjpeg.h\"\n> > +#include \"post_processor_jpeg.h\"\n> > +\n> > +void PostProcessorJpeg::SetEncoder()\n> > +{\n> > +     encoder_ = std::make_unique<EncoderLibJpeg>();\n> > +}\n> > diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build\n> > new file mode 100644\n> > index 00000000..8606acc4\n> > --- /dev/null\n> > +++ b/src/android/jpeg/meson.build\n> > @@ -0,0 +1,16 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +android_hal_sources += files([\n> > +    'exif.cpp',\n> > +    'post_processor_jpeg.cpp'])\n> > +\n> > +platform = get_option('android_platform')\n> > +if platform == 'generic'\n> > +    android_hal_sources += files(['encoder_libjpeg.cpp',\n> > +                                  'generic_post_processor_jpeg.cpp',\n> > +                                  'thumbnailer.cpp'])\n> > +elif platform == 'cros'\n> > +    android_hal_sources += files(['cros_post_processor_jpeg.cpp',\n> > +                                  '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 d72ebc3c..7ceba60e 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -9,10 +9,10 @@\n> >\n> >  #include <chrono>\n> >\n> > +#include \"../android_framebuffer.h\"\n> >  #include \"../camera_device.h\"\n> >  #include \"../camera_metadata.h\"\n> >  #include \"../camera_request.h\"\n> > -#include \"encoder_libjpeg.h\"\n> >  #include \"exif.h\"\n> >\n> >  #include <libcamera/base/log.h>\n> > @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const\n> StreamConfiguration &inCfg,\n> >\n> >       streamSize_ = outCfg.size;\n> >\n> > -     thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> > -\n> > -     encoder_ = std::make_unique<EncoderLibJpeg>();\n> > +     SetEncoder();\n> >\n> >       return encoder_->configure(inCfg);\n> >  }\n> >\n> > -void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> > -                                       const Size &targetSize,\n> > -                                       unsigned int quality,\n> > -                                       std::vector<unsigned char>\n> *thumbnail)\n> > -{\n> > -     /* Stores the raw scaled-down thumbnail bytes. */\n> > -     std::vector<unsigned char> rawThumbnail;\n> > -\n> > -     thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> > -\n> > -     StreamConfiguration thCfg;\n> > -     thCfg.size = targetSize;\n> > -     thCfg.pixelFormat = thumbnailer_.pixelFormat();\n> > -     int ret = thumbnailEncoder_.configure(thCfg);\n> > -\n> > -     if (!rawThumbnail.empty() && !ret) {\n> > -             /*\n> > -              * \\todo Avoid value-initialization of all elements of the\n> > -              * vector.\n> > -              */\n> > -             thumbnail->resize(rawThumbnail.size());\n> > -\n> > -             /*\n> > -              * Split planes manually as the encoder expects a vector of\n> > -              * planes.\n> > -              *\n> > -              * \\todo Pass a vector of planes directly to\n> > -              * Thumbnailer::createThumbnailer above and remove the\n> manual\n> > -              * planes split from here.\n> > -              */\n> > -             std::vector<Span<uint8_t>> thumbnailPlanes;\n> > -             const PixelFormatInfo &formatNV12 =\n> PixelFormatInfo::info(formats::NV12);\n> > -             size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> > -             size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> > -             thumbnailPlanes.push_back({ rawThumbnail.data(),\n> yPlaneSize });\n> > -             thumbnailPlanes.push_back({ rawThumbnail.data() +\n> yPlaneSize, uvPlaneSize });\n> > -\n> > -             int jpeg_size = thumbnailEncoder_.encode(thumbnailPlanes,\n> > -                                                      *thumbnail, {},\n> quality);\n> > -             thumbnail->resize(jpeg_size);\n> > -\n> > -             LOG(JPEG, Debug)\n> > -                     << \"Thumbnail compress returned \"\n> > -                     << jpeg_size << \" bytes\";\n> > -     }\n> > -}\n> > -\n> >  void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer\n> *streamBuffer)\n> >  {\n> >       ASSERT(encoder_);\n> > @@ -164,8 +115,8 @@ void\n> PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu\n> >\n> >               if (thumbnailSize != Size(0, 0)) {\n> >                       std::vector<unsigned char> thumbnail;\n> > -                     generateThumbnail(source, thumbnailSize, quality,\n> &thumbnail);\n> > -                     if (!thumbnail.empty())\n> > +                     ret = encoder_->generateThumbnail(source,\n> thumbnailSize, quality, &thumbnail);\n> > +                     if (ret > 0 && !thumbnail.empty())\n> >                               exif.setThumbnail(thumbnail,\n> Exif::Compression::JPEG);\n> >               }\n> >\n> > @@ -194,8 +145,7 @@ void\n> PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu\n> >       const uint8_t quality = ret ? *entry.data.u8 : 95;\n> >       resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);\n> >\n> > -     int jpeg_size = encoder_->encode(source, destination->plane(0),\n> > -                                      exif.data(), quality);\n> > +     int jpeg_size = encoder_->encode(streamBuffer, exif.data(),\n> quality);\n> >       if (jpeg_size < 0) {\n> >               LOG(JPEG, Error) << \"Failed to encode stream image\";\n> >               processComplete.emit(streamBuffer,\n> PostProcessor::Status::Error);\n> > diff --git a/src/android/jpeg/post_processor_jpeg.h\n> b/src/android/jpeg/post_processor_jpeg.h\n> > index 98309b01..a09f8798 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.h\n> > +++ b/src/android/jpeg/post_processor_jpeg.h\n> > @@ -8,11 +8,11 @@\n> >  #pragma once\n> >\n> >  #include \"../post_processor.h\"\n> > -#include \"encoder_libjpeg.h\"\n> > -#include \"thumbnailer.h\"\n> >\n> >  #include <libcamera/geometry.h>\n> >\n> > +#include \"encoder.h\"\n> > +\n> >  class CameraDevice;\n> >\n> >  class PostProcessorJpeg : public PostProcessor\n> > @@ -25,14 +25,9 @@ public:\n> >       void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)\n> override;\n> >\n> >  private:\n> > -     void generateThumbnail(const libcamera::FrameBuffer &source,\n> > -                            const libcamera::Size &targetSize,\n> > -                            unsigned int quality,\n> > -                            std::vector<unsigned char> *thumbnail);\n> > +     void SetEncoder();\n> >\n> >       CameraDevice *const cameraDevice_;\n> >       std::unique_ptr<Encoder> encoder_;\n> >       libcamera::Size streamSize_;\n> > -     EncoderLibJpeg thumbnailEncoder_;\n> > -     Thumbnailer thumbnailer_;\n> >  };\n> > diff --git a/src/android/meson.build b/src/android/meson.build\n> > index 75b4bf20..026b8b3c 100644\n> > --- a/src/android/meson.build\n> > +++ b/src/android/meson.build\n> > @@ -38,6 +38,7 @@ endif\n> >  android_deps += [libyuv_dep]\n> >\n> >  android_hal_sources = files([\n> > +    'android_framebuffer.cpp',\n> >      'camera3_hal.cpp',\n> >      'camera_capabilities.cpp',\n> >      'camera_device.cpp',\n> > @@ -47,10 +48,6 @@ android_hal_sources = files([\n> >      'camera_ops.cpp',\n> >      'camera_request.cpp',\n> >      'camera_stream.cpp',\n> > -    'jpeg/encoder_libjpeg.cpp',\n> > -    'jpeg/exif.cpp',\n> > -    'jpeg/post_processor_jpeg.cpp',\n> > -    'jpeg/thumbnailer.cpp',\n> >      'yuv/post_processor_yuv.cpp'\n> >  ])\n> >\n> > @@ -58,6 +55,7 @@ android_cpp_args = []\n> >\n> >  subdir('cros')\n> >  subdir('mm')\n> > +subdir('jpeg')\n> >\n> >  android_camera_metadata_sources = files([\n> >      'metadata/camera_metadata.c',\n> > diff --git a/src/android/mm/cros_frame_buffer_allocator.cpp\n> b/src/android/mm/cros_frame_buffer_allocator.cpp\n> > index 52e8c180..163c5d75 100644\n> > --- a/src/android/mm/cros_frame_buffer_allocator.cpp\n> > +++ b/src/android/mm/cros_frame_buffer_allocator.cpp\n> > @@ -14,6 +14,7 @@\n> >\n> >  #include \"libcamera/internal/framebuffer.h\"\n> >\n> > +#include \"../android_framebuffer.h\"\n> >  #include \"../camera_device.h\"\n> >  #include \"../frame_buffer_allocator.h\"\n> >  #include \"cros-camera/camera_buffer_manager.h\"\n> > @@ -47,11 +48,11 @@ public:\n> >       {\n> >       }\n> >\n> > -     std::unique_ptr<libcamera::FrameBuffer>\n> > +     std::unique_ptr<AndroidFrameBuffer>\n> >       allocate(int halPixelFormat, const libcamera::Size &size, uint32_t\n> usage);\n> >  };\n> >\n> > -std::unique_ptr<libcamera::FrameBuffer>\n> > +std::unique_ptr<AndroidFrameBuffer>\n> >  PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n> >                                               const libcamera::Size\n> &size,\n> >                                               uint32_t usage)\n> > @@ -80,9 +81,11 @@ PlatformFrameBufferAllocator::Private::allocate(int\n> halPixelFormat,\n> >               plane.length =\n> cros::CameraBufferManager::GetPlaneSize(handle, i);\n> >       }\n> >\n> > -     return std::make_unique<FrameBuffer>(\n> > -\n>  std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),\n> > -             planes);\n> > +     auto fb = std::make_unique<AndroidFrameBuffer>(handle,\n> > +\n> std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),\n> > +                                                    planes);\n> > +\n> > +     return fb;\n> >  }\n> >\n> >  PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION\n> > diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp\n> b/src/android/mm/generic_frame_buffer_allocator.cpp\n> > index acb2fa2b..c79b7b10 100644\n> > --- a/src/android/mm/generic_frame_buffer_allocator.cpp\n> > +++ b/src/android/mm/generic_frame_buffer_allocator.cpp\n> > @@ -18,6 +18,7 @@\n> >  #include <hardware/gralloc.h>\n> >  #include <hardware/hardware.h>\n> >\n> > +#include \"../android_framebuffer.h\"\n> >  #include \"../camera_device.h\"\n> >  #include \"../frame_buffer_allocator.h\"\n> >\n> > @@ -77,7 +78,7 @@ public:\n> >\n> >       ~Private() override;\n> >\n> > -     std::unique_ptr<libcamera::FrameBuffer>\n> > +     std::unique_ptr<AndroidFrameBuffer>\n> >       allocate(int halPixelFormat, const libcamera::Size &size, uint32_t\n> usage);\n> >\n> >  private:\n> > @@ -92,7 +93,7 @@ PlatformFrameBufferAllocator::Private::~Private()\n> >               gralloc_close(allocDevice_);\n> >  }\n> >\n> > -std::unique_ptr<libcamera::FrameBuffer>\n> > +std::unique_ptr<AndroidFrameBuffer>\n> >  PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,\n> >                                               const libcamera::Size\n> &size,\n> >                                               uint32_t usage)\n> > @@ -135,9 +136,9 @@ PlatformFrameBufferAllocator::Private::allocate(int\n> halPixelFormat,\n> >               offset += planeSize;\n> >       }\n> >\n> > -     return std::make_unique<FrameBuffer>(\n> > -             std::make_unique<GenericFrameBufferData>(allocDevice_,\n> handle),\n> > -             planes);\n> > +     return std::make_unique<AndroidFrameBuffer>(handle,\n> > +\n>  std::make_unique<GenericFrameBufferData>(allocDevice_, handle),\n> > +                                                 planes);\n> >  }\n> >\n> >  PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D392CC3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Apr 2022 09:18:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2CD4665647;\n\tTue, 26 Apr 2022 11:18:43 +0200 (CEST)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1304160431\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Apr 2022 11:18:41 +0200 (CEST)","by mail-lf1-x134.google.com with SMTP id w19so30793997lfu.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Apr 2022 02:18:41 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1650964723;\n\tbh=OZWBa+liAh3qoTVQ+wRto+FPFDwiE0gLSNsxOcrb/cA=;\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=QHzBwg3gYZQEqjIHfVTKfQ87SZgKHXCEEYUL0u20yOMEqPOOLrJ4VBLbGOIQn8VAf\n\tqV2BwAtVxXH3ZphNtSY0RmZ6vrnGBTMxxHA4gThQfagNOtUBxCJ3LPWfDhpJzKGV3W\n\t5AHPR6AS1qYtf1lzHcg8UKMXDx5jEpEA3gLKlccdKFK19PaSjpYrg5dj4rOntcZy3O\n\tL/VWhgauJWkXmY6W1kiRIRljjMZod2cD+C/ZUl+fRTlKivrZu375p1EMtT97CwTnbn\n\t/R67esoKMEmNq7KA8buNxCtLyQESMLmPK+kvsz8Rq+SoG/58k4Y8U8w2NFu4OUgDgJ\n\tKegvQ5WF+XY+g==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=GIPrxwuRTFJfAIhIutqGqSuYZ2YuPGzSI3tIDnEtAww=;\n\tb=C+l6OfBvvB0PgVKlNeigk3nwtQwG/UwaLeVUrL2hLE4QpsZpfpE8mMtbjEOXeFxF3d\n\tVbiUw/s33rGzOwpwnvgWZp22/IyKR7XacayukXWGonq8z0zjpUOWjKZg6kAPDWTWN14v\n\tT3lnFHWXUKnUg3o1NXPLYQPFHxO9oAQy3picU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"C+l6OfBv\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=GIPrxwuRTFJfAIhIutqGqSuYZ2YuPGzSI3tIDnEtAww=;\n\tb=hySZXhal71e/l1d6HBN2V47+PKow5qGaFYPeJX56pisbdWta6GCcgOu5Bg93pz+RcO\n\taifjT3jQ7UTfUNxkDEgHuQlpUN7Gdp5Nh7yPQdS1UUwQdYJtHnYeg2a4zoyVkMq4tHvS\n\t0zpZVCVjFGUZ8xhNapa4YhaQt3HOvghave8Vd+jh+pU/MfEwAmPnnujExOyNgOo+JRDM\n\tX67dz+o4S+/GRv6yzRDvYfaRk6azwu1jDg4D/RLvmZ9Fdt9zy6e4Vvu4BFlIOJknFKpq\n\t0us4j0Lgh+t7VKLUsEuUgsdsz0/HUSJENEEO/U4zl4jJdZDSp0GB+KHOIBTRQswlvuxE\n\tXypg==","X-Gm-Message-State":"AOAM530CIOK7Ye6fau+sC2HtUtsvvD6bNns8wO/j42y9M7ARq1zjMXud\n\thU83LibSHc2FIXvBXh9eh1qb45NyO1xKPShPlbgIFGNSLgESww==","X-Google-Smtp-Source":"ABdhPJyPM3+c+d1YCMkiSXr397zb2czwHgzpoA2B1p0wZ940n612hcwsXIgqEjvWJNnvPuSJu1ozs3XaxY5pRFEQbkc=","X-Received":"by 2002:a05:6512:2613:b0:448:5164:689d with SMTP id\n\tbt19-20020a056512261300b004485164689dmr15904328lfb.526.1650964720228;\n\tTue, 26 Apr 2022 02:18:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20220406094130.189862-1-chenghaoyang@chromium.org>\n\t<20220406094130.189862-2-chenghaoyang@chromium.org>\n\t<YmcjQHCsm9RoY5B+@pendragon.ideasonboard.com>","In-Reply-To":"<YmcjQHCsm9RoY5B+@pendragon.ideasonboard.com>","Date":"Tue, 26 Apr 2022 17:18:29 +0800","Message-ID":"<CAEB1ahu=m_OD-MtWz_xxD-nXdUR_DuK6oiv3=sp6ffv=VSkesg@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000afb7da05dd8b2d8e\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] Add CrOS JEA implementation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Cheng-Hao Yang via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Cheng-Hao Yang <chenghaoyang@chromium.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]