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

[Umang Jain]

Hi again,

On 9/23/21 1:23 PM, Umang Jain wrote:
> Hi Paul,
>
> On 9/23/21 1:10 PM, paul.elder at ideasonboard.com wrote:
>> Hi Umang,
>>
>> On Thu, Sep 23, 2021 at 12:54:53PM +0530, Umang Jain wrote:
>>> Generation of thumbnail is not occuring currently because
>>> ANDROID_JPEG_THUMBNAIL_SIZE is not set for request metadata passed
>>> to PostProcessorJpeg::process(). The commit 1264628d3c92("android:
>>> jpeg: Configure thumbnailer based on request metadata") introduced
>>> the mechanism to retrieve the thumbanil size from request metadata,
>>> however it didn't add the counterpart i.e. inserting the size in
>>> the request metadata in request metadata template, at the first place.
>>>
>>> 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>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
>>> ---
>>>   src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++-
>>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/android/camera_capabilities.cpp 
>>> b/src/android/camera_capabilities.cpp
>>> index 238b44db..ba6adf73 100644
>>> --- a/src/android/camera_capabilities.cpp
>>> +++ b/src/android/camera_capabilities.cpp
>>> @@ -1347,7 +1347,7 @@ std::unique_ptr<CameraMetadata> 
>>> CameraCapabilities::requestTemplatePreview() con
>>>        * CameraMetadata is capable to resize the container on the 
>>> fly, if the
>>>        * number of entries get exceeded.
>>>        */
>>> -    auto requestTemplate = std::make_unique<CameraMetadata>(21, 36);
>>> +    auto requestTemplate = std::make_unique<CameraMetadata>(22, 38);
>>>       if (!requestTemplate->isValid()) {
>>>           return nullptr;
>>>       }
>>> @@ -1368,6 +1368,28 @@ 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);
>>> +    ASSERT(found && entry.count >= 4);
>>> +    unsigned int j = 0;
>>> +    while (j < entry.count / 2) {
>> Since you're incrementing j by 2, shouldn't the limit be entry.count?
>
>
> My understanding is 2 entries make 1 size, no? 
> ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES be like for e.g.
>
>     {0, 0, w1, h1, w2, h2}
>
> So entry.count is 6 but sizes are 3
>
> To go to next Size, incremement by 2 so, j += 2
>
> Does it make sense?
>
>
>>
>>> +        if (entry.data.i32[j] == 0 || entry.data.i32[j + 1] == 0) {
>>> +            j += 2;
>>> +            continue;
>>> +        }
>>> +
>>> + requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,
>>> +                      entry.data.i32 + j, 2);
>>> +        break;
>>> +    }
>>> +
>>> +    requestTemplate->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE,
>>> +                  entry.data.i32 + 2, 2);
>> iirc adding an entry that already exists causes weird problems...
>
>
> Ah crap, this should be outside the loop. I am re-assigning it here. 
> Sorry, rebase fiasco it seems.


err,  I meant:

This should _not_ be outside the loop.

>
>>
>>
>> Paul
>>
>>> +
>>>       uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON;
>>>       requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
>>>   --
>>> 2.31.0
>>>