| Message ID | 20261019184526.252977-7-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Mon, Oct 19, 2026 at 08:45:13PM +0200, Barnabás Pőcze wrote: > Currently `splitExposure()` will raise the exposure and analogue gain above > what is specified in the last stage, if the desired exposure could not be > reached. This behaviour is different from that of the raspberry pi ipa, > and also could be surprising to the user. > > So make sure that the exposure time and analogue gain is limited by the > last stage, in addition to the runtime controlled upper bound. > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Now I see why you want to handle the no-stages case separately. Inverting the order of the two patches might help making it more clear. Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/ipa/libipa/exposure_mode_helper.cpp | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp > index 7a810a3673..cadd1903d8 100644 > --- a/src/ipa/libipa/exposure_mode_helper.cpp > +++ b/src/ipa/libipa/exposure_mode_helper.cpp > @@ -203,8 +203,9 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons > * value. > * > * If no combination of exposure time and gain limits is found that meets the > - * required exposure, the helper falls-back to simply maximising the exposure > - * time first, followed by analogue gain, followed by digital gain. > + * required exposure, then the exposure and gain of the last stage will be used, > + * unless there are no stages, in which case first the exposure, then the > + * analogue gain, then the digital gain will be maximized as necessary. > * > * During the calculations the gain missed due to quantization is recorded and > * returned as quantization gain. The quantization gain is not included in the > @@ -295,12 +296,11 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const > } > > /* > - * From here on all we can do is max out the exposure time, followed by > - * the analogue gain. If we still haven't achieved the target we send > - * the rest of the exposure time to digital gain. > + * If no stages could satisfy the desired exposure, then use the last > + * stage's exposure time and gain. > */ > - exposureTime = clampExposureTime(exposure / lastStageGain, &quantGain); > - gain = clampGain(exposure / exposureTime, &quantGain2); > + exposureTime = clampExposureTime(stages_.back().exposureTime, &quantGain); > + gain = clampGain(stages_.back().gain, &quantGain2); > quantGain *= quantGain2; > > return { exposureTime, gain, quantGain, > -- > 2.56.0 >
Hi 2026. 10. 20. 17:19 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Mon, Oct 19, 2026 at 08:45:13PM +0200, Barnabás Pőcze wrote: >> Currently `splitExposure()` will raise the exposure and analogue gain above >> what is specified in the last stage, if the desired exposure could not be >> reached. This behaviour is different from that of the raspberry pi ipa, >> and also could be surprising to the user. >> >> So make sure that the exposure time and analogue gain is limited by the >> last stage, in addition to the runtime controlled upper bound. >> >> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > Now I see why you want to handle the no-stages case separately. > > Inverting the order of the two patches might help making it more > clear. Yes, but then the behaviour would change, which I wanted to avoid. But I have removed the previous patch, so this is not relevant anymore. > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > >> --- >> src/ipa/libipa/exposure_mode_helper.cpp | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp >> index 7a810a3673..cadd1903d8 100644 >> --- a/src/ipa/libipa/exposure_mode_helper.cpp >> +++ b/src/ipa/libipa/exposure_mode_helper.cpp >> @@ -203,8 +203,9 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons >> * value. >> * >> * If no combination of exposure time and gain limits is found that meets the >> - * required exposure, the helper falls-back to simply maximising the exposure >> - * time first, followed by analogue gain, followed by digital gain. >> + * required exposure, then the exposure and gain of the last stage will be used, >> + * unless there are no stages, in which case first the exposure, then the >> + * analogue gain, then the digital gain will be maximized as necessary. >> * >> * During the calculations the gain missed due to quantization is recorded and >> * returned as quantization gain. The quantization gain is not included in the >> @@ -295,12 +296,11 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const >> } >> >> /* >> - * From here on all we can do is max out the exposure time, followed by >> - * the analogue gain. If we still haven't achieved the target we send >> - * the rest of the exposure time to digital gain. >> + * If no stages could satisfy the desired exposure, then use the last >> + * stage's exposure time and gain. >> */ >> - exposureTime = clampExposureTime(exposure / lastStageGain, &quantGain); >> - gain = clampGain(exposure / exposureTime, &quantGain2); >> + exposureTime = clampExposureTime(stages_.back().exposureTime, &quantGain); >> + gain = clampGain(stages_.back().gain, &quantGain2); >> quantGain *= quantGain2; >> >> return { exposureTime, gain, quantGain, >> -- >> 2.56.0 >>
diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp index 7a810a3673..cadd1903d8 100644 --- a/src/ipa/libipa/exposure_mode_helper.cpp +++ b/src/ipa/libipa/exposure_mode_helper.cpp @@ -203,8 +203,9 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons * value. * * If no combination of exposure time and gain limits is found that meets the - * required exposure, the helper falls-back to simply maximising the exposure - * time first, followed by analogue gain, followed by digital gain. + * required exposure, then the exposure and gain of the last stage will be used, + * unless there are no stages, in which case first the exposure, then the + * analogue gain, then the digital gain will be maximized as necessary. * * During the calculations the gain missed due to quantization is recorded and * returned as quantization gain. The quantization gain is not included in the @@ -295,12 +296,11 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const } /* - * From here on all we can do is max out the exposure time, followed by - * the analogue gain. If we still haven't achieved the target we send - * the rest of the exposure time to digital gain. + * If no stages could satisfy the desired exposure, then use the last + * stage's exposure time and gain. */ - exposureTime = clampExposureTime(exposure / lastStageGain, &quantGain); - gain = clampGain(exposure / exposureTime, &quantGain2); + exposureTime = clampExposureTime(stages_.back().exposureTime, &quantGain); + gain = clampGain(stages_.back().gain, &quantGain2); quantGain *= quantGain2; return { exposureTime, gain, quantGain,
Currently `splitExposure()` will raise the exposure and analogue gain above what is specified in the last stage, if the desired exposure could not be reached. This behaviour is different from that of the raspberry pi ipa, and also could be surprising to the user. So make sure that the exposure time and analogue gain is limited by the last stage, in addition to the runtime controlled upper bound. Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/exposure_mode_helper.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)