| Message ID | 20200721220126.202065-4-kieran.bingham@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
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}); > 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}); > > Note: The metadata entries/size updates are probably wrong, and should > be automated. What does make you think so ? > > Signed-off-by: Kieran Bingham <kieran.bingham@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; > > /* > * 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 ? > /* 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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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}); >> 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}); >> >> 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. > >> >> Signed-off-by: Kieran Bingham <kieran.bingham@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; >> >> /* >> * 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 ;-) >> /* 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@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
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@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@lists.libcamera.org > >> https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards > -- > Kieran
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; /* * 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); + /* 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,
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}); 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}); Note: The metadata entries/size updates are probably wrong, and should be automated. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/android/camera_device.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)