[libcamera-devel] [PATCH v3 4/4] Add JEA implementation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Apr 27 01:58:52 CEST 2022
Hi Harvey,
Thank you for the patch.
On Tue, Apr 26, 2022 at 09:14:09AM +0000, Harvey Yang via libcamera-devel wrote:
> This CL adds JEA implementation to replace Lib Jpeg in cros platform,
> where HW accelerator is available.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
> src/android/cros/camera3_hal.cpp | 3 +
> src/android/jpeg/cros_post_processor_jpeg.cpp | 14 ++++
> src/android/jpeg/encoder_jea.cpp | 82 +++++++++++++++++++
> src/android/jpeg/encoder_jea.h | 35 ++++++++
> src/android/jpeg/meson.build | 13 ++-
> 5 files changed, 144 insertions(+), 3 deletions(-)
> create mode 100644 src/android/jpeg/cros_post_processor_jpeg.cpp
> create mode 100644 src/android/jpeg/encoder_jea.cpp
> create mode 100644 src/android/jpeg/encoder_jea.h
>
> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp
> index fb863b5f..ea5577f0 100644
> --- a/src/android/cros/camera3_hal.cpp
> +++ b/src/android/cros/camera3_hal.cpp
> @@ -9,8 +9,11 @@
>
> #include "../camera_hal_manager.h"
>
> +cros::CameraMojoChannelManagerToken *g_cros_camera_token = nullptr;
gCrosCameraToken to match the coding style, but I'd name the variable
gCrosMojoToken to make it clearer it refers to mojo.
> +
> static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)
> {
> + g_cros_camera_token = token;
> }
>
> static void tear_down()
> diff --git a/src/android/jpeg/cros_post_processor_jpeg.cpp b/src/android/jpeg/cros_post_processor_jpeg.cpp
> new file mode 100644
> index 00000000..7020f0d0
> --- /dev/null
> +++ b/src/android/jpeg/cros_post_processor_jpeg.cpp
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * cros_post_processor_jpeg.cpp - CrOS JPEG Post Processor
> + */
> +
> +#include "encoder_jea.h"
> +#include "post_processor_jpeg.h"
> +
> +void PostProcessorJpeg::SetEncoder()
> +{
> + encoder_ = std::make_unique<EncoderJea>();
> +}
> diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp
> new file mode 100644
> index 00000000..838e8647
> --- /dev/null
> +++ b/src/android/jpeg/encoder_jea.cpp
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * encoder_jea.cpp - JPEG encoding using CrOS JEA
> + */
> +
> +#include "encoder_jea.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/mapped_framebuffer.h"
> +
> +#include <cros-camera/camera_mojo_channel_manager_token.h>
> +
> +#include "../android_framebuffer.h"
> +
> +extern cros::CameraMojoChannelManagerToken *g_cros_camera_token;
Please add this to a header file as appropriate.
> +
> +EncoderJea::EncoderJea() = default;
> +
> +EncoderJea::~EncoderJea() = default;
> +
> +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)
> +{
> + size_ = cfg.size;
> +
> + if (jpeg_compressor_.get())
You can write
if (jpeg_compressor_)
Same below.
> + return 0;
> +
> + if (g_cros_camera_token == nullptr)
> + return -ENOTSUP;
> +
> + jpeg_compressor_ = cros::JpegCompressor::GetInstance(g_cros_camera_token);
> +
> + return 0;
> +}
> +
> +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> + libcamera::Span<const uint8_t> exifData,
> + unsigned int quality)
> +{
> + if (!jpeg_compressor_.get())
> + return -1;
Could you pick an error code instead of -1 ? Same below.
> +
> + uint32_t out_data_size = 0;
outDataSize
Same below.
> +
> + if (!jpeg_compressor_->CompressImageFromHandle(
> + dynamic_cast<const AndroidFrameBuffer *>(
> + streamBuffer->srcBuffer)
> + ->getHandle(),
> + *streamBuffer->camera3Buffer, size_.width, size_.height, quality,
> + exifData.data(), exifData.size(), &out_data_size)) {
> + return -1;
> + }
An intermediate variable would make this easier to read.
const AndroidFrameBuffer *fb =
dynamic_cast<const AndroidFrameBuffer *>(streamBuffer->srcBuffer);
if (!jpeg_compressor_->CompressImageFromHandle(fb->getHandle(),
*streamBuffer->camera3Buffer,
size_.width, size_.height, quality,
exifData.data(), exifData.size(),
&out_data_size))
return -1;
> +
> + return out_data_size;
> +}
> +
> +int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> + unsigned int quality,
> + std::vector<unsigned char> *thumbnail)
> +{
> + if (!jpeg_compressor_.get())
> + return -1;
> +
> + libcamera::MappedFrameBuffer frame(&source, libcamera::MappedFrameBuffer::MapFlag::Read);
> +
> + if (frame.planes().empty())
> + return 0;
Isn't this an error ?
> +
> + uint32_t out_data_size = 0;
> +
> + if (!jpeg_compressor_->GenerateThumbnail(frame.planes()[0].data(),
> + size_.width, size_.height, targetSize.width, targetSize.height,
> + quality, thumbnail->size(), thumbnail->data(), &out_data_size)) {
> + return -1;
> + }
No need for curly braces, and a bit of line wrap would be nice :-)
> +
> + return out_data_size;
> +}
> diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h
> new file mode 100644
> index 00000000..d5c9f1f7
> --- /dev/null
> +++ b/src/android/jpeg/encoder_jea.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * encoder_jea.h - JPEG encoding using CrOS JEA
> + */
> +
> +#pragma once
> +
> +#include <libcamera/geometry.h>
> +
> +#include <cros-camera/jpeg_compressor.h>
> +
> +#include "encoder.h"
> +
> +class EncoderJea : public Encoder
> +{
> +public:
> + EncoderJea();
> + ~EncoderJea();
> +
> + int configure(const libcamera::StreamConfiguration &cfg) override;
> + int encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> + libcamera::Span<const uint8_t> exifData,
> + unsigned int quality) override;
> + int generateThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> + unsigned int quality,
> + std::vector<unsigned char> *thumbnail) override;
> +
> +private:
> + libcamera::Size size_;
> +
> + std::unique_ptr<cros::JpegCompressor> jpeg_compressor_;
jpegCompressor_
or just compressor_ to shorten lines.
> +};
> diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build
> index 94522dc0..8606acc4 100644
> --- a/src/android/jpeg/meson.build
> +++ b/src/android/jpeg/meson.build
> @@ -4,6 +4,13 @@ android_hal_sources += files([
> 'exif.cpp',
> 'post_processor_jpeg.cpp'])
>
> -android_hal_sources += files(['encoder_libjpeg.cpp',
> - 'generic_post_processor_jpeg.cpp',
> - 'thumbnailer.cpp'])
> +platform = get_option('android_platform')
> +if platform == 'generic'
> + android_hal_sources += files(['encoder_libjpeg.cpp',
> + 'generic_post_processor_jpeg.cpp',
> + 'thumbnailer.cpp'])
> +elif platform == 'cros'
> + android_hal_sources += files(['cros_post_processor_jpeg.cpp',
> + 'encoder_jea.cpp'])
> + android_deps += [dependency('libcros_camera')]
> +endif
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list