[libcamera-devel] [PATCH] wip: android: Fix generation of EXIF thumbnail

[Hirokazu Honda]

Hi Umang,

On Tue, Sep 7, 2021 at 6:53 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On 9/7/21 2:49 PM, Hirokazu Honda wrote:
> > Hi Umang, thank you for the patch.
> >
> > On Tue, Sep 7, 2021 at 3:01 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
> >> Generation of EXIF thumbnail seems to have regressed due to
> >> 1264628d3c92("android: jpeg: Configure thumbnailer based on request
> >> metadata"). The patch tries to fix the issue of
> >> ANDROID_JPEG_THUMBNAIL_SIZE not being available in request metadata
> >> template.
> >>
> >> However, there are a few open questions marked as comments in the
> >> patch, which should be addressed before merging the fix.
> >>
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >>   src/android/camera_capabilities.cpp      | 17 ++++++++++++++++-
> >>   src/android/jpeg/post_processor_jpeg.cpp |  1 +
> >>   2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> >> index e92bca42..e7f91020 100644
> >> --- a/src/android/camera_capabilities.cpp
> >> +++ b/src/android/camera_capabilities.cpp
> >> @@ -951,7 +951,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >>           */
> >>          constexpr Size maxJpegThumbnail(160, 160);
> >>          std::vector<Size> thumbnailSizes;
> >> -       thumbnailSizes.push_back({ 0, 0 });
> >> +       // thumbnailSizes.push_back({ 0, 0 }); not sure why this was added in first place?
> > Looking at the spec, (0, 0) is required.
> > If JPEG_THUMNAIL_SIZE is (0, 0), we should not generate a thumbnail.
> > (0, 0) indicates no thumbnail generation is supported.
> > https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics#JPEG_AVAILABLE_THUMBNAIL_SIZES
>
>
> Yes, I think it's required after reading the doc. While reading the code
> it didn't made any sense as why one would require a thumbnail of 0x0.
>
> >
> >>          for (const auto &entry : streamConfigurations_) {
> >>                  if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)
> >>                          continue;
> >> @@ -1364,6 +1364,21 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
> >>          requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,
> >>                                    entry.data.i32, 2);
> >>
> >> +       /*
> >> +        * Get Thumbnail sizes from static metadata and add to template.
> >> +        *
> >> +        * \todo Make sure you update entry size and byte size above to
> >> +        *  incorporate this.
> >> +        */
> >> +       found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,
> >> +                                         &entry);
> >> +       if (!found) {
> >> +               LOG(HAL, Error) << "ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES not found";
> >> +               // return here?
> >> +       }
> >> +       requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,
> >> +                                 entry.data.i32, 2);
> >> +
> > I think Size should be added here.
> > We should set just some non zero resolution just as a default.
> > Now I can understand why you would like to remove (0, 0) in the above.
> > I think we may want to add the second element of AVAILABLE_THUMBNAIL_SIZES?
>
>
> I am not sure here as well. Maybe the second (or say, first non-zero
> size) or maybe the highest size reported in AVAILABLE_THUMBNAIL_SIZES;
> that should used as target dimensions for the thumbnail. All I care, it
> should be non-zero :)

I wonder now if it should be zero. If there is a thumbnail, the
correct thumbnail size should be overwritten later?

-Hiro
>
> >
> > -Hiro
> >
> >>          uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;
> >>          requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> >>
> >> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> >> index 3160a784..2671a1ab 100644
> >> --- a/src/android/jpeg/post_processor_jpeg.cpp
> >> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> >> @@ -150,6 +150,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> >>                          generateThumbnail(source, thumbnailSize, quality, &thumbnail);
> >>                          if (!thumbnail.empty())
> >>                                  exif.setThumbnail(thumbnail, Exif::Compression::JPEG);
> >> +                       LOG(JPEG, Info) << "====THUMBNAIL GENERATED AND SET====";
> >>                  }
> >>
> >>                  resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2);
> >> --
> >> 2.31.0
> >>