[libcamera-devel] [PATCH v1 2/2] android: Fix generation of thumbnail for EXIF data

[Hirokazu Honda]

Hi Umang,

On Fri, Sep 10, 2021 at 11:31 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hi Hiro
>
> On 9/10/21 6:24 PM, Hirokazu Honda wrote:
> > Hi Umang, thank you for the patch.
> >
> > On Fri, Sep 10, 2021 at 7:47 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
> >> Generation of thumbnail is not occuring currently because
> >> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed
> >> to PostProcessorJpeg::process(). This is a regression introduced in
> >> 1264628d3c92("android: jpeg: Configure thumbnailer based on request
> >> metadata").
> >>
> >> The patch fixes this issue by setting ANDROID_JPEG_THUMBNAIL_SIZE in
> >> the request metadata template populated by
> >> CameraCapabilities::requestTemplatePreview(). The value for
> >> ANDROID_JPEG_THUMBNAIL_SIZE is set to be the first non-zero size
> >> reported by static metadata ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES.
> >>
> >> Fixes: 1264628d3c92("android: jpeg: Configure thumbnailer based on request metadata")
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >>   src/android/camera_capabilities.cpp | 17 +++++++++++++++--
> >>   1 file changed, 15 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> >> index e92bca42..ba551b86 100644
> >> --- a/src/android/camera_capabilities.cpp
> >> +++ b/src/android/camera_capabilities.cpp
> >> @@ -1341,9 +1341,9 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
> >>   {
> >>          /*
> >>           * \todo Keep this in sync with the actual number of entries.
> >> -        * Currently: 20 entries, 35 bytes
> >> +        * Currently: 21 entries, 37 bytes
> >>           */
> >> -       auto requestTemplate = std::make_unique<CameraMetadata>(21, 36);
> >> +       auto requestTemplate = std::make_unique<CameraMetadata>(22, 38);
> >>          if (!requestTemplate->isValid()) {
> >>                  return nullptr;
> >>          }
> >> @@ -1364,6 +1364,19 @@ 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 the first non-zero
> >> +        * size to the template.
> >> +        */
> >> +       found = staticMetadata_->getEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,
> >> +                                         &entry);
> >> +       if (!found) {
> >> +               LOG(HAL, Error) << "availableThumbnailSizes not found in static metadata";
> >> +               return nullptr;
> >> +       }
> > Is it possible to check if there is the second element?
>
> Second element ? Do you mean second (or other sizes) being reported in
> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES?
>
> It should be plausible to extract that from the entry's data I think.
> Is something unclear?
>

I meant if SIZES returns empty size (i.e. the sizes are one size
list), accessing entry.data.i32+2 is invalid.
I wonder if we should check the size of the returned size list.

-Hiro
> > -Hiro
> >> +       requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,
> >> +                                 entry.data.i32 + 2, 2);
> >> +
> >>          uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;
> >>          requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> >>
> >> --
> >> 2.31.0
> >>