Hi Umang, Thanks for the review.
On Thu, Oct 28, 2021 at 9:34 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hi Han-Lin,
>
> Thank you for the patch.
>
> On 10/28/21 3:33 PM, Han-Lin Chen wrote:
> > The frame use mode is set according to Android Capture Intent in Intel
> > HAL's implememtation. The current default mode ia_aiq_frame_use_still is
> > only used with the single capture request. For preview use case, it has
> > hard time converging AE and AF smoothly. Change the default mode to
> > ia_aiq_frame_use_preview for better user experience.
>
>
> I believe the discussions and understanding I have around frame_use, is
> that, it is something to be set to be from the applications's (or
> Request's) side and passed on to the IPA. Is this the way frame_use is
> set the existing HAL implementation on chrome side? I guess I'll need to
> check for confirmation.
It should reflect the ANDROID_CONTROL_CAPTURE_INTENT enum.
In theory, we will need to extend the IPA interface to get the
information from the pipeline handler.
In real use cases, The application should set it to
ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW for continuous preview frames,
and
only set it to ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE when the
user takes a photo, and resume to preview quickly.
I got to know the following by some experiments with Intel HAL. No
documentation is found -.-.
AF would be locked once the AIQ finds a focus and never rescan with
ia_aiq_frame_use_still mode.
AE would have a strong haunting before converging.
>
> Anyways, the patch looks fine to me so,
>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.com>
> > ---
> > aiq/aiq_input_parameters.cpp | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp
> > index 36e2b07..bc87b31 100644
> > --- a/aiq/aiq_input_parameters.cpp
> > +++ b/aiq/aiq_input_parameters.cpp
> > @@ -132,7 +132,7 @@ void AiqInputParameters::setAeAwbAfDefaults()
> > {
> > /*Ae Params */
> > aeInputParams.num_exposures = NUM_EXPOSURES;
> > - aeInputParams.frame_use = ia_aiq_frame_use_still;
> > + aeInputParams.frame_use = ia_aiq_frame_use_preview;
> > aeInputParams.flash_mode = ia_aiq_flash_mode_off;
> > aeInputParams.operation_mode = ia_aiq_ae_operation_mode_automatic;
> > aeInputParams.metering_mode = ia_aiq_ae_metering_mode_evaluative;
> > @@ -153,7 +153,7 @@ void AiqInputParameters::setAeAwbAfDefaults()
> > aeInputParams.manual_convergence_time = -1;
> >
> > /* AWB Params */
> > - awbParams.frame_use = ia_aiq_frame_use_still;
> > + awbParams.frame_use = ia_aiq_frame_use_preview;
> > awbParams.scene_mode = ia_aiq_awb_operation_mode_auto;
> > awbParams.manual_convergence_time = -1.0;
> > awbParams.manual_cct_range = nullptr;
> > @@ -161,7 +161,7 @@ void AiqInputParameters::setAeAwbAfDefaults()
> >
> > /* AF Params */
> > afParams = {
> > - ia_aiq_frame_use_still, 0, 1500,
> > + ia_aiq_frame_use_preview, 0, 1500,
> > ia_aiq_af_operation_mode_auto,
> > ia_aiq_af_range_normal,
> > ia_aiq_af_metering_mode_auto,
> > @@ -172,11 +172,11 @@ void AiqInputParameters::setAeAwbAfDefaults()
> > /* GBCE Params */
> > gbceParams.gbce_level = ia_aiq_gbce_level_bypass;
> > gbceParams.tone_map_level = ia_aiq_tone_map_level_default;
> > - gbceParams.frame_use = ia_aiq_frame_use_still;
> > + gbceParams.frame_use = ia_aiq_frame_use_preview;
> > gbceParams.ev_shift = 0;
> >
> > /* SA Params */
> > - saParams.frame_use = ia_aiq_frame_use_still;
> > + saParams.frame_use = ia_aiq_frame_use_preview;
> > }
> >
> > } /* namespace ipa::ipu3::aiq */