[libcamera-devel] [PATCH v3 6/8] android: jpeg: Configure thumbnailer based on request metadata
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jan 23 10:39:37 CET 2021
Hi Paul,
Thank you for the patch.
On Sat, Jan 23, 2021 at 02:17:02PM +0900, Paul Elder wrote:
> Configure the thumbnailer based on the thumbnail parameters given by the
> android request metadata. Only the thumbnail encoder needs to be
> configured, and since it is only used at post-processing time, move the
> configuration out of the post-processor constructor and into the
> processing step.
>
> Also set the following android result metadata tags:
> - ANDROID_JPEG_THUMBNAIL_SIZE
> - ANDROID_JPEG_THUMBNAIL_QUALITY
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> New in v3
> - split from "android: Set result metadata and EXIF fields based on
> request"
> ---
> src/android/jpeg/post_processor_jpeg.cpp | 40 +++++++++++++++++-------
> src/android/jpeg/post_processor_jpeg.h | 1 +
> src/android/jpeg/thumbnailer.cpp | 25 ++-------------
> src/android/jpeg/thumbnailer.h | 4 +--
> 4 files changed, 34 insertions(+), 36 deletions(-)
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index b325a06a..e5e42d39 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -41,12 +41,6 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,
> streamSize_ = outCfg.size;
>
> thumbnailer_.configure(inCfg.size, inCfg.pixelFormat);
> - StreamConfiguration thCfg = inCfg;
> - thCfg.size = thumbnailer_.size();
> - if (thumbnailEncoder_.configure(thCfg) != 0) {
> - LOG(JPEG, Error) << "Failed to configure thumbnail encoder";
> - return -EINVAL;
> - }
>
> encoder_ = std::make_unique<EncoderLibJpeg>();
>
> @@ -54,14 +48,20 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,
> }
>
> void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
> + const Size &targetSize,
> std::vector<unsigned char> *thumbnail)
> {
> /* Stores the raw scaled-down thumbnail bytes. */
> std::vector<unsigned char> rawThumbnail;
>
> - thumbnailer_.createThumbnail(source, &rawThumbnail);
> + thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);
> +
> + StreamConfiguration thCfg;
> + thCfg.size = targetSize;
> + thCfg.pixelFormat = thumbnailer_.pixelFormat();
> + int ret = thumbnailEncoder_.configure(thCfg);
It may make sense to also pass the size and format to the encoder's
encode function, but that's a possible improvement on top.
>
> - if (!rawThumbnail.empty()) {
> + if (!rawThumbnail.empty() && !ret) {
> /*
> * \todo Avoid value-initialization of all elements of the
> * vector.
> @@ -125,10 +125,26 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> entry.data.i64, 1);
> }
>
> - std::vector<unsigned char> thumbnail;
> - generateThumbnail(source, &thumbnail);
> - if (!thumbnail.empty())
> - exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
> + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_SIZE, &entry);
> + if (ret) {
> + const int32_t *data = entry.data.i32;
> + Size thumbnailSize = { static_cast<uint32_t>(data[0]),
> + static_cast<uint32_t>(data[1]) };
> +
> + if (thumbnailSize != Size(0, 0)) {
> + std::vector<unsigned char> thumbnail;
> + generateThumbnail(source, thumbnailSize, &thumbnail);
> + if (!thumbnail.empty())
> + exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
> + }
> +
> + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2);
> +
> + /* \todo Use this quality as a parameter to the encoder */
> + ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry);
> + if (ret)
> + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, entry.data.u8, 1);
Line wrap ?
> + }
>
> ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry);
> if (ret) {
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index d721d1b9..660b79b4 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -31,6 +31,7 @@ public:
>
> private:
> void generateThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> std::vector<unsigned char> *thumbnail);
>
> CameraDevice *const cameraDevice_;
> diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
> index 5374538a..f709d343 100644
> --- a/src/android/jpeg/thumbnailer.cpp
> +++ b/src/android/jpeg/thumbnailer.cpp
> @@ -32,30 +32,11 @@ void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat)
> return;
> }
>
> - targetSize_ = computeThumbnailSize();
> -
> valid_ = true;
> }
>
> -/*
> - * The Exif specification recommends the width of the thumbnail to be a
> - * multiple of 16 (section 4.8.1). Hence, compute the corresponding height
> - * keeping the aspect ratio same as of the source.
> - */
> -Size Thumbnailer::computeThumbnailSize() const
> -{
> - unsigned int targetHeight;
> - constexpr unsigned int kTargetWidth = 160;
> -
> - targetHeight = kTargetWidth * sourceSize_.height / sourceSize_.width;
> -
> - if (targetHeight & 1)
> - targetHeight++;
> -
> - return Size(kTargetWidth, targetHeight);
> -}
> -
> void Thumbnailer::createThumbnail(const FrameBuffer &source,
> + const Size &targetSize,
> std::vector<unsigned char> *destination)
> {
> MappedFrameBuffer frame(&source, PROT_READ);
> @@ -73,8 +54,8 @@ void Thumbnailer::createThumbnail(const FrameBuffer &source,
>
> const unsigned int sw = sourceSize_.width;
> const unsigned int sh = sourceSize_.height;
> - const unsigned int tw = targetSize_.width;
> - const unsigned int th = targetSize_.height;
> + const unsigned int tw = targetSize.width;
> + const unsigned int th = targetSize.height;
>
> ASSERT(tw % 2 == 0 && th % 2 == 0);
>
> diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h
> index 98f11833..bbd9832d 100644
> --- a/src/android/jpeg/thumbnailer.h
> +++ b/src/android/jpeg/thumbnailer.h
> @@ -20,15 +20,15 @@ public:
> void configure(const libcamera::Size &sourceSize,
> libcamera::PixelFormat pixelFormat);
> void createThumbnail(const libcamera::FrameBuffer &source,
> + const libcamera::Size &targetSize,
> std::vector<unsigned char> *dest);
> - const libcamera::Size &size() const { return targetSize_; }
> + const libcamera::PixelFormat &pixelFormat() const { return pixelFormat_; }
>
> private:
> libcamera::Size computeThumbnailSize() const;
This should be dropped as the function is removed.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> libcamera::PixelFormat pixelFormat_;
> libcamera::Size sourceSize_;
> - libcamera::Size targetSize_;
>
> bool valid_;
> };
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list