[libcamera-devel] [RFC PATCH 3/6] android: camera_device: Add ANDROID_JPEG_MAX_SIZE

[Jacopo Mondi]

Hi Kieran

On Wed, Jul 22, 2020 at 12:12:10PM +0100, Kieran Bingham wrote:
>
>
> On 22/07/2020 11:35, Jacopo Mondi wrote:
> > Hi Kieran
> >
> > On Tue, Jul 21, 2020 at 11:01:23PM +0100, Kieran Bingham wrote:
> >> Provide a key for ANDROID_JPEG_MAX_SIZE. The value of 13MB is currently
> >> arbitrary, but taken from the USB Camera HAL in CrOS platform2:
> >>
> >>   // android.jpeg
> >>   update_static(ANDROID_JPEG_MAX_SIZE, int32_t{13 << 20});

This is a static control

> >>   update_request(ANDROID_JPEG_QUALITY, uint8_t{90});
> >>   update_request(ANDROID_JPEG_THUMBNAIL_QUALITY, uint8_t{90});
> >>   update_request(ANDROID_JPEG_ORIENTATION, int32_t{0});
> >>

This are dynamics

> >> Note: The metadata entries/size updates are probably wrong, and should
> >> be automated.
> >
> > What does make you think so ?
>
> Because I did it by hand, and when I wrote this I was getting arbitrary
> crashes of the whole GUI which I had not been able to pin down.
>

Do you refer to the sizes of the dynamic controls here only, or to the
static metadata pack size ?

>
> >
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >> ---
> >>  src/android/camera_device.cpp | 10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >> index 3f3d7857f0ab..56652ac57676 100644
> >> --- a/src/android/camera_device.cpp
> >> +++ b/src/android/camera_device.cpp
> >> @@ -407,10 +407,10 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >>  {
> >>  	/*
> >>  	 * \todo Keep this in sync with the actual number of entries.
> >> -	 * Currently: 50 entries, 647 bytes of static metadata
> >> +	 * Currently: 51 entries, 651 bytes of static metadata
> >>  	 */
> >> -	uint32_t numEntries = 50;
> >> -	uint32_t byteSize = 647;
> >> +	uint32_t numEntries = 51;
> >> +	uint32_t byteSize = 651;

I mean this one ^

> >>
> >>  	/*
> >>  	 * Calculate space occupation in bytes for dynamically built metadata
> >> @@ -567,6 +567,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >>  				  availableThumbnailSizes.data(),
> >>  				  availableThumbnailSizes.size());
> >>
> >> +	int32_t jpegMaxSize = 13 << 20; /* 13631488 from USB HAL */
> >> +	staticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &jpegMaxSize, 1);
> >> +
> >
> > Makes sense as long as we don't have a more precise value.
> > Where should this come from ? By combining the frame size with libjpeg
> > features ?
>
>
> We can ask libjpeg what size it would require, *if* we have the relevant
> configuration parameters for libjpeg. So we could construct a
> CompressorJPEG and give it a 'maximum' expected configuration, and then
> add a function to return the expected/required maximum buffer size from
> that.
>
> But it's a bit chicken/egg, as this happens before we configure streams
> - so we'd have to have created a temporary compressor object (or just
> provide a static function on a compressor that says "given this
> configuration, what is the maximum size" ... then the compression object
> can do what it likes to return the value.
>
> Ok that makes more sense so that woudl be somethign like:
>
> max_size = myCompressor->maxBufferSize(StreamConfiguration &cfg);
>
> That doesn't seem too outlandish ;-)
>

My understanding is that the size computed on the maximum frame size a
camera can produce is what android expects

"Maximum size in bytes for the compressed JPEG buffer
Must be large enough to fit any JPEG produced by the camera"

I would go for a static method which given a size/configuration
returns the required size in bytes

>
> >>  	/* Sensor static metadata. */
> >>  	int32_t pixelArraySize[] = {
> >>  		2592, 1944,
> >> @@ -773,6 +776,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >>  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> >>  		ANDROID_CONTROL_AVAILABLE_MODES,
> >>  		ANDROID_JPEG_AVAILABLE_THUMBNAIL_SIZES,
> >> +		ANDROID_JPEG_MAX_SIZE,
> >>  		ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> >>  		ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> >>  		ANDROID_SENSOR_INFO_SENSITIVITY_RANGE,
> >> --
> >> 2.25.1
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran