Message ID | 20200724142120.95538-7-jacopo@jmondi.org |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, Thank you for the patch. On Fri, Jul 24, 2020 at 04:21:20PM +0200, Jacopo Mondi wrote: > Add 5 controls to the generate preview template to comply with the > camera3 specification. > > This change fixes CTS 9.0.r12 test [14/253] > android.hardware.camera2.cts.CameraDeviceTest#testCameraDevicePreviewTemplate > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index b8cb118a960e..58901a119c18 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() > * Currently: 50 entries, 647 bytes of static metadata > */ > uint32_t numEntries = 50; > - uint32_t byteSize = 647; > + uint32_t byteSize = 667; > > /* > * Calculate space occupation in bytes for dynamically built metadata > @@ -779,6 +779,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > ANDROID_CONTROL_AE_MODE, > ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > + ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > + ANDROID_CONTROL_AE_ANTIBANDING_MODE, > ANDROID_CONTROL_AE_LOCK, > ANDROID_CONTROL_AF_TRIGGER, > ANDROID_CONTROL_AWB_MODE, > @@ -787,6 +789,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > ANDROID_STATISTICS_FACE_DETECT_MODE, > ANDROID_NOISE_REDUCTION_MODE, > ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > + ANDROID_LENS_APERTURE, > + ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > + ANDROID_CONTROL_MODE, > ANDROID_CONTROL_CAPTURE_INTENT, > }; > staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > @@ -825,9 +830,9 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > { > /* > * \todo Keep this in sync with the actual number of entries. > - * Currently: 12 entries, 15 bytes > + * Currently: 20 entries, 35 bytes > */ > - CameraMetadata *requestTemplate = new CameraMetadata(15, 20); > + CameraMetadata *requestTemplate = new CameraMetadata(20, 35); It's lovely how (15, 20) didn't match the comment :-) I'll be happy when we'll get a better API. > if (!requestTemplate->isValid()) > return nullptr; > > @@ -847,6 +852,17 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > requestTemplate->addEntry(ANDROID_CONTROL_AE_LOCK, > &aeLock, 1); > > + std::vector<int32_t> aeFpsTarget = { > + 15, 30, > + }; > + requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > + aeFpsTarget.data(), > + aeFpsTarget.size()); > + > + uint8_t aeAntibandingMode = ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO; Should we report OFF instead, as we don't support antibanding at the moment ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + requestTemplate->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, > + &aeAntibandingMode, 1); > + > uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > requestTemplate->addEntry(ANDROID_CONTROL_AF_TRIGGER, > &afTrigger, 1); > @@ -875,6 +891,16 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > requestTemplate->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > &aberrationMode, 1); > > + uint8_t controlMode = ANDROID_CONTROL_MODE_AUTO; > + requestTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1); > + > + float lensAperture = 2.53 / 100; > + requestTemplate->addEntry(ANDROID_LENS_APERTURE, &lensAperture, 1); > + > + uint8_t opticalStabilization = ANDROID_LENS_OPTICAL_STABILIZATION_MODE_OFF; > + requestTemplate->addEntry(ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > + &opticalStabilization, 1); > + > uint8_t captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT, > &captureIntent, 1);
Hi Laurent, On Fri, Jul 24, 2020 at 07:07:15PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Fri, Jul 24, 2020 at 04:21:20PM +0200, Jacopo Mondi wrote: > > Add 5 controls to the generate preview template to comply with the > > camera3 specification. > > > > This change fixes CTS 9.0.r12 test [14/253] > > android.hardware.camera2.cts.CameraDeviceTest#testCameraDevicePreviewTemplate > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++--- > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > index b8cb118a960e..58901a119c18 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() > > * Currently: 50 entries, 647 bytes of static metadata > > */ > > uint32_t numEntries = 50; > > - uint32_t byteSize = 647; > > + uint32_t byteSize = 667; > > > > /* > > * Calculate space occupation in bytes for dynamically built metadata > > @@ -779,6 +779,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > > ANDROID_CONTROL_AE_MODE, > > ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > + ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE, > > ANDROID_CONTROL_AE_LOCK, > > ANDROID_CONTROL_AF_TRIGGER, > > ANDROID_CONTROL_AWB_MODE, > > @@ -787,6 +789,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > > ANDROID_STATISTICS_FACE_DETECT_MODE, > > ANDROID_NOISE_REDUCTION_MODE, > > ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > > + ANDROID_LENS_APERTURE, > > + ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > > + ANDROID_CONTROL_MODE, > > ANDROID_CONTROL_CAPTURE_INTENT, > > }; > > staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > > @@ -825,9 +830,9 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > { > > /* > > * \todo Keep this in sync with the actual number of entries. > > - * Currently: 12 entries, 15 bytes > > + * Currently: 20 entries, 35 bytes > > */ > > - CameraMetadata *requestTemplate = new CameraMetadata(15, 20); > > + CameraMetadata *requestTemplate = new CameraMetadata(20, 35); > > It's lovely how (15, 20) didn't match the comment :-) I'll be happy when > we'll get a better API. > It's a real pain, yes > > if (!requestTemplate->isValid()) > > return nullptr; > > > > @@ -847,6 +852,17 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > requestTemplate->addEntry(ANDROID_CONTROL_AE_LOCK, > > &aeLock, 1); > > > > + std::vector<int32_t> aeFpsTarget = { > > + 15, 30, > > + }; > > + requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > > + aeFpsTarget.data(), > > + aeFpsTarget.size()); > > + > > + uint8_t aeAntibandingMode = ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO; > > Should we report OFF instead, as we don't support antibanding at the > moment ? CTS complains if I report OFF. I'm not sure I got why, as we report OFF in the available anti-banding modes reported with ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES. I think I'll stick with AUTO for the moment. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > + requestTemplate->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, > > + &aeAntibandingMode, 1); > > + > > uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > > requestTemplate->addEntry(ANDROID_CONTROL_AF_TRIGGER, > > &afTrigger, 1); > > @@ -875,6 +891,16 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > requestTemplate->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > > &aberrationMode, 1); > > > > + uint8_t controlMode = ANDROID_CONTROL_MODE_AUTO; > > + requestTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1); > > + > > + float lensAperture = 2.53 / 100; > > + requestTemplate->addEntry(ANDROID_LENS_APERTURE, &lensAperture, 1); > > + > > + uint8_t opticalStabilization = ANDROID_LENS_OPTICAL_STABILIZATION_MODE_OFF; > > + requestTemplate->addEntry(ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > > + &opticalStabilization, 1); > > + > > uint8_t captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > > requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT, > > &captureIntent, 1); > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Sat, Jul 25, 2020 at 02:36:52PM +0200, Jacopo Mondi wrote: > On Fri, Jul 24, 2020 at 07:07:15PM +0300, Laurent Pinchart wrote: > > On Fri, Jul 24, 2020 at 04:21:20PM +0200, Jacopo Mondi wrote: > > > Add 5 controls to the generate preview template to comply with the > > > camera3 specification. > > > > > > This change fixes CTS 9.0.r12 test [14/253] > > > android.hardware.camera2.cts.CameraDeviceTest#testCameraDevicePreviewTemplate > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++--- > > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > > index b8cb118a960e..58901a119c18 100644 > > > --- a/src/android/camera_device.cpp > > > +++ b/src/android/camera_device.cpp > > > @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() > > > * Currently: 50 entries, 647 bytes of static metadata > > > */ > > > uint32_t numEntries = 50; > > > - uint32_t byteSize = 647; > > > + uint32_t byteSize = 667; > > > > > > /* > > > * Calculate space occupation in bytes for dynamically built metadata > > > @@ -779,6 +779,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > > > ANDROID_CONTROL_AE_MODE, > > > ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > > ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > > + ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE, > > > ANDROID_CONTROL_AE_LOCK, > > > ANDROID_CONTROL_AF_TRIGGER, > > > ANDROID_CONTROL_AWB_MODE, > > > @@ -787,6 +789,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() > > > ANDROID_STATISTICS_FACE_DETECT_MODE, > > > ANDROID_NOISE_REDUCTION_MODE, > > > ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > > > + ANDROID_LENS_APERTURE, > > > + ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > > > + ANDROID_CONTROL_MODE, > > > ANDROID_CONTROL_CAPTURE_INTENT, > > > }; > > > staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > > > @@ -825,9 +830,9 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > > { > > > /* > > > * \todo Keep this in sync with the actual number of entries. > > > - * Currently: 12 entries, 15 bytes > > > + * Currently: 20 entries, 35 bytes > > > */ > > > - CameraMetadata *requestTemplate = new CameraMetadata(15, 20); > > > + CameraMetadata *requestTemplate = new CameraMetadata(20, 35); > > > > It's lovely how (15, 20) didn't match the comment :-) I'll be happy when > > we'll get a better API. > > It's a real pain, yes > > > > if (!requestTemplate->isValid()) > > > return nullptr; > > > > > > @@ -847,6 +852,17 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > > requestTemplate->addEntry(ANDROID_CONTROL_AE_LOCK, > > > &aeLock, 1); > > > > > > + std::vector<int32_t> aeFpsTarget = { > > > + 15, 30, > > > + }; > > > + requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, > > > + aeFpsTarget.data(), > > > + aeFpsTarget.size()); > > > + > > > + uint8_t aeAntibandingMode = ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO; > > > > Should we report OFF instead, as we don't support antibanding at the > > moment ? > > CTS complains if I report OFF. > > I'm not sure I got why, as we report OFF in the available anti-banding > modes reported with ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES. I think it's due to commit 9ecdbaf9b0f50dcb9bf456310415238b4767e59b Author: Yin-Chia Yeh <yinchiayeh@google.com> Date: Tue Nov 25 11:56:01 2014 -0800 Camera2: update antibanding spec Allow auto mode not always available. When auto is unavailable, the default should be either 50HZ or 60HZ and both 50HZ and 60HZ modes should be available. Bug: 18503791 Change-Id: I32c99ca64fe53d2cef8caaedd4208357b24221bc (https://android.googlesource.com/platform/cts) If auto mode isn't supported, CTS requires manual mode to be supported. It's actually documented in platform/system/media/camera/docs/docs.html: "AUTO mode is the default if it is available on given camera device. When AUTO mode is not available, the default will be either 50HZ or 60HZ, and both 50HZ and 60HZ will be available." > I think I'll stick with AUTO for the moment. If we mandate support of anti-banding in IPAs, it will be easier to implement manual mode and auto mode. Should we start by exposing manual mode ? > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + requestTemplate->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, > > > + &aeAntibandingMode, 1); > > > + > > > uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > > > requestTemplate->addEntry(ANDROID_CONTROL_AF_TRIGGER, > > > &afTrigger, 1); > > > @@ -875,6 +891,16 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() > > > requestTemplate->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE, > > > &aberrationMode, 1); > > > > > > + uint8_t controlMode = ANDROID_CONTROL_MODE_AUTO; > > > + requestTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1); > > > + > > > + float lensAperture = 2.53 / 100; > > > + requestTemplate->addEntry(ANDROID_LENS_APERTURE, &lensAperture, 1); > > > + > > > + uint8_t opticalStabilization = ANDROID_LENS_OPTICAL_STABILIZATION_MODE_OFF; > > > + requestTemplate->addEntry(ANDROID_LENS_OPTICAL_STABILIZATION_MODE, > > > + &opticalStabilization, 1); > > > + > > > uint8_t captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > > > requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT, > > > &captureIntent, 1);
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index b8cb118a960e..58901a119c18 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize() * Currently: 50 entries, 647 bytes of static metadata */ uint32_t numEntries = 50; - uint32_t byteSize = 647; + uint32_t byteSize = 667; /* * Calculate space occupation in bytes for dynamically built metadata @@ -779,6 +779,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() ANDROID_CONTROL_AE_MODE, ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, + ANDROID_CONTROL_AE_TARGET_FPS_RANGE, + ANDROID_CONTROL_AE_ANTIBANDING_MODE, ANDROID_CONTROL_AE_LOCK, ANDROID_CONTROL_AF_TRIGGER, ANDROID_CONTROL_AWB_MODE, @@ -787,6 +789,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() ANDROID_STATISTICS_FACE_DETECT_MODE, ANDROID_NOISE_REDUCTION_MODE, ANDROID_COLOR_CORRECTION_ABERRATION_MODE, + ANDROID_LENS_APERTURE, + ANDROID_LENS_OPTICAL_STABILIZATION_MODE, + ANDROID_CONTROL_MODE, ANDROID_CONTROL_CAPTURE_INTENT, }; staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, @@ -825,9 +830,9 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() { /* * \todo Keep this in sync with the actual number of entries. - * Currently: 12 entries, 15 bytes + * Currently: 20 entries, 35 bytes */ - CameraMetadata *requestTemplate = new CameraMetadata(15, 20); + CameraMetadata *requestTemplate = new CameraMetadata(20, 35); if (!requestTemplate->isValid()) return nullptr; @@ -847,6 +852,17 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() requestTemplate->addEntry(ANDROID_CONTROL_AE_LOCK, &aeLock, 1); + std::vector<int32_t> aeFpsTarget = { + 15, 30, + }; + requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, + aeFpsTarget.data(), + aeFpsTarget.size()); + + uint8_t aeAntibandingMode = ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO; + requestTemplate->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE, + &aeAntibandingMode, 1); + uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; requestTemplate->addEntry(ANDROID_CONTROL_AF_TRIGGER, &afTrigger, 1); @@ -875,6 +891,16 @@ const CameraMetadata *CameraDevice::captureTemplatePreview() requestTemplate->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE, &aberrationMode, 1); + uint8_t controlMode = ANDROID_CONTROL_MODE_AUTO; + requestTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1); + + float lensAperture = 2.53 / 100; + requestTemplate->addEntry(ANDROID_LENS_APERTURE, &lensAperture, 1); + + uint8_t opticalStabilization = ANDROID_LENS_OPTICAL_STABILIZATION_MODE_OFF; + requestTemplate->addEntry(ANDROID_LENS_OPTICAL_STABILIZATION_MODE, + &opticalStabilization, 1); + uint8_t captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT, &captureIntent, 1);
Add 5 controls to the generate preview template to comply with the camera3 specification. This change fixes CTS 9.0.r12 test [14/253] android.hardware.camera2.cts.CameraDeviceTest#testCameraDevicePreviewTemplate Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)