[libcamera-devel] [RFC PATCH 2/2] android: jpeg: Add a basic NV12 image thumbnailer
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Oct 21 13:09:23 CEST 2020
Hi Umang,
On 21/10/2020 12:03, Umang Jain wrote:
> 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().
:-) Ok - then please apply all of my earlier comments to that part
instead. ;-)
We certainly need somewhere to store the compressed image, to be able to
pass it into the exif :-D
--
Kieran
>>>>> + 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([
>>>>>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list