[libcamera-devel] [PATCH v7 6/6] Add JEA implementation

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Dec 7 06:12:16 CET 2022


Hi Harvey,

Thank you for the patch.

On Thu, Dec 01, 2022 at 09:27:33AM +0000, Harvey Yang via libcamera-devel wrote:
> From: Harvey Yang <chenghaoyang at chromium.org>
> 
> This patch adds JEA implementation to replace Lib Jpeg in cros platform,

s/Lib Jpeg/libjpeg/
s/cros/CrOS/

> where HW accelerator is available.

s/HW/hardware/

> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  src/android/cros/camera3_hal.cpp         |  2 +
>  src/android/cros_mojo_token.h            | 12 +++
>  src/android/jpeg/encoder_jea.cpp         | 93 ++++++++++++++++++++++++
>  src/android/jpeg/encoder_jea.h           | 35 +++++++++
>  src/android/jpeg/meson.build             | 12 ++-
>  src/android/jpeg/post_processor_jpeg.cpp |  8 ++
>  6 files changed, 160 insertions(+), 2 deletions(-)
>  create mode 100644 src/android/cros_mojo_token.h
>  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..d75afccb 100644
> --- a/src/android/cros/camera3_hal.cpp
> +++ b/src/android/cros/camera3_hal.cpp
> @@ -8,9 +8,11 @@
>  #include <cros-camera/cros_camera_hal.h>
>  
>  #include "../camera_hal_manager.h"
> +#include "../cros_mojo_token.h"
>  
>  static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)

You can drop [[maybe_unused]].

>  {
> +	gCrosMojoToken = token;
>  }
>  
>  static void tear_down()
> diff --git a/src/android/cros_mojo_token.h b/src/android/cros_mojo_token.h
> new file mode 100644
> index 00000000..043c752a
> --- /dev/null
> +++ b/src/android/cros_mojo_token.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * cros_mojo_token.h - cros-specific mojo token
> + */
> +
> +#pragma once
> +
> +#include <cros-camera/cros_camera_hal.h>
> +
> +inline cros::CameraMojoChannelManagerToken *gCrosMojoToken = nullptr;
> diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp
> new file mode 100644
> index 00000000..66a854c4
> --- /dev/null
> +++ b/src/android/jpeg/encoder_jea.cpp
> @@ -0,0 +1,93 @@
> +/* 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/internal/mapped_framebuffer.h"
> +
> +#include <cros-camera/camera_mojo_channel_manager_token.h>
> +
> +#include "../cros_mojo_token.h"
> +#include "../hal_framebuffer.h"
> +
> +EncoderJea::EncoderJea() = default;
> +
> +EncoderJea::~EncoderJea() = default;
> +
> +int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)
> +{
> +	size_ = cfg.size;
> +
> +	if (jpegCompressor_)
> +		return 0;
> +
> +	if (gCrosMojoToken == nullptr)
> +		return -ENOTSUP;
> +
> +	jpegCompressor_ = cros::JpegCompressor::GetInstance(gCrosMojoToken);
> +
> +	return 0;
> +}
> +
> +int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> +		       libcamera::Span<const uint8_t> exifData,
> +		       unsigned int quality)
> +{
> +	if (!jpegCompressor_)
> +		return -ENOTSUP;
> +
> +	uint32_t outDataSize = 0;
> +	const HALFrameBuffer *fb = dynamic_cast<const HALFrameBuffer *>(
> +		streamBuffer->srcBuffer);

It looks like we could avoid subclassing FrameBuffer (basically dropping
patches 1/6 and 2/6) if we stored the handle in the StreamBuffer class
instead of in HALFrameBuffer. We can keep it as-is for now, I'm
interested in experimenting with inheriting from FrameBuffer, but maybe
that will change in the future.

> +
> +	if (!jpegCompressor_->CompressImageFromHandle(fb->handle(),
> +						      *streamBuffer->camera3Buffer,
> +						      size_.width, size_.height,
> +						      quality, exifData.data(),
> +						      exifData.size(),
> +						      &outDataSize))
> +		return -EBUSY;
> +
> +	return outDataSize;
> +}
> +
> +int EncoderJea::generateThumbnail(const libcamera::FrameBuffer &source,
> +				  const libcamera::Size &targetSize,
> +				  unsigned int quality,
> +				  std::vector<unsigned char> *thumbnail)
> +{
> +	if (!jpegCompressor_)
> +		return -ENOTSUP;
> +
> +	libcamera::MappedFrameBuffer frame(&source,
> +					   libcamera::MappedFrameBuffer::MapFlag::Read);
> +
> +	if (frame.planes().empty())
> +		return -ENODATA;
> +
> +	uint32_t outDataSize = 0;
> +
> +	// Since the structure of the App1 segment is like:
> +	//   0xFF [1 byte marker] [2 bytes size] [data]
> +	// And it should not be larger than 64K.

C-style comments.

> +	const int kApp1MaxDataSize = 65532;

constexpr

> +	thumbnail->resize(kApp1MaxDataSize);
> +
> +	if (!jpegCompressor_->GenerateThumbnail(frame.planes()[0].data(),
> +						size_.width, size_.height,
> +						targetSize.width,
> +						targetSize.height, quality,
> +						thumbnail->size(),
> +						thumbnail->data(),
> +						&outDataSize)) {
> +		thumbnail->clear();
> +		return -EBUSY;
> +	}
> +
> +	thumbnail->resize(outDataSize);
> +	return outDataSize;
> +}
> diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h
> new file mode 100644
> index 00000000..2eba31c2
> --- /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> jpegCompressor_;
> +};
> diff --git a/src/android/jpeg/meson.build b/src/android/jpeg/meson.build
> index 08397a87..d25016b1 100644
> --- a/src/android/jpeg/meson.build
> +++ b/src/android/jpeg/meson.build
> @@ -1,8 +1,16 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  android_hal_sources += files([
> -    'encoder_libjpeg.cpp',
>      'exif.cpp',
>      'post_processor_jpeg.cpp',
> -    'thumbnailer.cpp'
>  ])
> +
> +platform = get_option('android_platform')
> +if platform == 'generic'
> +    android_hal_sources += files(['encoder_libjpeg.cpp',
> +                                  'thumbnailer.cpp'
> +    ])

    android_hal_sources += files([
        'encoder_libjpeg.cpp',
        'thumbnailer.cpp',
    ])

> +elif platform == 'cros'
> +    android_hal_sources += files(['encoder_jea.cpp'])
> +    android_deps += [dependency('libcros_camera')]
> +endif
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 10ac4666..15115424 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -12,7 +12,11 @@
>  #include "../camera_device.h"
>  #include "../camera_metadata.h"
>  #include "../camera_request.h"
> +#if defined(OS_CHROMEOS)
> +#include "encoder_jea.h"
> +#else // !defined(OS_CHROMEOS)

C-style comments.

>  #include "encoder_libjpeg.h"
> +#endif
>  #include "exif.h"
>  
>  #include <libcamera/base/log.h>
> @@ -44,7 +48,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,
>  
>  	streamSize_ = outCfg.size;
>  
> +#if defined(OS_CHROMEOS)
> +	encoder_ = std::make_unique<EncoderJea>();
> +#else // !defined(OS_CHROMEOS)

Same here.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  	encoder_ = std::make_unique<EncoderLibJpeg>();
> +#endif
>  
>  	return encoder_->configure(inCfg);
>  }

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list