[libcamera-devel] [PATCH v2] android: camera_device: Set AE precapture trigger according to request

[Laurent Pinchart]

Hi Jacopo and Paul,

On Thu, Jan 28, 2021 at 12:03:03PM +0100, Jacopo Mondi wrote:
> On Thu, Jan 28, 2021 at 07:21:40PM +0900, Paul Elder wrote:
> > Set the AE precapture triggler tag in the android result metadata
> > according to what was passed in the request metadata.
> >
> > This allows the following CTS test to pass:
> > - android.hardware.camera2.cts.StillCaptureTest#testAePrecaptureTriggerCancelJpegCapture
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> >
> > ---
> > Changes in v2:
> > - move setting the tag to getResultMetadata()
> > ---
> >  src/android/camera_device.cpp | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 82bf0d3a..f5066709 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1965,8 +1965,11 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
> >  				 aeFpsTarget.data(), aeFpsTarget.size());
> >
> >  	value = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;
> > -	resultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> > -				 &value, 1);
> > +	camera_metadata_ro_entry_t entry;
> 
> You can move this one at the beginning of the function. It is used in
> many places, and we'll continue moving it otherwise.
> 
> > +	/* \todo Handle IPA appropriately */
> > +	bool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);
> > +	resultMetadata->updateEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> > +				    ret ? entry.data.u8 : &value, 1);
> 
> idk... we're blindly taking this from from the settings, while I think
> it would be worth plumbing it into the pipeline handler already,
> even if it will be functionally equivalent to what you have here.
> 
> There's also a draft control already defined for this.
> 
> To close the loop this would require to:
> - register the control in the pipeline handler (for IPU3 at initControls() time)
> - translate the android metadata into a libcamera control in
>   CameraDevice::processControls()
> - Return in the request complete handler of the pipeline handler the
>   value passed in the request settings (see how ScalerCrop is handled
>   in the IPU3 imguOutputBufferReady() slot)
> - Translate back the libcamera control from the request to the android
>   metadata (look again at how ScalerCrop is handled in
>   getResultMetadata())
> 
> This might seems like a useless exercize, but once the IPA needs to
> handle this control for real, this will have to be done and I would
> not delay it.
> 
> Laurent, do you think this is worth going to the pipeline handler loop
> already ?

There's a state machine to we need to implement to handle the AE. It
involves multiple controls. As we don't support still image capture yet
in the libcamera core I think it could be too early to plumb this
through, I'm be tempted to just go with this shortcut for the very short
term.

> >  	value = ANDROID_CONTROL_AE_STATE_CONVERGED;
> >  	resultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);
> > @@ -2010,8 +2013,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
> >  	value = ANDROID_FLASH_STATE_UNAVAILABLE;
> >  	resultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);
> >
> > -	camera_metadata_ro_entry_t entry;
> > -	int ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
> > +	ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);
> >  	if (ret)
> >  		resultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);
> >

-- 
Regards,

Laurent Pinchart