[libcamera-devel] [RFC PATCH 2/2] android: jpeg: Add a basic NV12 image thumbnailer
Hirokazu Honda
hiroh at chromium.org
Thu Oct 22 13:42:01 CEST 2020
Hi Umang,
Thanks for the work. I couldn't have time to review these today and
will review tomorrow.
In my understanding, the thumbnail class is basically to scale frames
to the given destination.
Can we generalize it as PostProcessor interface so that we will be
able to make use of it for down-scaling regardless of whether jpeg
encoding is required.
It will be necessary when Android HAL adaptation layer has to output
multiple different NV12 streams while the native camera can produce a
single NV12 stream simultaneously.
Best Regards,
-Hiro
On Wed, Oct 21, 2020 at 8:09 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> 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
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list