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

[Hirokazu Honda]

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

>         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?

-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
>