[libcamera-devel] [PATCH v3 2/2] android: camera_device: Generate JPEG thumbnail sizes

[Jacopo Mondi]

Hi Laurent,

On Thu, Feb 04, 2021 at 07:10:13PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Feb 04, 2021 at 05:22:44PM +0100, Jacopo Mondi wrote:
> > The list of the available thumbnail sizes is generated from the
> > list of available JPEG resolution, one for each aspect ratio.
> >
> > This change fixes the CTS test
> > android.hardware.cts.CameraTest#testJpegThumbnailSize
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 55 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 48 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 70170e243d06..65d37860cf95 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -705,10 +705,10 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  {
> >  	/*
> >  	 * \todo Keep this in sync with the actual number of entries.
> > -	 * Currently: 53 entries, 846 bytes of static metadata
> > +	 * Currently: 53 entries, 838 bytes of static metadata
> >  	 */
> >  	uint32_t numEntries = 53;
> > -	uint32_t byteSize = 846;
> > +	uint32_t byteSize = 838;
> >
> >  	/*
> >  	 * Calculate space occupation in bytes for dynamically built metadata
> > @@ -720,6 +720,18 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  	 */
> >  	byteSize += streamConfigurations_.size() * 48;
> >
> > +	/*
> > +	 * 2 32bits integers for each HAL_PIXEL_FORMAT_BLOB for thumbnail sizes
> > +	 * 2 32bits integers for the (0, 0) thumbnail size
>
> I'd add
>
> 	* This is a worst case estimates as different configurations with the
> 	* same aspect ratio will generate the same size.
>
> (and there's no need to optimize this)
>
> > +	 */
> > +	for (const auto &entry : streamConfigurations_) {
> > +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)
> > +			continue;
> > +
> > +		byteSize += 8;
> > +	}
> > +	byteSize += 8;
> > +
> >  	return std::make_tuple(numEntries, byteSize);
> >  }
> >
> > @@ -871,12 +883,41 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  &availableControlModes, 1);
> >
> >  	/* JPEG static metadata. */
> > -	std::vector<int32_t> availableThumbnailSizes = {
> > -		0, 0,
> > -	};
> > +
> > +	/*
> > +	 * Create the list of supported thumbnail sizes by inspecting the
> > +	 * available JPEG resolutions collected in streamConfigurations_ and
> > +	 * generate one entry for each aspect ratio.
> > +	 *
> > +	 * The JPEG thumbnailer can freely scale, so pick an arbitrary
> > +	 * (160, 160) size as designated thumbnail size.
>
> Maybe
>
> 	 * (160, 160) size as the bounding rectangle, which is then cropped to
> 	 * the different supported aspect ratios.
>
> or something similar ?
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Sure, I'll take your comments in and I'll push.

Thanks
  j

>
> > +	 */
> > +	constexpr Size maxJpegThumbnail(160, 160);
> > +	std::vector<Size> thumbnailSizes;
> > +	thumbnailSizes.push_back({ 0, 0 });
> > +	for (const auto &entry : streamConfigurations_) {
> > +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)
> > +			continue;
> > +
> > +		Size thumbnailSize = maxJpegThumbnail
> > +				     .boundedToAspectRatio({ entry.resolution.width,
> > +							     entry.resolution.height });
> > +		thumbnailSizes.push_back(thumbnailSize);
> > +	}
> > +
> > +	std::sort(thumbnailSizes.begin(), thumbnailSizes.end());
> > +	auto last = std::unique(thumbnailSizes.begin(), thumbnailSizes.end());
> > +	thumbnailSizes.erase(last, thumbnailSizes.end());
> > +
> > +	/* Transform sizes in to a list of integers that can be consumed. */
> > +	std::vector<int32_t> thumbnailEntries;
> > +	thumbnailEntries.reserve(thumbnailSizes.size() * 2);
> > +	for (const auto &size : thumbnailSizes) {
> > +		thumbnailEntries.push_back(size.width);
> > +		thumbnailEntries.push_back(size.height);
> > +	}
> >  	staticMetadata_->addEntry(ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,
> > -				  availableThumbnailSizes.data(),
> > -				  availableThumbnailSizes.size());
> > +				  thumbnailEntries.data(), thumbnailEntries.size());
> >
> >  	/*
> >  	 * \todo Calculate the maximum JPEG buffer size by asking the encoder
>
> --
> Regards,
>
> Laurent Pinchart