| Message ID | 20210923072453.130346-3-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Hirokazu Honda <hiroh@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? > + 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... Paul > + > uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; > requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode); > > -- > 2.31.0 >
Hi Paul, On 9/23/21 1:10 PM, paul.elder@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@ideasonboard.com> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Reviewed-by: Hirokazu Honda <hiroh@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. > > > Paul > >> + >> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; >> requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode); >> >> -- >> 2.31.0 >>
Hi again, On 9/23/21 1:23 PM, Umang Jain wrote: > Hi Paul, > > On 9/23/21 1:10 PM, paul.elder@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@ideasonboard.com> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>> Reviewed-by: Hirokazu Honda <hiroh@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 >>>
Hi Umang, On Thu, Sep 23, 2021 at 01:23:53PM +0530, Umang Jain wrote: > Hi Paul, > > On 9/23/21 1:10 PM, paul.elder@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@ideasonboard.com> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Reviewed-by: Hirokazu Honda <hiroh@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 That's true, but for example: {0, 0, w1, h1, w2, h2} Iteration 1: j = 0, so you take [0, 1]; afterward j += 2 Iteration 2: j = 2, so you take [2, 3]; afterward j += 2 Iteration 3: j = 4, so you take [4, 5]; afterward j += 2 Itreation 4: j = 6, fails j < entry.count; skip If you stop at entry.count / 2 then you'll skip the second half of pairs. Paul > > 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. > > > > > > > Paul > > > > > + > > > uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; > > > requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode); > > > -- > > > 2.31.0 > > >
Hi Umang, Thank you for the patch. 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@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Hirokazu Honda <hiroh@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; Anything wrong with i as a loop index ? > + while (j < entry.count / 2) { > + 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; > + } I think that's overkill really. Android requires the first entry to be { 0, 0 }. You can take the second entry unconditionally. > + > + 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); >
Hi Paul, On 9/23/21 3:29 PM, paul.elder@ideasonboard.com wrote: > Hi Umang, > > On Thu, Sep 23, 2021 at 01:23:53PM +0530, Umang Jain wrote: >> Hi Paul, >> >> On 9/23/21 1:10 PM, paul.elder@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@ideasonboard.com> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>>> Reviewed-by: Hirokazu Honda <hiroh@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 > That's true, but for example: > > {0, 0, w1, h1, w2, h2} > > Iteration 1: j = 0, so you take [0, 1]; afterward j += 2 > Iteration 2: j = 2, so you take [2, 3]; afterward j += 2 > Iteration 3: j = 4, so you take [4, 5]; afterward j += 2 > Itreation 4: j = 6, fails j < entry.count; skip > > If you stop at entry.count / 2 then you'll skip the second half of > pairs. Oh, you are correct. I missed to see that somehow :-S > > > Paul > >> 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. >> >>> >>> Paul >>> >>>> + >>>> uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; >>>> requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode); >>>> -- >>>> 2.31.0 >>>>
Hi Laurent, On 9/23/21 3:44 PM, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > 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@ideasonboard.com> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Reviewed-by: Hirokazu Honda <hiroh@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; > Anything wrong with i as a loop index ? > >> + while (j < entry.count / 2) { >> + 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; >> + } > I think that's overkill really. Android requires the first entry to be > { 0, 0 }. You can take the second entry unconditionally. Ok, I am keeping the ASSERT and dropping the loop, similar to v2. I'll merge the patches after validating CTS (which could take a while to get in) > >> + >> + 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); >>
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) { + 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); + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; requestTemplate->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
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@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> --- src/android/camera_capabilities.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)