[libcamera-devel] [PATCH v2 1/1] Fix Android adaptor thumbnail buffer lifetime

[Laurent Pinchart]

Hi Harvey,

Thank you for the patch.

On Fri, Jul 22, 2022 at 07:06:00AM +0000, Harvey Yang via libcamera-devel wrote:
> Previously the thumbnail buffer is destructed before even being used in
> Exif. This patch moves the buffer into class Exif, so that the developer
> won't need to worry about its lifetime.
> 
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/android/jpeg/exif.cpp                | 13 +++++--------
>  src/android/jpeg/exif.h                  |  5 ++++-
>  src/android/jpeg/post_processor_jpeg.cpp |  2 +-
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 3220b458..6b1d0f1f 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -430,16 +430,13 @@ void Exif::setOrientation(int orientation)
>  	setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);
>  }
>  
> -/*
> - * The thumbnail data should remain valid until the Exif object is destroyed.
> - * Failing to do so, might result in no thumbnail data being set even after a
> - * call to Exif::setThumbnail().
> - */
> -void Exif::setThumbnail(Span<const unsigned char> thumbnail,
> +void Exif::setThumbnail(std::vector<unsigned char> &&thumbnail,
>  			Compression compression)
>  {
> -	data_->data = const_cast<unsigned char *>(thumbnail.data());
> -	data_->size = thumbnail.size();
> +	thumbnailData_ = std::move(thumbnail);
> +
> +	data_->data = thumbnailData_.data();
> +	data_->size = thumbnailData_.size();
>  
>  	setShort(EXIF_IFD_0, EXIF_TAG_COMPRESSION, compression);
>  }
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 2ff8fb78..e68716f3 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -10,6 +10,7 @@
>  #include <chrono>
>  #include <string>
>  #include <time.h>
> +#include <vector>
>  
>  #include <libexif/exif-data.h>
>  
> @@ -60,7 +61,7 @@ public:
>  
>  	void setOrientation(int orientation);
>  	void setSize(const libcamera::Size &size);
> -	void setThumbnail(libcamera::Span<const unsigned char> thumbnail,
> +	void setThumbnail(std::vector<unsigned char> &&thumbnail,
>  			  Compression compression);
>  	void setTimestamp(time_t timestamp, std::chrono::milliseconds msec);
>  
> @@ -106,4 +107,6 @@ private:
>  
>  	unsigned char *exifData_;
>  	unsigned int size_;
> +
> +	std::vector<unsigned char> thumbnailData_;
>  };
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index d72ebc3c..0cf56716 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -166,7 +166,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu
>  			std::vector<unsigned char> thumbnail;
>  			generateThumbnail(source, thumbnailSize, quality, &thumbnail);
>  			if (!thumbnail.empty())
> -				exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
> +				exif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG);
>  		}
>  
>  		resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2);

-- 
Regards,

Laurent Pinchart