[libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to PostProcessor interface
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Oct 16 12:25:13 CEST 2020
Hi Umang, Laurent,
On 16/10/2020 08:15, Umang Jain wrote:
> 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 :)
Ok - I had a few hit and misses due to apparent hardware failure (the
camera didn't power up, now up with a full power cycle) - and then the
cros build not actually building the code.
all of that is solved, and confirmed that it's definitely Umang's code
running on the device ... successfully ;-)
I'll push these patches now.
Thanks
Kieran
>>
>>> ---
>>> 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([
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list