[libcamera-devel] [RFC PATCH 2/2] android: jpeg: Add a basic NV12 image thumbnailer
Umang Jain
email at uajain.com
Wed Oct 21 13:03:40 CEST 2020
Hi Kieran,
On 10/21/20 4:25 PM, Kieran Bingham wrote:
> Hi Umang,
>
>
> On 21/10/2020 11:51, Umang Jain wrote:
>> Hi Kieran,
>>
>> Thanks for the comments.
>>
>> On 10/21/20 2:49 PM, Kieran Bingham wrote:
>>> Hi Umang,
>>>
>>> On 21/10/2020 09:08, Umang Jain wrote:
>>>> Add a basic image thumbnailer for NV12 frames being captured.
>>>> It shall generate a thumbnail image to be embedded as a part of
>>>> EXIF metadata of the frame. The output of the thumbnail will still
>>>> be NV12.
>>>>
>>>> Signed-off-by: Umang Jain <email at uajain.com>
>>>> ---
>>>> src/android/jpeg/post_processor_jpeg.cpp | 39 +++++++++
>>>> src/android/jpeg/post_processor_jpeg.h | 3 +
>>>> src/android/jpeg/thumbnailer.cpp | 100 +++++++++++++++++++++++
>>>> src/android/jpeg/thumbnailer.h | 40 +++++++++
>>>> src/android/meson.build | 1 +
>>>> 5 files changed, 183 insertions(+)
>>>> create mode 100644 src/android/jpeg/thumbnailer.cpp
>>>> create mode 100644 src/android/jpeg/thumbnailer.h
>>>>
>>>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp
>>>> b/src/android/jpeg/post_processor_jpeg.cpp
>>>> index 9d452b7..f5f1f78 100644
>>>> --- a/src/android/jpeg/post_processor_jpeg.cpp
>>>> +++ b/src/android/jpeg/post_processor_jpeg.cpp
>>>> @@ -11,6 +11,7 @@
>>>> #include "../camera_metadata.h"
>>>> #include "encoder_libjpeg.h"
>>>> #include "exif.h"
>>>> +#include "thumbnailer.h"
>>>> #include <libcamera/formats.h>
>>>> @@ -44,6 +45,32 @@ int PostProcessorJpeg::configure(const
>>>> StreamConfiguration &inCfg,
>>>> return encoder_->configure(inCfg);
>>>> }
>>>> +void PostProcessorJpeg::generateThumbnail(const
>>>> libcamera::Span<uint8_t> &source,
>>>> + std::vector<unsigned char> &thumbnail)
>>>> +{
>>>> + libcamera::Span<uint8_t> destination;
>>>> + Thumbnailer thumbnailer;
>>>> +
>>>> + thumbnailer.configure(streamSize_, formats::NV12);
>>>> + libcamera::Size targetSize = thumbnailer.computeThumbnailSize();
>>>> + thumbnailer.scaleBuffer(source, thumbnail);
>>>> +
>>>> + if (thumbnail.data()) {
>>>> + StreamConfiguration thumbnailCfg;
>>>> +
>>>> + std::unique_ptr<EncoderLibJpeg> encoder =
>>>> + std::make_unique<EncoderLibJpeg>();
>>>> +
>>>> + thumbnailCfg.pixelFormat = formats::NV12;
>>>> + thumbnailCfg.size = targetSize;
>>>> + encoder->configure(thumbnailCfg);
>>> thumbnail.capacity() might be quite low here.
>>> We need to make sure the vector is big enough at this point, you might
>>> need to do something like:
>>>
>>> thumbnail.resize(targetSize.width * targetSize.height * 4);
>> I am not sure I follow. This is compressing the thumbnail part right? So
>> thumbnail is the "source" for the encoder here (Please refer to
>> ->encode() below) and why would we resize the source(i.e. 'thumbnail')?
> Ugh, sorry - tired eyes indeed. Yes, please ignore those comments.
>
>
>
>>> Really we should obtain that size from the encoder. I thought we had a
>>> helper to do that already, but it seems we don't.
>>> We could/should add Encoder::maxOutput(); which would ask the encoder
>>> "For your given configuration, what is the maximum number of bytes you
>>> might output".
>>>
>>> Then of course we'd do:
>>> thumbnail.resize(encoder.maxOutput());
>>>
>>>
>>>> + int jpeg_size = encoder->encode({ thumbnail.data(),
>>>> thumbnail.capacity() },
>>>> + destination, { });
>> As said above, thumbnail is the source here. The compressed thumbnail
>> output is carried in destination. And, destination is a local span<>
>> here. Does it makes sense?
> Yes, I had mixed up source and destination I'm sorry.
>
> So who/where is allocating the destination?
Ah, that's a good question and I think I just found a bug. I currently
pass on a locally allocated Span<> 'destination' to the encoder. But
there is no explicit allocation here. I think I need to point / create
the Span with the output size / bytes and then pass to the encode().
>
>
>>>> + LOG(JPEG, Info) << "Thumbnail compress returned "
>>>> + << jpeg_size << " bytes";
>>> And I presume we could then do an:
>>> thumbnail.resize(jpeg_size);
>>>
>>> here to update the thumbnail with the correct size. I'd be weary of the
>>> resize operations doing lots of re-allocations though, so perhaps we
>>> want to minimize that. But lets get to something that works first before
>>> worrying about optimising.
>>>
>>>
>>>> + }
>>>> +}
>>>> +
>>>> int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,
>>>> const libcamera::Span<uint8_t> &destination,
>>>> CameraMetadata *metadata)
>>>> @@ -73,6 +100,18 @@ int PostProcessorJpeg::process(const
>>>> libcamera::FrameBuffer *source,
>>>> return jpeg_size;
>>>> }
>>>> + std::vector<unsigned char> thumbnail;
>>> You need to resize this somewhere.
>>> Edit: Now seen a better place above ;-)
>> The thumbnail vector just gets resized in Thumbnailer::scaleBuffer(). :)
>> We just pass in to the thumbnailer, while keeping its ownership in
>> PostProcessorJpeg.
>>>> + generateThumbnail(destination, thumbnail);
>>>> + /*
>>>> + * \todo: Write the compressed thumbnail to a file for inspection.
>>>> + * (I) Check if we can still write the thumbnail to EXIF here.
>>>> + * If not, we might need to move the thumbnailer logic to
>>>> encoder.
>>>> + * And if we do that, first we need to make sure we get can
>>>> + * compressed data written to destination first before calling
>>>> + * jpeg_finish_compress operation somehow. Thumbnailing will
>>>> + * only occur if we have compressed data available first.
>>>> + */
>>>> +
>>>> /*
>>>> * Fill in the JPEG blob header.
>>>> *
>>>> diff --git a/src/android/jpeg/post_processor_jpeg.h
>>>> b/src/android/jpeg/post_processor_jpeg.h
>>>> index 62c8650..05601ee 100644
>>>> --- a/src/android/jpeg/post_processor_jpeg.h
>>>> +++ b/src/android/jpeg/post_processor_jpeg.h
>>>> @@ -28,6 +28,9 @@ public:
>>>> CameraMetadata *metadata) override;
>>>> private:
>>>> + void generateThumbnail(const libcamera::Span<uint8_t> &source,
>>>> + std::vector<unsigned char> &thumbnail);
>>>> +
>>>> CameraDevice *cameraDevice_;
>>>> std::unique_ptr<Encoder> encoder_;
>>>> libcamera::Size streamSize_;
>>>> diff --git a/src/android/jpeg/thumbnailer.cpp
>>>> b/src/android/jpeg/thumbnailer.cpp
>>>> new file mode 100644
>>>> index 0000000..3163576
>>>> --- /dev/null
>>>> +++ b/src/android/jpeg/thumbnailer.cpp
>>>> @@ -0,0 +1,100 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Copyright (C) 2020, Google Inc.
>>>> + *
>>>> + * thumbnailer.cpp - Basic image thumbnailer from NV12
>>>> + */
>>>> +
>>>> +#include "thumbnailer.h"
>>>> +
>>>> +#include <libcamera/formats.h>
>>>> +
>>>> +#include "libcamera/internal/file.h"
>>>> +#include "libcamera/internal/log.h"
>>>> +
>>>> +using namespace libcamera;
>>>> +
>>>> +LOG_DEFINE_CATEGORY(Thumbnailer)
>>>> +
>>>> +Thumbnailer::Thumbnailer()
>>>> + : validConfiguration_(false)
>>>> +{
>>>> +}
>>>> +
>>>> +void Thumbnailer::configure(const Size &sourceSize, PixelFormat
>>>> pixelFormat)
>>>> +{
>>>> + sourceSize_ = sourceSize;
>>>> + pixelFormat_ = pixelFormat;
>>>> +
>>>> + if (pixelFormat_ != formats::NV12) {
>>>> + LOG (Thumbnailer, Error) << "Failed to configure: Pixel
>>>> Format "
>>>> + << pixelFormat_.toString() << " unsupported.";
>>>> + return;
>>>> + }
>>>> +
>>>> + validConfiguration_ = true;
>>>> +}
>>>> +
>>>> +/*
>>>> + * The Exif specification recommends the width of the thumbnail to be a
>>>> + * mutiple of 16 (section 4.8.1). Hence, compute the corresponding
>>>> height
>>>> + * keeping the aspect ratio same as of the source.
>>>> + */
>>>> +Size Thumbnailer::computeThumbnailSize()
>>>> +{
>>>> + unsigned int targetHeight;
>>>> + unsigned int targetWidth = 160;
>>>> +
>>>> + targetHeight = targetWidth * sourceSize_.height /
>>>> sourceSize_.width;
>>>> +
>>>> + if (targetHeight & 1)
>>>> + targetHeight++;
>>>> +
>>>> + return Size(targetWidth, targetHeight);
>>>> +}
>>>> +
>>>> +void
>>>> +Thumbnailer::scaleBuffer(const libcamera::Span<uint8_t> &source,
>>>> + std::vector<unsigned char> &destination)
>>>> +{
>>>> + if (!validConfiguration_) {
>>>> + LOG(Thumbnailer, Error) << "config is unconfigured or
>>>> invalid.";
>>>> + return;
>>>> + }
>>>> +
>>>> + targetSize_ = computeThumbnailSize();
>>>> +
>>>> + const unsigned int sw = sourceSize_.width;
>>>> + const unsigned int sh = sourceSize_.height;
>>>> + const unsigned int tw = targetSize_.width;
>>>> + const unsigned int th = targetSize_.height;
>>>> +
>>>> + /* Image scaling block implementing nearest-neighbour algorithm. */
>>>> + unsigned char *src = static_cast<unsigned char *>(source.data());
>>>> + unsigned char *src_c = src + sh * sw;
>>>> + unsigned char *src_cb, *src_cr;
>>>> +
>>>> + size_t dstSize = (th * tw) + ((th/2) * tw);
>>>> + destination.reserve(dstSize);
>>>> + unsigned char *dst = destination.data();
>>>> + unsigned char *dst_c = dst + th * tw;
>>>> +
>>>> + for (unsigned int y = 0; y < th; y+=2) {
>>>> + unsigned int sourceY = (sh*y + th/2) / th;
>>>> +
>>>> + src_cb = src_c + (sourceY/2) * sw + 0;
>>>> + src_cr = src_c + (sourceY/2) * sw + 1;
>>>> +
>>>> + for (unsigned int x = 0; x < tw; x+=2) {
>>>> + unsigned int sourceX = (sw*x + tw/2) / tw;
>>>> +
>>>> + dst[y * tw + x] = src[sw * sourceY + sourceX];
>>>> + dst[(y+1) * tw + x] = src[sw * (sourceY+1) + sourceX];
>>>> + dst[y * tw + (x+1)] = src[sw * sourceY +
>>>> (sourceX+1)];
>>>> + dst[(y+1) * tw + (x+1)] = src[sw * (sourceY+1) +
>>>> (sourceX+1)];
>>>> +
>>>> + dst_c[(y/2) * tw + x + 0] = src_cb[(sourceX/2) * 2];
>>>> + dst_c[(y/2) * tw + x + 1] = src_cr[(sourceX/2) * 2];
>>>> + }
>>>> + }
>>>> +}
>>>> diff --git a/src/android/jpeg/thumbnailer.h
>>>> b/src/android/jpeg/thumbnailer.h
>>>> new file mode 100644
>>>> index 0000000..bab9855
>>>> --- /dev/null
>>>> +++ b/src/android/jpeg/thumbnailer.h
>>>> @@ -0,0 +1,40 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Copyright (C) 2020, Google Inc.
>>>> + *
>>>> + * thumbnailer.h - Basic image thumbnailer from NV12
>>>> + */
>>>> +#ifndef __ANDROID_JPEG_THUMBNAILER_H__
>>>> +#define __ANDROID_JPEG_THUMBNAILER_H__
>>>> +
>>>> +#include <libcamera/geometry.h>
>>>> +
>>>> +#include "libcamera/internal/buffer.h"
>>>> +#include "libcamera/internal/formats.h"
>>>> +
>>>> +class Thumbnailer
>>>> +{
>>>> +public:
>>>> + Thumbnailer();
>>>> +
>>>> + void configure(const libcamera::Size &sourceSize,
>>>> + libcamera::PixelFormat pixelFormat);
>>>> +
>>>> + /*
>>>> + * \todo: Discuss if we can return targetSize_ via configure() or
>>>> + * scaleBuffer(). We need targetSize_ to re-encode the scaled
>>>> buffer
>>>> + * via encoder in PostProcssorJpeg::writeThumbnail().
>>>> + */
>>>> + libcamera::Size computeThumbnailSize();
>>>> + void scaleBuffer(const libcamera::Span<uint8_t> &source,
>>>> + std::vector<unsigned char> &dest);
>>>> +
>>>> +private:
>>>> + libcamera::PixelFormat pixelFormat_;
>>>> + libcamera::Size sourceSize_;
>>>> + libcamera::Size targetSize_;
>>>> +
>>>> + bool validConfiguration_;
>>>> +};
>>>> +
>>>> +#endif /* __ANDROID_JPEG_THUMBNAILER_H__ */
>>>> diff --git a/src/android/meson.build b/src/android/meson.build
>>>> index 5a01bea..3905e2f 100644
>>>> --- a/src/android/meson.build
>>>> +++ b/src/android/meson.build
>>>> @@ -25,6 +25,7 @@ android_hal_sources = files([
>>>> 'jpeg/encoder_libjpeg.cpp',
>>>> 'jpeg/exif.cpp',
>>>> 'jpeg/post_processor_jpeg.cpp',
>>>> + 'jpeg/thumbnailer.cpp',
>>>> ])
>>>> android_camera_metadata_sources = files([
>>>>
More information about the libcamera-devel
mailing list