| Message ID | 20230612130608.11651-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 618b9aaa17713bed359112bda358e81e1eaea86c |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for the patch! On Mon, 12 Jun 2023 at 14:30, Naushir Patuck <naush@raspberrypi.com> wrote: > > In commit 0ee9339331c6, a default metering/exposure/constraint mode is > used if a control sets a mode that is not listed in the camera tuning > file. > > Setting a default mode may be undesirable in these cases, so instead > keep the agc mode unchanged. This also matches the behaviour for other > IPA controls where no changes are made in error conditions. > > Fixes: 0ee9339331c6 ("ipa: rpi: agc: Gracefully handle missing agc modes") > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> It's only a marginal thing but yes, I think I prefer this version! Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > --- > src/ipa/rpi/controller/rpi/agc.cpp | 27 ++++++++++----------------- > 1 file changed, 10 insertions(+), 17 deletions(-) > > diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp > index e60ca33f867b..ae9ff219fa03 100644 > --- a/src/ipa/rpi/controller/rpi/agc.cpp > +++ b/src/ipa/rpi/controller/rpi/agc.cpp > @@ -541,39 +541,32 @@ void Agc::housekeepConfig() > if (meteringModeName_ != status_.meteringMode) { > auto it = config_.meteringModes.find(meteringModeName_); > if (it == config_.meteringModes.end()) { > - LOG(RPiAgc, Warning) << "No metering mode " << meteringModeName_ > - << ", defaulting to " << config_.defaultMeteringMode; > - meteringModeName_ = config_.defaultMeteringMode; > - meteringMode_ = &config_.meteringModes[meteringModeName_]; > + LOG(RPiAgc, Warning) << "No metering mode " << meteringModeName_; > + meteringModeName_ = status_.meteringMode; > } else { > meteringMode_ = &it->second; > + status_.meteringMode = meteringModeName_; > } > - status_.meteringMode = meteringModeName_; > } > if (exposureModeName_ != status_.exposureMode) { > auto it = config_.exposureModes.find(exposureModeName_); > if (it == config_.exposureModes.end()) { > - LOG(RPiAgc, Warning) << "No exposure profile " << exposureModeName_ > - << ", defaulting to " << config_.defaultExposureMode; > - exposureModeName_ = config_.defaultExposureMode; > - exposureMode_ = &config_.exposureModes[exposureModeName_]; > + LOG(RPiAgc, Warning) << "No exposure profile " << exposureModeName_; > + exposureModeName_ = status_.exposureMode; > } else { > exposureMode_ = &it->second; > + status_.exposureMode = exposureModeName_; > } > - status_.exposureMode = exposureModeName_; > } > if (constraintModeName_ != status_.constraintMode) { > - auto it = > - config_.constraintModes.find(constraintModeName_); > + auto it = config_.constraintModes.find(constraintModeName_); > if (it == config_.constraintModes.end()) { > - LOG(RPiAgc, Warning) << "No constraint list " << constraintModeName_ > - << ", defaulting to " << config_.defaultConstraintMode; > - constraintModeName_ = config_.defaultConstraintMode; > - constraintMode_ = &config_.constraintModes[constraintModeName_]; > + LOG(RPiAgc, Warning) << "No constraint list " << constraintModeName_; > + constraintModeName_ = status_.constraintMode; > } else { > constraintMode_ = &it->second; > + status_.constraintMode = constraintModeName_; > } > - status_.constraintMode = constraintModeName_; > } > LOG(RPiAgc, Debug) << "exposureMode " > << exposureModeName_ << " constraintMode " > -- > 2.34.1 >
diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp index e60ca33f867b..ae9ff219fa03 100644 --- a/src/ipa/rpi/controller/rpi/agc.cpp +++ b/src/ipa/rpi/controller/rpi/agc.cpp @@ -541,39 +541,32 @@ void Agc::housekeepConfig() if (meteringModeName_ != status_.meteringMode) { auto it = config_.meteringModes.find(meteringModeName_); if (it == config_.meteringModes.end()) { - LOG(RPiAgc, Warning) << "No metering mode " << meteringModeName_ - << ", defaulting to " << config_.defaultMeteringMode; - meteringModeName_ = config_.defaultMeteringMode; - meteringMode_ = &config_.meteringModes[meteringModeName_]; + LOG(RPiAgc, Warning) << "No metering mode " << meteringModeName_; + meteringModeName_ = status_.meteringMode; } else { meteringMode_ = &it->second; + status_.meteringMode = meteringModeName_; } - status_.meteringMode = meteringModeName_; } if (exposureModeName_ != status_.exposureMode) { auto it = config_.exposureModes.find(exposureModeName_); if (it == config_.exposureModes.end()) { - LOG(RPiAgc, Warning) << "No exposure profile " << exposureModeName_ - << ", defaulting to " << config_.defaultExposureMode; - exposureModeName_ = config_.defaultExposureMode; - exposureMode_ = &config_.exposureModes[exposureModeName_]; + LOG(RPiAgc, Warning) << "No exposure profile " << exposureModeName_; + exposureModeName_ = status_.exposureMode; } else { exposureMode_ = &it->second; + status_.exposureMode = exposureModeName_; } - status_.exposureMode = exposureModeName_; } if (constraintModeName_ != status_.constraintMode) { - auto it = - config_.constraintModes.find(constraintModeName_); + auto it = config_.constraintModes.find(constraintModeName_); if (it == config_.constraintModes.end()) { - LOG(RPiAgc, Warning) << "No constraint list " << constraintModeName_ - << ", defaulting to " << config_.defaultConstraintMode; - constraintModeName_ = config_.defaultConstraintMode; - constraintMode_ = &config_.constraintModes[constraintModeName_]; + LOG(RPiAgc, Warning) << "No constraint list " << constraintModeName_; + constraintModeName_ = status_.constraintMode; } else { constraintMode_ = &it->second; + status_.constraintMode = constraintModeName_; } - status_.constraintMode = constraintModeName_; } LOG(RPiAgc, Debug) << "exposureMode " << exposureModeName_ << " constraintMode "
In commit 0ee9339331c6, a default metering/exposure/constraint mode is used if a control sets a mode that is not listed in the camera tuning file. Setting a default mode may be undesirable in these cases, so instead keep the agc mode unchanged. This also matches the behaviour for other IPA controls where no changes are made in error conditions. Fixes: 0ee9339331c6 ("ipa: rpi: agc: Gracefully handle missing agc modes") Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/controller/rpi/agc.cpp | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)