<div dir="ltr">Thanks Laurent!<div>No worries about the schedule. I'm not in a hurry :)</div><div><br><div>I've splitted it into four CLs as you suggested in my PATCH v3 (Sorry for the spam as I forgot to add the Signed-off-by line :'( )</div><div>Please check :)</div></div><div><br></div><div>BR,</div><div>Harvey</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 26, 2022 at 6:40 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey,<br>
<br>
Thank you for the patch, and sorry for the late reply. Catching up with<br>
e-mail after travel is painful. Next time I'll try to get the whole<br>
world to travel at the same time, maybe I'll get less e-mails :-)<br>
<br>
On Wed, Apr 06, 2022 at 05:41:30PM +0800, Harvey Yang via libcamera-devel wrote:<br>
> This CL uses CrOS JpegCompressor with potential HW accelerator to do<br>
> JPEG encoding.<br>
> <br>
> As CrOS JpegCompressor might need file descriptors to get the source<br>
> data and pass the jpeg result, this CL extends FrameBuffer in the<br>
> android source code as Android_FrameBuffer, which stores the<br>
> buffer_handle_t when constructing the frame buffer, and adds a<br>
> getter function to access it.<br>
> <br>
> This CL also redefines src/android/jpeg/encoder interfaces and adds<br>
> Encoder::generateThumbnail, which might also be accelerated by CrOS<br>
> HW. It simplifies PostProcessorJpeg's logic when generating the<br>
> thumbnail. The original implementation is then moved into the<br>
> EncoderLibJpeg::generateThumbnail.<br>
<br>
This is missing a Signed-off-by line, see<br>
Documentation/contributing.rst.<br>
<br>
> ---<br>
> include/libcamera/framebuffer.h | 3 +-<br>
> src/android/android_framebuffer.cpp | 32 ++++++++<br>
> src/android/android_framebuffer.h | 28 +++++++<br>
> src/android/camera_device.cpp | 3 +-<br>
> src/android/cros/camera3_hal.cpp | 3 +<br>
> src/android/frame_buffer_allocator.h | 37 +++++----<br>
> src/android/jpeg/cros_post_processor_jpeg.cpp | 14 ++++<br>
> src/android/jpeg/encoder.h | 9 +-<br>
> src/android/jpeg/encoder_jea.cpp | 82 +++++++++++++++++++<br>
> src/android/jpeg/encoder_jea.h | 35 ++++++++<br>
> src/android/jpeg/encoder_libjpeg.cpp | 70 ++++++++++++++++<br>
> src/android/jpeg/encoder_libjpeg.h | 21 ++++-<br>
> .../jpeg/generic_post_processor_jpeg.cpp | 14 ++++<br>
> src/android/jpeg/meson.build | 16 ++++<br>
> src/android/jpeg/post_processor_jpeg.cpp | 60 ++------------<br>
> src/android/jpeg/post_processor_jpeg.h | 11 +--<br>
> src/android/meson.build | 6 +-<br>
> .../mm/cros_frame_buffer_allocator.cpp | 13 +--<br>
> .../mm/generic_frame_buffer_allocator.cpp | 11 +--<br>
<br>
There are lots of changes here, making this hard to review. Could you<br>
please split this patch in pieces, with one logical change by patch, and<br>
bundle them as a series ? Candidates are<br>
<br>
- Drop the final keyword from FrameBuffer and make the destructor<br>
virtual<br>
- Add AndroidFrameBuffer and use it in the HAL (you could even split<br>
that in two if desired, but bundling a new class with its user(s) can<br>
make review easier, if the result isn't too big)<br>
- Rework the JPEG encoder API and implementation to prepare for the<br>
needs of JEA<br>
- Add the JEA implementation<br>
<br>
> 19 files changed, 367 insertions(+), 101 deletions(-)<br>
> create mode 100644 src/android/android_framebuffer.cpp<br>
> create mode 100644 src/android/android_framebuffer.h<br>
> create mode 100644 src/android/jpeg/cros_post_processor_jpeg.cpp<br>
> create mode 100644 src/android/jpeg/encoder_jea.cpp<br>
> create mode 100644 src/android/jpeg/encoder_jea.h<br>
> create mode 100644 src/android/jpeg/generic_post_processor_jpeg.cpp<br>
> create mode 100644 src/android/jpeg/meson.build<br>
> <br>
> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h<br>
> index de172d97..c902cc18 100644<br>
> --- a/include/libcamera/framebuffer.h<br>
> +++ b/include/libcamera/framebuffer.h<br>
> @@ -46,7 +46,7 @@ private:<br>
> std::vector<Plane> planes_;<br>
> };<br>
> <br>
> -class FrameBuffer final : public Extensible<br>
> +class FrameBuffer : public Extensible<br>
> {<br>
> LIBCAMERA_DECLARE_PRIVATE()<br>
> <br>
> @@ -61,6 +61,7 @@ public:<br>
> FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie = 0);<br>
> FrameBuffer(std::unique_ptr<Private> d,<br>
> const std::vector<Plane> &planes, unsigned int cookie = 0);<br>
> + virtual ~FrameBuffer() {}<br>
> <br>
> const std::vector<Plane> &planes() const { return planes_; }<br>
> Request *request() const;<br>
> diff --git a/src/android/android_framebuffer.cpp b/src/android/android_framebuffer.cpp<br>
> new file mode 100644<br>
> index 00000000..1ff7018e<br>
> --- /dev/null<br>
> +++ b/src/android/android_framebuffer.cpp<br>
> @@ -0,0 +1,32 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * android_framebuffer.cpp - Android Frame buffer handling<br>
> + */<br>
> +<br>
> +#include "android_framebuffer.h"<br>
> +<br>
> +#include <hardware/camera3.h><br>
> +<br>
> +AndroidFrameBuffer::AndroidFrameBuffer(<br>
> + buffer_handle_t handle,<br>
> + std::unique_ptr<Private> d,<br>
> + const std::vector<Plane> &planes,<br>
> + unsigned int cookie)<br>
> + : FrameBuffer(std::move(d), planes, cookie), handle_(handle)<br>
> +{<br>
> +}<br>
> +<br>
> +AndroidFrameBuffer::AndroidFrameBuffer(<br>
> + buffer_handle_t handle,<br>
> + const std::vector<Plane> &planes,<br>
> + unsigned int cookie)<br>
> + : FrameBuffer(planes, cookie), handle_(handle)<br>
> +{<br>
> +}<br>
> +<br>
> +buffer_handle_t AndroidFrameBuffer::getHandle() const<br>
> +{<br>
> + return handle_;<br>
> +}<br>
> diff --git a/src/android/android_framebuffer.h b/src/android/android_framebuffer.h<br>
> new file mode 100644<br>
> index 00000000..49df9756<br>
> --- /dev/null<br>
> +++ b/src/android/android_framebuffer.h<br>
> @@ -0,0 +1,28 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * android_framebuffer.h - Android Frame buffer handling<br>
> + */<br>
> +<br>
> +#pragma once<br>
> +<br>
> +#include "libcamera/internal/framebuffer.h"<br>
> +<br>
> +#include <hardware/camera3.h><br>
> +<br>
> +class AndroidFrameBuffer final : public libcamera::FrameBuffer<br>
> +{<br>
> +public:<br>
> + AndroidFrameBuffer(<br>
> + buffer_handle_t handle, std::unique_ptr<Private> d,<br>
> + const std::vector<Plane> &planes,<br>
> + unsigned int cookie = 0);<br>
> + AndroidFrameBuffer(buffer_handle_t handle,<br>
> + const std::vector<Plane> &planes,<br>
> + unsigned int cookie = 0);<br>
> + buffer_handle_t getHandle() const;<br>
> +<br>
> +private:<br>
> + buffer_handle_t handle_ = nullptr;<br>
> +};<br>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
> index 00d48471..643b4dee 100644<br>
> --- a/src/android/camera_device.cpp<br>
> +++ b/src/android/camera_device.cpp<br>
> @@ -25,6 +25,7 @@<br>
> <br>
> #include "system/graphics.h"<br>
> <br>
> +#include "android_framebuffer.h"<br>
> #include "camera_buffer.h"<br>
> #include "camera_hal_config.h"<br>
> #include "camera_ops.h"<br>
> @@ -754,7 +755,7 @@ CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer,<br>
> planes[i].length = buf.size(i);<br>
> }<br>
> <br>
> - return std::make_unique<FrameBuffer>(planes);<br>
> + return std::make_unique<AndroidFrameBuffer>(camera3buffer, planes);<br>
> }<br>
> <br>
> int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)<br>
> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp<br>
> index fb863b5f..ea5577f0 100644<br>
> --- a/src/android/cros/camera3_hal.cpp<br>
> +++ b/src/android/cros/camera3_hal.cpp<br>
> @@ -9,8 +9,11 @@<br>
> <br>
> #include "../camera_hal_manager.h"<br>
> <br>
> +cros::CameraMojoChannelManagerToken *g_cros_camera_token = nullptr;<br>
> +<br>
> static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)<br>
> {<br>
> + g_cros_camera_token = token;<br>
> }<br>
> <br>
> static void tear_down()<br>
> diff --git a/src/android/frame_buffer_allocator.h b/src/android/frame_buffer_allocator.h<br>
> index 5d2eeda1..e26422a3 100644<br>
> --- a/src/android/frame_buffer_allocator.h<br>
> +++ b/src/android/frame_buffer_allocator.h<br>
> @@ -13,9 +13,10 @@<br>
> #include <libcamera/base/class.h><br>
> <br>
> #include <libcamera/camera.h><br>
> -#include <libcamera/framebuffer.h><br>
> #include <libcamera/geometry.h><br>
> <br>
> +#include "android_framebuffer.h"<br>
> +<br>
> class CameraDevice;<br>
> <br>
> class PlatformFrameBufferAllocator : libcamera::Extensible<br>
> @@ -31,25 +32,25 @@ public:<br>
> * Note: The returned FrameBuffer needs to be destroyed before<br>
> * PlatformFrameBufferAllocator is destroyed.<br>
> */<br>
> - std::unique_ptr<libcamera::FrameBuffer> allocate(<br>
> + std::unique_ptr<AndroidFrameBuffer> allocate(<br>
> int halPixelFormat, const libcamera::Size &size, uint32_t usage);<br>
> };<br>
> <br>
> -#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION \<br>
> -PlatformFrameBufferAllocator::PlatformFrameBufferAllocator( \<br>
> - CameraDevice *const cameraDevice) \<br>
> - : Extensible(std::make_unique<Private>(cameraDevice)) \<br>
> -{ \<br>
> -} \<br>
> -PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator() \<br>
> -{ \<br>
> -} \<br>
> -std::unique_ptr<libcamera::FrameBuffer> \<br>
> -PlatformFrameBufferAllocator::allocate(int halPixelFormat, \<br>
> - const libcamera::Size &size, \<br>
> - uint32_t usage) \<br>
> -{ \<br>
> - return _d()->allocate(halPixelFormat, size, usage); \<br>
> -}<br>
> +#define PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION \<br>
> + PlatformFrameBufferAllocator::PlatformFrameBufferAllocator( \<br>
> + CameraDevice *const cameraDevice) \<br>
> + : Extensible(std::make_unique<Private>(cameraDevice)) \<br>
> + { \<br>
> + } \<br>
> + PlatformFrameBufferAllocator::~PlatformFrameBufferAllocator() \<br>
> + { \<br>
> + } \<br>
> + std::unique_ptr<AndroidFrameBuffer> \<br>
> + PlatformFrameBufferAllocator::allocate(int halPixelFormat, \<br>
> + const libcamera::Size &size, \<br>
> + uint32_t usage) \<br>
> + { \<br>
> + return _d()->allocate(halPixelFormat, size, usage); \<br>
> + }<br>
> <br>
> #endif /* __ANDROID_FRAME_BUFFER_ALLOCATOR_H__ */<br>
> diff --git a/src/android/jpeg/cros_post_processor_jpeg.cpp b/src/android/jpeg/cros_post_processor_jpeg.cpp<br>
> new file mode 100644<br>
> index 00000000..7020f0d0<br>
> --- /dev/null<br>
> +++ b/src/android/jpeg/cros_post_processor_jpeg.cpp<br>
> @@ -0,0 +1,14 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * cros_post_processor_jpeg.cpp - CrOS JPEG Post Processor<br>
> + */<br>
> +<br>
> +#include "encoder_jea.h"<br>
> +#include "post_processor_jpeg.h"<br>
> +<br>
> +void PostProcessorJpeg::SetEncoder()<br>
> +{<br>
> + encoder_ = std::make_unique<EncoderJea>();<br>
> +}<br>
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h<br>
> index b974d367..6d527d91 100644<br>
> --- a/src/android/jpeg/encoder.h<br>
> +++ b/src/android/jpeg/encoder.h<br>
> @@ -12,14 +12,19 @@<br>
> #include <libcamera/framebuffer.h><br>
> #include <libcamera/stream.h><br>
> <br>
> +#include "../camera_request.h"<br>
> +<br>
> class Encoder<br>
> {<br>
> public:<br>
> virtual ~Encoder() = default;<br>
> <br>
> virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;<br>
> - virtual int encode(const libcamera::FrameBuffer &source,<br>
> - libcamera::Span<uint8_t> destination,<br>
> + virtual int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> unsigned int quality) = 0;<br>
> + virtual int generateThumbnail(const libcamera::FrameBuffer &source,<br>
> + const libcamera::Size &targetSize,<br>
> + unsigned int quality,<br>
> + std::vector<unsigned char> *thumbnail) = 0;<br>
> };<br>
> diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp<br>
> new file mode 100644<br>
> index 00000000..838e8647<br>
> --- /dev/null<br>
> +++ b/src/android/jpeg/encoder_jea.cpp<br>
> @@ -0,0 +1,82 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * encoder_jea.cpp - JPEG encoding using CrOS JEA<br>
> + */<br>
> +<br>
> +#include "encoder_jea.h"<br>
> +<br>
> +#include <libcamera/base/log.h><br>
> +<br>
> +#include "libcamera/internal/mapped_framebuffer.h"<br>
> +<br>
> +#include <cros-camera/camera_mojo_channel_manager_token.h><br>
> +<br>
> +#include "../android_framebuffer.h"<br>
> +<br>
> +extern cros::CameraMojoChannelManagerToken *g_cros_camera_token;<br>
> +<br>
> +EncoderJea::EncoderJea() = default;<br>
> +<br>
> +EncoderJea::~EncoderJea() = default;<br>
> +<br>
> +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)<br>
> +{<br>
> + size_ = cfg.size;<br>
> +<br>
> + if (jpeg_compressor_.get())<br>
> + return 0;<br>
> +<br>
> + if (g_cros_camera_token == nullptr)<br>
> + return -ENOTSUP;<br>
> +<br>
> + jpeg_compressor_ = cros::JpegCompressor::GetInstance(g_cros_camera_token);<br>
> +<br>
> + return 0;<br>
> +}<br>
> +<br>
> +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality)<br>
> +{<br>
> + if (!jpeg_compressor_.get())<br>
> + return -1;<br>
> +<br>
> + uint32_t out_data_size = 0;<br>
> +<br>
> + if (!jpeg_compressor_->CompressImageFromHandle(<br>
> + dynamic_cast<const AndroidFrameBuffer *>(<br>
> + streamBuffer->srcBuffer)<br>
> + ->getHandle(),<br>
> + *streamBuffer->camera3Buffer, size_.width, size_.height, quality,<br>
> + exifData.data(), exifData.size(), &out_data_size)) {<br>
> + return -1;<br>
> + }<br>
> +<br>
> + return out_data_size;<br>
> +}<br>
> +<br>
> +int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,<br>
> + const libcamera::Size &targetSize,<br>
> + unsigned int quality,<br>
> + std::vector<unsigned char> *thumbnail)<br>
> +{<br>
> + if (!jpeg_compressor_.get())<br>
> + return -1;<br>
> +<br>
> + libcamera::MappedFrameBuffer frame(&source, libcamera::MappedFrameBuffer::MapFlag::Read);<br>
> +<br>
> + if (frame.planes().empty())<br>
> + return 0;<br>
> +<br>
> + uint32_t out_data_size = 0;<br>
> +<br>
> + if (!jpeg_compressor_->GenerateThumbnail(frame.planes()[0].data(),<br>
> + size_.width, size_.height, targetSize.width, targetSize.height,<br>
> + quality, thumbnail->size(), thumbnail->data(), &out_data_size)) {<br>
> + return -1;<br>
> + }<br>
> +<br>
> + return out_data_size;<br>
> +}<br>
> diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h<br>
> new file mode 100644<br>
> index 00000000..d5c9f1f7<br>
> --- /dev/null<br>
> +++ b/src/android/jpeg/encoder_jea.h<br>
> @@ -0,0 +1,35 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * encoder_jea.h - JPEG encoding using CrOS JEA<br>
> + */<br>
> +<br>
> +#pragma once<br>
> +<br>
> +#include <libcamera/geometry.h><br>
> +<br>
> +#include <cros-camera/jpeg_compressor.h><br>
> +<br>
> +#include "encoder.h"<br>
> +<br>
> +class EncoderJea : public Encoder<br>
> +{<br>
> +public:<br>
> + EncoderJea();<br>
> + ~EncoderJea();<br>
> +<br>
> + int configure(const libcamera::StreamConfiguration &cfg) override;<br>
> + int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality) override;<br>
> + int generateThumbnail(const libcamera::FrameBuffer &source,<br>
> + const libcamera::Size &targetSize,<br>
> + unsigned int quality,<br>
> + std::vector<unsigned char> *thumbnail) override;<br>
> +<br>
> +private:<br>
> + libcamera::Size size_;<br>
> +<br>
> + std::unique_ptr<cros::JpegCompressor> jpeg_compressor_;<br>
> +};<br>
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp<br>
> index 21a3b33d..b5591e33 100644<br>
> --- a/src/android/jpeg/encoder_libjpeg.cpp<br>
> +++ b/src/android/jpeg/encoder_libjpeg.cpp<br>
> @@ -24,6 +24,8 @@<br>
> #include "libcamera/internal/formats.h"<br>
> #include "libcamera/internal/mapped_framebuffer.h"<br>
> <br>
> +#include "../camera_buffer.h"<br>
> +<br>
> using namespace libcamera;<br>
> <br>
> LOG_DECLARE_CATEGORY(JPEG)<br>
> @@ -82,8 +84,17 @@ EncoderLibJpeg::~EncoderLibJpeg()<br>
> }<br>
> <br>
> int EncoderLibJpeg::configure(const StreamConfiguration &cfg)<br>
> +{<br>
> + thumbnailer_.configure(cfg.size, cfg.pixelFormat);<br>
> + cfg_ = cfg;<br>
> +<br>
> + return internalConfigure(cfg);<br>
> +}<br>
> +<br>
> +int EncoderLibJpeg::internalConfigure(const StreamConfiguration &cfg)<br>
> {<br>
> const struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat);<br>
> +<br>
> if (info.colorSpace == JCS_UNKNOWN)<br>
> return -ENOTSUP;<br>
> <br>
> @@ -178,6 +189,65 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)<br>
> }<br>
> }<br>
> <br>
> +int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality)<br>
> +{<br>
> + internalConfigure(cfg_);<br>
> + return encode(*streamBuffer->srcBuffer, streamBuffer->dstBuffer.get()->plane(0), exifData, quality);<br>
> +}<br>
> +<br>
> +int EncoderLibJpeg::generateThumbnail(<br>
> + const libcamera::FrameBuffer &source,<br>
> + const libcamera::Size &targetSize,<br>
> + unsigned int quality,<br>
> + std::vector<unsigned char> *thumbnail)<br>
> +{<br>
> + /* Stores the raw scaled-down thumbnail bytes. */<br>
> + std::vector<unsigned char> rawThumbnail;<br>
> +<br>
> + thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);<br>
> +<br>
> + StreamConfiguration thCfg;<br>
> + thCfg.size = targetSize;<br>
> + thCfg.pixelFormat = thumbnailer_.pixelFormat();<br>
> + int ret = internalConfigure(thCfg);<br>
> +<br>
> + if (!rawThumbnail.empty() && !ret) {<br>
> + /*<br>
> + * \todo Avoid value-initialization of all elements of the<br>
> + * vector.<br>
> + */<br>
> + thumbnail->resize(rawThumbnail.size());<br>
> +<br>
> + /*<br>
> + * Split planes manually as the encoder expects a vector of<br>
> + * planes.<br>
> + *<br>
> + * \todo Pass a vector of planes directly to<br>
> + * Thumbnailer::createThumbnailer above and remove the manual<br>
> + * planes split from here.<br>
> + */<br>
> + std::vector<Span<uint8_t>> thumbnailPlanes;<br>
> + const PixelFormatInfo &formatNV12 = PixelFormatInfo::info(formats::NV12);<br>
> + size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);<br>
> + size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);<br>
> + thumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });<br>
> + thumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize, uvPlaneSize });<br>
> +<br>
> + int jpeg_size = encode(thumbnailPlanes, *thumbnail, {}, quality);<br>
> + thumbnail->resize(jpeg_size);<br>
> +<br>
> + LOG(JPEG, Debug)<br>
> + << "Thumbnail compress returned "<br>
> + << jpeg_size << " bytes";<br>
> +<br>
> + return jpeg_size;<br>
> + }<br>
> +<br>
> + return -1;<br>
> +}<br>
> +<br>
> int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,<br>
> Span<const uint8_t> exifData, unsigned int quality)<br>
> {<br>
> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h<br>
> index 1b3ac067..56b27bae 100644<br>
> --- a/src/android/jpeg/encoder_libjpeg.h<br>
> +++ b/src/android/jpeg/encoder_libjpeg.h<br>
> @@ -15,6 +15,8 @@<br>
> <br>
> #include <jpeglib.h><br>
> <br>
> +#include "thumbnailer.h"<br>
> +<br>
> class EncoderLibJpeg : public Encoder<br>
> {<br>
> public:<br>
> @@ -22,19 +24,32 @@ public:<br>
> ~EncoderLibJpeg();<br>
> <br>
> int configure(const libcamera::StreamConfiguration &cfg) override;<br>
> + int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,<br>
> + libcamera::Span<const uint8_t> exifData,<br>
> + unsigned int quality) override;<br>
> + int generateThumbnail(<br>
> + const libcamera::FrameBuffer &source,<br>
> + const libcamera::Size &targetSize,<br>
> + unsigned int quality,<br>
> + std::vector<unsigned char> *thumbnail) override;<br>
> +<br>
> +private:<br>
> + int internalConfigure(const libcamera::StreamConfiguration &cfg);<br>
> +<br>
> int encode(const libcamera::FrameBuffer &source,<br>
> libcamera::Span<uint8_t> destination,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> - unsigned int quality) override;<br>
> + unsigned int quality);<br>
> int encode(const std::vector<libcamera::Span<uint8_t>> &planes,<br>
> libcamera::Span<uint8_t> destination,<br>
> libcamera::Span<const uint8_t> exifData,<br>
> unsigned int quality);<br>
> <br>
> -private:<br>
> void compressRGB(const std::vector<libcamera::Span<uint8_t>> &planes);<br>
> void compressNV(const std::vector<libcamera::Span<uint8_t>> &planes);<br>
> <br>
> + libcamera::StreamConfiguration cfg_;<br>
> +<br>
> struct jpeg_compress_struct compress_;<br>
> struct jpeg_error_mgr jerr_;<br>
> <br>
> @@ -42,4 +57,6 @@ private:<br>
> <br>
> bool nv_;<br>
> bool nvSwap_;<br>
> +<br>
> + Thumbnailer thumbnailer_;<br>
> };<br>
> diff --git a/src/android/jpeg/generic_post_processor_jpeg.cpp b/src/android/jpeg/generic_post_processor_jpeg.cpp<br>
> new file mode 100644<br>
> index 00000000..890f6972<br>
> --- /dev/null<br>
> +++ b/src/android/jpeg/generic_post_processor_jpeg.cpp<br>
> @@ -0,0 +1,14 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2022, Google Inc.<br>
> + *<br>
> + * generic_post_processor_jpeg.cpp - Generic JPEG Post Processor<br>
> + */<br>
> +<br>
> +#include "encoder_libjpeg.h"<br>
> +#include "post_processor_jpeg.h"<br>
> +<br>
> +void PostProcessorJpeg::SetEncoder()<br>
> +{<br>
> + encoder_ = std::make_unique<EncoderLibJpeg>();<br>
> +}<br>
> diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build<br>
> new file mode 100644<br>
> index 00000000..8606acc4<br>
> --- /dev/null<br>
> +++ b/src/android/jpeg/meson.build<br>
> @@ -0,0 +1,16 @@<br>
> +# SPDX-License-Identifier: CC0-1.0<br>
> +<br>
> +android_hal_sources += files([<br>
> + 'exif.cpp',<br>
> + 'post_processor_jpeg.cpp'])<br>
> +<br>
> +platform = get_option('android_platform')<br>
> +if platform == 'generic'<br>
> + android_hal_sources += files(['encoder_libjpeg.cpp',<br>
> + 'generic_post_processor_jpeg.cpp',<br>
> + 'thumbnailer.cpp'])<br>
> +elif platform == 'cros'<br>
> + android_hal_sources += files(['cros_post_processor_jpeg.cpp',<br>
> + 'encoder_jea.cpp'])<br>
> + android_deps += [dependency('libcros_camera')]<br>
> +endif<br>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp<br>
> index d72ebc3c..7ceba60e 100644<br>
> --- a/src/android/jpeg/post_processor_jpeg.cpp<br>
> +++ b/src/android/jpeg/post_processor_jpeg.cpp<br>
> @@ -9,10 +9,10 @@<br>
> <br>
> #include <chrono><br>
> <br>
> +#include "../android_framebuffer.h"<br>
> #include "../camera_device.h"<br>
> #include "../camera_metadata.h"<br>
> #include "../camera_request.h"<br>
> -#include "encoder_libjpeg.h"<br>
> #include "exif.h"<br>
> <br>
> #include <libcamera/base/log.h><br>
> @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,<br>
> <br>
> streamSize_ = outCfg.size;<br>
> <br>
> - thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);<br>
> -<br>
> - encoder_ = std::make_unique<EncoderLibJpeg>();<br>
> + SetEncoder();<br>
> <br>
> return encoder_->configure(inCfg);<br>
> }<br>
> <br>
> -void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,<br>
> - const Size &targetSize,<br>
> - unsigned int quality,<br>
> - std::vector<unsigned char> *thumbnail)<br>
> -{<br>
> - /* Stores the raw scaled-down thumbnail bytes. */<br>
> - std::vector<unsigned char> rawThumbnail;<br>
> -<br>
> - thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);<br>
> -<br>
> - StreamConfiguration thCfg;<br>
> - thCfg.size = targetSize;<br>
> - thCfg.pixelFormat = thumbnailer_.pixelFormat();<br>
> - int ret = thumbnailEncoder_.configure(thCfg);<br>
> -<br>
> - if (!rawThumbnail.empty() && !ret) {<br>
> - /*<br>
> - * \todo Avoid value-initialization of all elements of the<br>
> - * vector.<br>
> - */<br>
> - thumbnail->resize(rawThumbnail.size());<br>
> -<br>
> - /*<br>
> - * Split planes manually as the encoder expects a vector of<br>
> - * planes.<br>
> - *<br>
> - * \todo Pass a vector of planes directly to<br>
> - * Thumbnailer::createThumbnailer above and remove the manual<br>
> - * planes split from here.<br>
> - */<br>
> - std::vector<Span<uint8_t>> thumbnailPlanes;<br>
> - const PixelFormatInfo &formatNV12 = PixelFormatInfo::info(formats::NV12);<br>
> - size_t yPlaneSize = formatNV12.planeSize(targetSize, 0);<br>
> - size_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);<br>
> - thumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });<br>
> - thumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize, uvPlaneSize });<br>
> -<br>
> - int jpeg_size = thumbnailEncoder_.encode(thumbnailPlanes,<br>
> - *thumbnail, {}, quality);<br>
> - thumbnail->resize(jpeg_size);<br>
> -<br>
> - LOG(JPEG, Debug)<br>
> - << "Thumbnail compress returned "<br>
> - << jpeg_size << " bytes";<br>
> - }<br>
> -}<br>
> -<br>
> void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)<br>
> {<br>
> ASSERT(encoder_);<br>
> @@ -164,8 +115,8 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu<br>
> <br>
> if (thumbnailSize != Size(0, 0)) {<br>
> std::vector<unsigned char> thumbnail;<br>
> - generateThumbnail(source, thumbnailSize, quality, &thumbnail);<br>
> - if (!thumbnail.empty())<br>
> + ret = encoder_->generateThumbnail(source, thumbnailSize, quality, &thumbnail);<br>
> + if (ret > 0 && !thumbnail.empty())<br>
> exif.setThumbnail(thumbnail, Exif::Compression::JPEG);<br>
> }<br>
> <br>
> @@ -194,8 +145,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu<br>
> const uint8_t quality = ret ? *entry.data.u8 : 95;<br>
> resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);<br>
> <br>
> - int jpeg_size = encoder_->encode(source, destination->plane(0),<br>
> - exif.data(), quality);<br>
> + int jpeg_size = encoder_->encode(streamBuffer, exif.data(), quality);<br>
> if (jpeg_size < 0) {<br>
> LOG(JPEG, Error) << "Failed to encode stream image";<br>
> processComplete.emit(streamBuffer, PostProcessor::Status::Error);<br>
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h<br>
> index 98309b01..a09f8798 100644<br>
> --- a/src/android/jpeg/post_processor_jpeg.h<br>
> +++ b/src/android/jpeg/post_processor_jpeg.h<br>
> @@ -8,11 +8,11 @@<br>
> #pragma once<br>
> <br>
> #include "../post_processor.h"<br>
> -#include "encoder_libjpeg.h"<br>
> -#include "thumbnailer.h"<br>
> <br>
> #include <libcamera/geometry.h><br>
> <br>
> +#include "encoder.h"<br>
> +<br>
> class CameraDevice;<br>
> <br>
> class PostProcessorJpeg : public PostProcessor<br>
> @@ -25,14 +25,9 @@ public:<br>
> void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override;<br>
> <br>
> private:<br>
> - void generateThumbnail(const libcamera::FrameBuffer &source,<br>
> - const libcamera::Size &targetSize,<br>
> - unsigned int quality,<br>
> - std::vector<unsigned char> *thumbnail);<br>
> + void SetEncoder();<br>
> <br>
> CameraDevice *const cameraDevice_;<br>
> std::unique_ptr<Encoder> encoder_;<br>
> libcamera::Size streamSize_;<br>
> - EncoderLibJpeg thumbnailEncoder_;<br>
> - Thumbnailer thumbnailer_;<br>
> };<br>
> diff --git a/src/android/meson.build b/src/android/meson.build<br>
> index 75b4bf20..026b8b3c 100644<br>
> --- a/src/android/meson.build<br>
> +++ b/src/android/meson.build<br>
> @@ -38,6 +38,7 @@ endif<br>
> android_deps += [libyuv_dep]<br>
> <br>
> android_hal_sources = files([<br>
> + 'android_framebuffer.cpp',<br>
> 'camera3_hal.cpp',<br>
> 'camera_capabilities.cpp',<br>
> 'camera_device.cpp',<br>
> @@ -47,10 +48,6 @@ android_hal_sources = files([<br>
> 'camera_ops.cpp',<br>
> 'camera_request.cpp',<br>
> 'camera_stream.cpp',<br>
> - 'jpeg/encoder_libjpeg.cpp',<br>
> - 'jpeg/exif.cpp',<br>
> - 'jpeg/post_processor_jpeg.cpp',<br>
> - 'jpeg/thumbnailer.cpp',<br>
> 'yuv/post_processor_yuv.cpp'<br>
> ])<br>
> <br>
> @@ -58,6 +55,7 @@ android_cpp_args = []<br>
> <br>
> subdir('cros')<br>
> subdir('mm')<br>
> +subdir('jpeg')<br>
> <br>
> android_camera_metadata_sources = files([<br>
> 'metadata/camera_metadata.c',<br>
> diff --git a/src/android/mm/cros_frame_buffer_allocator.cpp b/src/android/mm/cros_frame_buffer_allocator.cpp<br>
> index 52e8c180..163c5d75 100644<br>
> --- a/src/android/mm/cros_frame_buffer_allocator.cpp<br>
> +++ b/src/android/mm/cros_frame_buffer_allocator.cpp<br>
> @@ -14,6 +14,7 @@<br>
> <br>
> #include "libcamera/internal/framebuffer.h"<br>
> <br>
> +#include "../android_framebuffer.h"<br>
> #include "../camera_device.h"<br>
> #include "../frame_buffer_allocator.h"<br>
> #include "cros-camera/camera_buffer_manager.h"<br>
> @@ -47,11 +48,11 @@ public:<br>
> {<br>
> }<br>
> <br>
> - std::unique_ptr<libcamera::FrameBuffer><br>
> + std::unique_ptr<AndroidFrameBuffer><br>
> allocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);<br>
> };<br>
> <br>
> -std::unique_ptr<libcamera::FrameBuffer><br>
> +std::unique_ptr<AndroidFrameBuffer><br>
> PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,<br>
> const libcamera::Size &size,<br>
> uint32_t usage)<br>
> @@ -80,9 +81,11 @@ PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,<br>
> plane.length = cros::CameraBufferManager::GetPlaneSize(handle, i);<br>
> }<br>
> <br>
> - return std::make_unique<FrameBuffer>(<br>
> - std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),<br>
> - planes);<br>
> + auto fb = std::make_unique<AndroidFrameBuffer>(handle,<br>
> + std::make_unique<CrosFrameBufferData>(std::move(scopedHandle)),<br>
> + planes);<br>
> +<br>
> + return fb;<br>
> }<br>
> <br>
> PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION<br>
> diff --git a/src/android/mm/generic_frame_buffer_allocator.cpp b/src/android/mm/generic_frame_buffer_allocator.cpp<br>
> index acb2fa2b..c79b7b10 100644<br>
> --- a/src/android/mm/generic_frame_buffer_allocator.cpp<br>
> +++ b/src/android/mm/generic_frame_buffer_allocator.cpp<br>
> @@ -18,6 +18,7 @@<br>
> #include <hardware/gralloc.h><br>
> #include <hardware/hardware.h><br>
> <br>
> +#include "../android_framebuffer.h"<br>
> #include "../camera_device.h"<br>
> #include "../frame_buffer_allocator.h"<br>
> <br>
> @@ -77,7 +78,7 @@ public:<br>
> <br>
> ~Private() override;<br>
> <br>
> - std::unique_ptr<libcamera::FrameBuffer><br>
> + std::unique_ptr<AndroidFrameBuffer><br>
> allocate(int halPixelFormat, const libcamera::Size &size, uint32_t usage);<br>
> <br>
> private:<br>
> @@ -92,7 +93,7 @@ PlatformFrameBufferAllocator::Private::~Private()<br>
> gralloc_close(allocDevice_);<br>
> }<br>
> <br>
> -std::unique_ptr<libcamera::FrameBuffer><br>
> +std::unique_ptr<AndroidFrameBuffer><br>
> PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,<br>
> const libcamera::Size &size,<br>
> uint32_t usage)<br>
> @@ -135,9 +136,9 @@ PlatformFrameBufferAllocator::Private::allocate(int halPixelFormat,<br>
> offset += planeSize;<br>
> }<br>
> <br>
> - return std::make_unique<FrameBuffer>(<br>
> - std::make_unique<GenericFrameBufferData>(allocDevice_, handle),<br>
> - planes);<br>
> + return std::make_unique<AndroidFrameBuffer>(handle,<br>
> + std::make_unique<GenericFrameBufferData>(allocDevice_, handle),<br>
> + planes);<br>
> }<br>
> <br>
> PUBLIC_FRAME_BUFFER_ALLOCATOR_IMPLEMENTATION<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div>