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

[Cheng-Hao Yang]

Sure and thanks, Umang!

BR,
Harvey

On Fri, Jul 22, 2022 at 8:43 PM Umang Jain <umang.jain at ideasonboard.com>
wrote:

> 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);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220723/5875ab76/attachment.htm>