[libcamera-devel] [PATCH v3 4/4] Add JEA implementation
Cheng-Hao Yang
chenghaoyang at chromium.org
Wed May 4 09:18:04 CEST 2022
Thanks Laurent!
Sorry I forgot to send this reply before going on vacation...
And I should also reply to libcamera-devel as well.
> Could you pick an error code instead of -1 ? Same below.
Not sure which error code to use when |jpegCompressor_| fails to do the
compression. Using -EBUSY for now.
Followed mostly your suggestions. Please check if I miss anything.
BR,
Harvey
On Wed, Apr 27, 2022 at 7:58 AM Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220504/dc853d7f/attachment.htm>
More information about the libcamera-devel
mailing list