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

[Umang Jain]

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

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