<div dir="ltr">Sure and thanks, Umang!<div><br></div><div>BR,</div><div>Harvey</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 22, 2022 at 8:43 PM Umang Jain <<a href="mailto:umang.jain@ideasonboard.com">umang.jain@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<br>
Thank you for other version.<br>
<br>
I would re-write the subject line with:<br>
<br>
     android: exif: Fix thumbnail buffer lifetime<br>
<br>
It can be handled during integration, no need for v3.<br>
<br>
On 7/22/22 13:59, Laurent Pinchart via libcamera-devel wrote:<br>
> Hi Harvey,<br>
><br>
> Thank you for the patch.<br>
><br>
> On Fri, Jul 22, 2022 at 07:06:00AM +0000, Harvey Yang via libcamera-devel wrote:<br>
>> Previously the thumbnail buffer is destructed before even being used in<br>
>> Exif. This patch moves the buffer into class Exif, so that the developer<br>
>> won't need to worry about its lifetime.<br>
>><br>
>> Signed-off-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
<br>
Again,<br>
<br>
Reviewed-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
<br>
><br>
>> ---<br>
>>   src/android/jpeg/exif.cpp                | 13 +++++--------<br>
>>   src/android/jpeg/exif.h                  |  5 ++++-<br>
>>   src/android/jpeg/post_processor_jpeg.cpp |  2 +-<br>
>>   3 files changed, 10 insertions(+), 10 deletions(-)<br>
>><br>
>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp<br>
>> index 3220b458..6b1d0f1f 100644<br>
>> --- a/src/android/jpeg/exif.cpp<br>
>> +++ b/src/android/jpeg/exif.cpp<br>
>> @@ -430,16 +430,13 @@ void Exif::setOrientation(int orientation)<br>
>>      setShort(EXIF_IFD_0, EXIF_TAG_ORIENTATION, value);<br>
>>   }<br>
>>   <br>
>> -/*<br>
>> - * The thumbnail data should remain valid until the Exif object is destroyed.<br>
>> - * Failing to do so, might result in no thumbnail data being set even after a<br>
>> - * call to Exif::setThumbnail().<br>
>> - */<br>
>> -void Exif::setThumbnail(Span<const unsigned char> thumbnail,<br>
>> +void Exif::setThumbnail(std::vector<unsigned char> &&thumbnail,<br>
>>                      Compression compression)<br>
>>   {<br>
>> -    data_->data = const_cast<unsigned char *>(thumbnail.data());<br>
>> -    data_->size = thumbnail.size();<br>
>> +    thumbnailData_ = std::move(thumbnail);<br>
>> +<br>
>> +    data_->data = thumbnailData_.data();<br>
>> +    data_->size = thumbnailData_.size();<br>
>>   <br>
>>      setShort(EXIF_IFD_0, EXIF_TAG_COMPRESSION, compression);<br>
>>   }<br>
>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h<br>
>> index 2ff8fb78..e68716f3 100644<br>
>> --- a/src/android/jpeg/exif.h<br>
>> +++ b/src/android/jpeg/exif.h<br>
>> @@ -10,6 +10,7 @@<br>
>>   #include <chrono><br>
>>   #include <string><br>
>>   #include <time.h><br>
>> +#include <vector><br>
>>   <br>
>>   #include <libexif/exif-data.h><br>
>>   <br>
>> @@ -60,7 +61,7 @@ public:<br>
>>   <br>
>>      void setOrientation(int orientation);<br>
>>      void setSize(const libcamera::Size &size);<br>
>> -    void setThumbnail(libcamera::Span<const unsigned char> thumbnail,<br>
>> +    void setThumbnail(std::vector<unsigned char> &&thumbnail,<br>
>>                        Compression compression);<br>
>>      void setTimestamp(time_t timestamp, std::chrono::milliseconds msec);<br>
>>   <br>
>> @@ -106,4 +107,6 @@ private:<br>
>>   <br>
>>      unsigned char *exifData_;<br>
>>      unsigned int size_;<br>
>> +<br>
>> +    std::vector<unsigned char> thumbnailData_;<br>
>>   };<br>
>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp<br>
>> index d72ebc3c..0cf56716 100644<br>
>> --- a/src/android/jpeg/post_processor_jpeg.cpp<br>
>> +++ b/src/android/jpeg/post_processor_jpeg.cpp<br>
>> @@ -166,7 +166,7 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu<br>
>>                      std::vector<unsigned char> thumbnail;<br>
>>                      generateThumbnail(source, thumbnailSize, quality, &thumbnail);<br>
>>                      if (!thumbnail.empty())<br>
>> -                            exif.setThumbnail(thumbnail, Exif::Compression::JPEG);<br>
>> +                            exif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG);<br>
>>              }<br>
>>   <br>
>>              resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2);<br>
</blockquote></div>