[libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to PostProcessor interface
Umang Jain
email at uajain.com
Fri Oct 16 09:15:20 CEST 2020
Hi,
On 10/16/20 12:02 PM, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:
>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.
>> This encapsulates the encoder and EXIF generation code into the
>> PostProcessorJpeg layer and removes these specifics related to JPEG,
>> from the CameraStream itself.
>>
>> Signed-off-by: Umang Jain <email at uajain.com>
>> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Kieran, would you be able to test this new version, and push it if all
> looks good to you ?
Yes please, one final testing should be done before actual pushing.
Thanks for the reviews. Thanks to Kieran for removing blockers on this
and testing efforts :)
>
>> ---
>> src/android/camera_device.cpp | 1 +
>> src/android/camera_stream.cpp | 77 ++++-------------
>> src/android/camera_stream.h | 4 +-
>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-
>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++
>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++
>> src/android/meson.build | 1 +
>> 7 files changed, 164 insertions(+), 62 deletions(-)
>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp
>> create mode 100644 src/android/jpeg/post_processor_jpeg.h
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 0983232..d706cf4 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -7,6 +7,7 @@
>>
>> #include "camera_device.h"
>> #include "camera_ops.h"
>> +#include "post_processor.h"
>>
>> #include <sys/mman.h>
>> #include <tuple>
>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
>> index d8e45c2..1b8afa8 100644
>> --- a/src/android/camera_stream.cpp
>> +++ b/src/android/camera_stream.cpp
>> @@ -9,9 +9,9 @@
>>
>> #include "camera_device.h"
>> #include "camera_metadata.h"
>> -#include "jpeg/encoder.h"
>> -#include "jpeg/encoder_libjpeg.h"
>> -#include "jpeg/exif.h"
>> +#include "jpeg/post_processor_jpeg.h"
>> +
>> +#include <libcamera/formats.h>
>>
>> using namespace libcamera;
>>
>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,
>> {
>> config_ = cameraDevice_->cameraConfiguration();
>>
>> - if (type_ == Type::Internal || type_ == Type::Mapped)
>> - encoder_ = std::make_unique<EncoderLibJpeg>();
>> + if (type_ == Type::Internal || type_ == Type::Mapped) {
>> + /*
>> + * \todo There might be multiple post-processors. The logic
>> + * which should be instantiated here, is deferred for the
>> + * future. For now, we only have PostProcessorJpeg and that
>> + * is what we instantiate here.
>> + */
>> + postProcessor_ = std::make_unique<PostProcessorJpeg>(cameraDevice_);
>> + }
>>
>> if (type == Type::Internal) {
>> allocator_ = std::make_unique<FrameBufferAllocator>(cameraDevice_->camera());
>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const
>>
>> int CameraStream::configure()
>> {
>> - if (encoder_) {
>> - int ret = encoder_->configure(configuration());
>> + if (postProcessor_) {
>> + StreamConfiguration output = configuration();
>> + output.pixelFormat = formats::MJPEG;
>> + int ret = postProcessor_->configure(configuration(), output);
>> if (ret)
>> return ret;
>> }
>> @@ -90,60 +99,10 @@ int CameraStream::configure()
>> int CameraStream::process(const libcamera::FrameBuffer &source,
>> MappedCamera3Buffer *dest, CameraMetadata *metadata)
>> {
>> - if (!encoder_)
>> + if (!postProcessor_)
>> return 0;
>>
>> - /* Set EXIF metadata for various tags. */
>> - Exif exif;
>> - /* \todo Set Make and Model from external vendor tags. */
>> - exif.setMake("libcamera");
>> - exif.setModel("cameraModel");
>> - exif.setOrientation(cameraDevice_->orientation());
>> - exif.setSize(configuration().size);
>> - /*
>> - * We set the frame's EXIF timestamp as the time of encode.
>> - * Since the precision we need for EXIF timestamp is only one
>> - * second, it is good enough.
>> - */
>> - exif.setTimestamp(std::time(nullptr));
>> - if (exif.generate() != 0)
>> - LOG(HAL, Error) << "Failed to generate valid EXIF data";
>> -
>> - int jpeg_size = encoder_->encode(&source, dest->maps()[0], exif.data());
>> - if (jpeg_size < 0) {
>> - LOG(HAL, Error) << "Failed to encode stream image";
>> - return jpeg_size;
>> - }
>> -
>> - /*
>> - * Fill in the JPEG blob header.
>> - *
>> - * The mapped size of the buffer is being returned as
>> - * substantially larger than the requested JPEG_MAX_SIZE
>> - * (which is referenced from maxJpegBufferSize_). Utilise
>> - * this static size to ensure the correct offset of the blob is
>> - * determined.
>> - *
>> - * \todo Investigate if the buffer size mismatch is an issue or
>> - * expected behaviour.
>> - */
>> - uint8_t *resultPtr = dest->maps()[0].data() +
>> - cameraDevice_->maxJpegBufferSize() -
>> - sizeof(struct camera3_jpeg_blob);
>> - auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
>> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
>> - blob->jpeg_size = jpeg_size;
>> -
>> - /* Update the JPEG result Metadata. */
>> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
>> -
>> - const uint32_t jpeg_quality = 95;
>> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
>> -
>> - const uint32_t jpeg_orientation = 0;
>> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
>> -
>> - return 0;
>> + return postProcessor_->process(&source, dest->maps()[0], metadata);
>> }
>>
>> FrameBuffer *CameraStream::getBuffer()
>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
>> index d3925fb..c367a5f 100644
>> --- a/src/android/camera_stream.h
>> +++ b/src/android/camera_stream.h
>> @@ -19,10 +19,10 @@
>> #include <libcamera/geometry.h>
>> #include <libcamera/pixel_format.h>
>>
>> -class Encoder;
>> class CameraDevice;
>> class CameraMetadata;
>> class MappedCamera3Buffer;
>> +class PostProcessor;
>>
>> class CameraStream
>> {
>> @@ -130,7 +130,6 @@ private:
>> camera3_stream_t *camera3Stream_;
>> unsigned int index_;
>>
>> - std::unique_ptr<Encoder> encoder_;
>> std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
>> std::vector<libcamera::FrameBuffer *> buffers_;
>> /*
>> @@ -138,6 +137,7 @@ private:
>> * an std::vector in CameraDevice.
>> */
>> std::unique_ptr<std::mutex> mutex_;
>> + std::unique_ptr<PostProcessor> postProcessor_;
>> };
>>
>> #endif /* __ANDROID_CAMERA_STREAM__ */
>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
>> index a77f5b2..8995ba5 100644
>> --- a/src/android/jpeg/encoder_libjpeg.cpp
>> +++ b/src/android/jpeg/encoder_libjpeg.cpp
>> @@ -25,7 +25,7 @@
>>
>> using namespace libcamera;
>>
>> -LOG_DEFINE_CATEGORY(JPEG)
>> +LOG_DECLARE_CATEGORY(JPEG);
>>
>> namespace {
>>
>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
>> new file mode 100644
>> index 0000000..753c28e
>> --- /dev/null
>> +++ b/src/android/jpeg/post_processor_jpeg.cpp
>> @@ -0,0 +1,105 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2020, Google Inc.
>> + *
>> + * post_processor_jpeg.cpp - JPEG Post Processor
>> + */
>> +
>> +#include "post_processor_jpeg.h"
>> +
>> +#include "../camera_device.h"
>> +#include "../camera_metadata.h"
>> +#include "encoder_libjpeg.h"
>> +#include "exif.h"
>> +
>> +#include <libcamera/formats.h>
>> +
>> +#include "libcamera/internal/log.h"
>> +
>> +using namespace libcamera;
>> +
>> +LOG_DEFINE_CATEGORY(JPEG);
>> +
>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)
>> + : cameraDevice_(device)
>> +{
>> +}
>> +
>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,
>> + const StreamConfiguration &outCfg)
>> +{
>> + if (inCfg.size != outCfg.size) {
>> + LOG(JPEG, Error) << "Mismatch of input and output stream sizes";
>> + return -EINVAL;
>> + }
>> +
>> + if (outCfg.pixelFormat != formats::MJPEG) {
>> + LOG(JPEG, Error) << "Output stream pixel format is not JPEG";
>> + return -EINVAL;
>> + }
>> +
>> + streamSize_ = outCfg.size;
>> + encoder_ = std::make_unique<EncoderLibJpeg>();
>> +
>> + return encoder_->configure(inCfg);
>> +}
>> +
>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,
>> + const libcamera::Span<uint8_t> &destination,
>> + CameraMetadata *metadata)
>> +{
>> + if (!encoder_)
>> + return 0;
>> +
>> + /* Set EXIF metadata for various tags. */
>> + Exif exif;
>> + /* \todo Set Make and Model from external vendor tags. */
>> + exif.setMake("libcamera");
>> + exif.setModel("cameraModel");
>> + exif.setOrientation(cameraDevice_->orientation());
>> + exif.setSize(streamSize_);
>> + /*
>> + * We set the frame's EXIF timestamp as the time of encode.
>> + * Since the precision we need for EXIF timestamp is only one
>> + * second, it is good enough.
>> + */
>> + exif.setTimestamp(std::time(nullptr));
>> + if (exif.generate() != 0)
>> + LOG(JPEG, Error) << "Failed to generate valid EXIF data";
>> +
>> + int jpeg_size = encoder_->encode(source, destination, exif.data());
>> + if (jpeg_size < 0) {
>> + LOG(JPEG, Error) << "Failed to encode stream image";
>> + return jpeg_size;
>> + }
>> +
>> + /*
>> + * Fill in the JPEG blob header.
>> + *
>> + * The mapped size of the buffer is being returned as
>> + * substantially larger than the requested JPEG_MAX_SIZE
>> + * (which is referenced from maxJpegBufferSize_). Utilise
>> + * this static size to ensure the correct offset of the blob is
>> + * determined.
>> + *
>> + * \todo Investigate if the buffer size mismatch is an issue or
>> + * expected behaviour.
>> + */
>> + uint8_t *resultPtr = destination.data() +
>> + cameraDevice_->maxJpegBufferSize() -
>> + sizeof(struct camera3_jpeg_blob);
>> + auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
>> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
>> + blob->jpeg_size = jpeg_size;
>> +
>> + /* Update the JPEG result Metadata. */
>> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
>> +
>> + const uint32_t jpeg_quality = 95;
>> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
>> +
>> + const uint32_t jpeg_orientation = 0;
>> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
>> +
>> + return 0;
>> +}
>> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
>> new file mode 100644
>> index 0000000..62c8650
>> --- /dev/null
>> +++ b/src/android/jpeg/post_processor_jpeg.h
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2020, Google Inc.
>> + *
>> + * post_processor_jpeg.h - JPEG Post Processor
>> + */
>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__
>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__
>> +
>> +#include "../post_processor.h"
>> +
>> +#include <libcamera/geometry.h>
>> +
>> +#include "libcamera/internal/buffer.h"
>> +
>> +class Encoder;
>> +class CameraDevice;
>> +
>> +class PostProcessorJpeg : public PostProcessor
>> +{
>> +public:
>> + PostProcessorJpeg(CameraDevice *device);
>> +
>> + int configure(const libcamera::StreamConfiguration &incfg,
>> + const libcamera::StreamConfiguration &outcfg) override;
>> + int process(const libcamera::FrameBuffer *source,
>> + const libcamera::Span<uint8_t> &destination,
>> + CameraMetadata *metadata) override;
>> +
>> +private:
>> + CameraDevice *cameraDevice_;
>> + std::unique_ptr<Encoder> encoder_;
>> + libcamera::Size streamSize_;
>> +};
>> +
>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */
>> diff --git a/src/android/meson.build b/src/android/meson.build
>> index b2b2293..5a01bea 100644
>> --- a/src/android/meson.build
>> +++ b/src/android/meson.build
>> @@ -24,6 +24,7 @@ android_hal_sources = files([
>> 'camera_worker.cpp',
>> 'jpeg/encoder_libjpeg.cpp',
>> 'jpeg/exif.cpp',
>> + 'jpeg/post_processor_jpeg.cpp',
>> ])
>>
>> android_camera_metadata_sources = files([
More information about the libcamera-devel
mailing list