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

[Umang Jain]

Hello,

Thank you for other version.

I would re-write the subject line with:

     android: exif: Fix thumbnail buffer lifetime

It can be handled during integration, no need for v3.

On 7/22/22 13:59, Laurent Pinchart via libcamera-devel wrote:
> 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>

Again,

Reviewed-by: Umang Jain <umang.jain 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);