[libcamera-devel] [RFC PATCH 2/2] android: jpeg: Add a basic NV12 image thumbnailer

Umang Jain email at uajain.com
Wed Oct 21 12:51:20 CEST 2020


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')?
>
>
> 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?
>> +		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