| Message ID | 20211028100349.1098545-1-hanlinchen@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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. Anyways, the patch looks fine to me so, Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > Signed-off-by: Han-Lin Chen <hanlinchen@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 */
Hi Umang, Thanks for the review. On Thu, Oct 28, 2021 at 9:34 PM Umang Jain <umang.jain@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@ideasonboard.com> > > > > Signed-off-by: Han-Lin Chen <hanlinchen@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 */
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 */
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. Signed-off-by: Han-Lin Chen <hanlinchen@chromium.com> --- aiq/aiq_input_parameters.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)