| Message ID | 20251103144917.439212-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thank you for the patch! Quoting Barnabás Pőcze (2025-11-03 14:49:17) > The enumerated controls/properties use `int32_t` as their backing type. > In multiple cases, when parsing such an enum value from a source, an > integer type is used. Replace the integer type with the proper enum > type where it is trivially doable. > > This change also brings `CameraSensorLegacy::initProperties()` in line > with `CameraSensorRaw::initProperties()`, by defaulting the color > filter arrangement to `MONO`. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/agc_mean_luminance.cpp | 2 +- > src/ipa/rpi/common/ipa_base.cpp | 4 +++- > src/libcamera/sensor/camera_sensor_legacy.cpp | 5 +++-- > src/libcamera/sensor/camera_sensor_raw.cpp | 5 ++--- > 4 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index 64f36bd75d..0c46329c09 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -297,7 +297,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData) > * possible before touching gain. > */ > if (availableExposureModes.empty()) { > - int32_t exposureModeId = controls::ExposureNormal; > + auto exposureModeId = controls::ExposureNormal; What is the type of 'auto' here? I'm not sure how I feel about using auto, because I'm not sure its more readable (especially in the case of the MONO below). Assuming its still a int32_t, looks good to me! Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > std::vector<std::pair<utils::Duration, double>> stages = { }; > > std::shared_ptr<ExposureModeHelper> helper = > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 8dfe35cc32..865035e578 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -1584,7 +1584,9 @@ void IpaBase::reportMetadata(unsigned int ipaContext) > > const AfStatus *afStatus = rpiMetadata.getLocked<AfStatus>("af.status"); > if (afStatus) { > - int32_t s, p; > + controls::AfStateEnum s; > + controls::AfPauseStateEnum p; > + > switch (afStatus->state) { > case AfState::Scanning: > s = controls::AfStateScanning; > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > index f9e685a9ac..8b6df10ee6 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -571,7 +571,7 @@ int CameraSensorLegacy::initProperties() > const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > if (orientation != controls.end()) { > int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > - int32_t propertyValue; > + properties::LocationEnum propertyValue; > > switch (v4l2Orientation) { > default: > @@ -624,7 +624,8 @@ int CameraSensorLegacy::initProperties() > > /* Color filter array pattern, register only for RAW sensors. */ > if (bayerFormat_) { > - int32_t cfa; > + auto cfa = properties::draft::MONO; > + > switch (bayerFormat_->order) { > case BayerFormat::BGGR: > cfa = properties::draft::BGGR; > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > index 8ea4423698..cabaa21635 100644 > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > @@ -576,7 +576,7 @@ int CameraSensorRaw::initProperties() > const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > if (orientation != controls.end()) { > int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > - int32_t propertyValue; > + properties::LocationEnum propertyValue; > > switch (v4l2Orientation) { > default: > @@ -628,7 +628,7 @@ int CameraSensorRaw::initProperties() > properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); > > /* Color filter array pattern. */ > - uint32_t cfa; > + auto cfa = properties::draft::MONO; > > switch (cfaPattern_) { > case BayerFormat::BGGR: > @@ -644,7 +644,6 @@ int CameraSensorRaw::initProperties() > cfa = properties::draft::RGGB; > break; > case BayerFormat::MONO: > - default: > cfa = properties::draft::MONO; > break; > } > -- > 2.51.2 >
Hi 2025. 11. 04. 16:06 keltezéssel, Isaac Scott írta: > Hi Barnabás, > > Thank you for the patch! > > Quoting Barnabás Pőcze (2025-11-03 14:49:17) >> The enumerated controls/properties use `int32_t` as their backing type. >> In multiple cases, when parsing such an enum value from a source, an >> integer type is used. Replace the integer type with the proper enum >> type where it is trivially doable. >> >> This change also brings `CameraSensorLegacy::initProperties()` in line >> with `CameraSensorRaw::initProperties()`, by defaulting the color >> filter arrangement to `MONO`. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/libipa/agc_mean_luminance.cpp | 2 +- >> src/ipa/rpi/common/ipa_base.cpp | 4 +++- >> src/libcamera/sensor/camera_sensor_legacy.cpp | 5 +++-- >> src/libcamera/sensor/camera_sensor_raw.cpp | 5 ++--- >> 4 files changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp >> index 64f36bd75d..0c46329c09 100644 >> --- a/src/ipa/libipa/agc_mean_luminance.cpp >> +++ b/src/ipa/libipa/agc_mean_luminance.cpp >> @@ -297,7 +297,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData) >> * possible before touching gain. >> */ >> if (availableExposureModes.empty()) { >> - int32_t exposureModeId = controls::ExposureNormal; >> + auto exposureModeId = controls::ExposureNormal; > > What is the type of 'auto' here? I'm not sure how I feel about using > auto, because I'm not sure its more readable (especially in the case of > the MONO below). The type would be `controls::AeExposureModeEnum` here and `controls::draft::ColorFilterArrangementEnum` for "MONO"; but I thought they would be a bit long. > > Assuming its still a int32_t, looks good to me! > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > >> std::vector<std::pair<utils::Duration, double>> stages = { }; >> >> std::shared_ptr<ExposureModeHelper> helper = >> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp >> index 8dfe35cc32..865035e578 100644 >> --- a/src/ipa/rpi/common/ipa_base.cpp >> +++ b/src/ipa/rpi/common/ipa_base.cpp >> @@ -1584,7 +1584,9 @@ void IpaBase::reportMetadata(unsigned int ipaContext) >> >> const AfStatus *afStatus = rpiMetadata.getLocked<AfStatus>("af.status"); >> if (afStatus) { >> - int32_t s, p; >> + controls::AfStateEnum s; >> + controls::AfPauseStateEnum p; >> + >> switch (afStatus->state) { >> case AfState::Scanning: >> s = controls::AfStateScanning; >> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp >> index f9e685a9ac..8b6df10ee6 100644 >> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp >> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp >> @@ -571,7 +571,7 @@ int CameraSensorLegacy::initProperties() >> const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); >> if (orientation != controls.end()) { >> int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); >> - int32_t propertyValue; >> + properties::LocationEnum propertyValue; >> >> switch (v4l2Orientation) { >> default: >> @@ -624,7 +624,8 @@ int CameraSensorLegacy::initProperties() >> >> /* Color filter array pattern, register only for RAW sensors. */ >> if (bayerFormat_) { >> - int32_t cfa; >> + auto cfa = properties::draft::MONO; >> + >> switch (bayerFormat_->order) { >> case BayerFormat::BGGR: >> cfa = properties::draft::BGGR; >> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp >> index 8ea4423698..cabaa21635 100644 >> --- a/src/libcamera/sensor/camera_sensor_raw.cpp >> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp >> @@ -576,7 +576,7 @@ int CameraSensorRaw::initProperties() >> const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); >> if (orientation != controls.end()) { >> int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); >> - int32_t propertyValue; >> + properties::LocationEnum propertyValue; >> >> switch (v4l2Orientation) { >> default: >> @@ -628,7 +628,7 @@ int CameraSensorRaw::initProperties() >> properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); >> >> /* Color filter array pattern. */ >> - uint32_t cfa; >> + auto cfa = properties::draft::MONO; >> >> switch (cfaPattern_) { >> case BayerFormat::BGGR: >> @@ -644,7 +644,6 @@ int CameraSensorRaw::initProperties() >> cfa = properties::draft::RGGB; >> break; >> case BayerFormat::MONO: >> - default: >> cfa = properties::draft::MONO; >> break; >> } >> -- >> 2.51.2 >>
Quoting Barnabás Pőcze (2025-11-03 14:49:17) > The enumerated controls/properties use `int32_t` as their backing type. > In multiple cases, when parsing such an enum value from a source, an > integer type is used. Replace the integer type with the proper enum > type where it is trivially doable. > > This change also brings `CameraSensorLegacy::initProperties()` in line > with `CameraSensorRaw::initProperties()`, by defaulting the color > filter arrangement to `MONO`. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/agc_mean_luminance.cpp | 2 +- > src/ipa/rpi/common/ipa_base.cpp | 4 +++- > src/libcamera/sensor/camera_sensor_legacy.cpp | 5 +++-- > src/libcamera/sensor/camera_sensor_raw.cpp | 5 ++--- > 4 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index 64f36bd75d..0c46329c09 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -297,7 +297,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData) > * possible before touching gain. > */ > if (availableExposureModes.empty()) { > - int32_t exposureModeId = controls::ExposureNormal; > + auto exposureModeId = controls::ExposureNormal; > std::vector<std::pair<utils::Duration, double>> stages = { }; > > std::shared_ptr<ExposureModeHelper> helper = > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 8dfe35cc32..865035e578 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -1584,7 +1584,9 @@ void IpaBase::reportMetadata(unsigned int ipaContext) > > const AfStatus *afStatus = rpiMetadata.getLocked<AfStatus>("af.status"); > if (afStatus) { > - int32_t s, p; > + controls::AfStateEnum s; > + controls::AfPauseStateEnum p; > + > switch (afStatus->state) { > case AfState::Scanning: > s = controls::AfStateScanning; > diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > index f9e685a9ac..8b6df10ee6 100644 > --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > @@ -571,7 +571,7 @@ int CameraSensorLegacy::initProperties() > const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > if (orientation != controls.end()) { > int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > - int32_t propertyValue; > + properties::LocationEnum propertyValue; > > switch (v4l2Orientation) { > default: > @@ -624,7 +624,8 @@ int CameraSensorLegacy::initProperties() > > /* Color filter array pattern, register only for RAW sensors. */ > if (bayerFormat_) { > - int32_t cfa; > + auto cfa = properties::draft::MONO; > + > switch (bayerFormat_->order) { > case BayerFormat::BGGR: > cfa = properties::draft::BGGR; > diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > index 8ea4423698..cabaa21635 100644 > --- a/src/libcamera/sensor/camera_sensor_raw.cpp > +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > @@ -576,7 +576,7 @@ int CameraSensorRaw::initProperties() > const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > if (orientation != controls.end()) { > int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > - int32_t propertyValue; > + properties::LocationEnum propertyValue; > > switch (v4l2Orientation) { > default: > @@ -628,7 +628,7 @@ int CameraSensorRaw::initProperties() > properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); > > /* Color filter array pattern. */ > - uint32_t cfa; > + auto cfa = properties::draft::MONO; I'm fine with this patch so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> But (I expect this is yak-shaving) - can we take this opportunity to get rid of the only ::draft:: property ? 5d3d0dcedb36f991b7f395c15b9112694f44d87d introduced this back in 2020! libcamera: properties: ColorFilterArrangement draft property Define the 'ColorFilterArrangement' draft property. The property is currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> If we drop the text "Currently identical to ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT." I think we can move this to a real property. This should never have been a draft :-) I think we can be fairly sure that these are accepted CFA arrangements. Maybe we'll add some more exotic arrangements later - but I don't think there's anything /wrong/ with these as they are. -- Kieran > > switch (cfaPattern_) { > case BayerFormat::BGGR: > @@ -644,7 +644,6 @@ int CameraSensorRaw::initProperties() > cfa = properties::draft::RGGB; > break; > case BayerFormat::MONO: > - default: > cfa = properties::draft::MONO; > break; > } > -- > 2.51.2 >
On Thu, Nov 06, 2025 at 02:43:20PM +0100, Barnabás Pőcze wrote: > 2025. 11. 04. 16:06 keltezéssel, Isaac Scott írta: > > Quoting Barnabás Pőcze (2025-11-03 14:49:17) > >> The enumerated controls/properties use `int32_t` as their backing type. > >> In multiple cases, when parsing such an enum value from a source, an > >> integer type is used. Replace the integer type with the proper enum > >> type where it is trivially doable. > >> > >> This change also brings `CameraSensorLegacy::initProperties()` in line > >> with `CameraSensorRaw::initProperties()`, by defaulting the color > >> filter arrangement to `MONO`. > >> > >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> --- > >> src/ipa/libipa/agc_mean_luminance.cpp | 2 +- > >> src/ipa/rpi/common/ipa_base.cpp | 4 +++- > >> src/libcamera/sensor/camera_sensor_legacy.cpp | 5 +++-- > >> src/libcamera/sensor/camera_sensor_raw.cpp | 5 ++--- > >> 4 files changed, 9 insertions(+), 7 deletions(-) > >> > >> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > >> index 64f36bd75d..0c46329c09 100644 > >> --- a/src/ipa/libipa/agc_mean_luminance.cpp > >> +++ b/src/ipa/libipa/agc_mean_luminance.cpp > >> @@ -297,7 +297,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData) > >> * possible before touching gain. > >> */ > >> if (availableExposureModes.empty()) { > >> - int32_t exposureModeId = controls::ExposureNormal; > >> + auto exposureModeId = controls::ExposureNormal; > > > > What is the type of 'auto' here? I'm not sure how I feel about using > > auto, because I'm not sure its more readable (especially in the case of > > the MONO below). > > The type would be `controls::AeExposureModeEnum` here and `controls::draft::ColorFilterArrangementEnum` > for "MONO"; but I thought they would be a bit long. controls::AeExposureModeEnum exposureModeId = controls::ExposureNormal; It's a bit long, but I agree with Isaac, I think auto hinders readability here. > > Assuming its still a int32_t, looks good to me! > > > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > > > >> std::vector<std::pair<utils::Duration, double>> stages = { }; > >> > >> std::shared_ptr<ExposureModeHelper> helper = > >> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > >> index 8dfe35cc32..865035e578 100644 > >> --- a/src/ipa/rpi/common/ipa_base.cpp > >> +++ b/src/ipa/rpi/common/ipa_base.cpp > >> @@ -1584,7 +1584,9 @@ void IpaBase::reportMetadata(unsigned int ipaContext) > >> > >> const AfStatus *afStatus = rpiMetadata.getLocked<AfStatus>("af.status"); > >> if (afStatus) { > >> - int32_t s, p; > >> + controls::AfStateEnum s; > >> + controls::AfPauseStateEnum p; > >> + > >> switch (afStatus->state) { > >> case AfState::Scanning: > >> s = controls::AfStateScanning; > >> diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp > >> index f9e685a9ac..8b6df10ee6 100644 > >> --- a/src/libcamera/sensor/camera_sensor_legacy.cpp > >> +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp > >> @@ -571,7 +571,7 @@ int CameraSensorLegacy::initProperties() > >> const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > >> if (orientation != controls.end()) { > >> int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > >> - int32_t propertyValue; > >> + properties::LocationEnum propertyValue; > >> > >> switch (v4l2Orientation) { > >> default: > >> @@ -624,7 +624,8 @@ int CameraSensorLegacy::initProperties() > >> > >> /* Color filter array pattern, register only for RAW sensors. */ > >> if (bayerFormat_) { > >> - int32_t cfa; > >> + auto cfa = properties::draft::MONO; controls::draft::ColorFilterArrangementEnum cfa = properties::draft::MONO; Even a bit longer, but if we drop draft as requested by Kieran... :-) This being said, once we rename the enumerators (or switch to enum class) as discussed separately, this would become (without draft) controls::ColorFilterArrangementEnum cfa = properties::ColorFilterArrangementEnum::MONO; or controls::ColorFilterArrangementEnum cfa = properties::ColorFilterArrangementEnum::MONO; compared to auto cfa = properties::ColorFilterArrangementEnum::MONO; I think I would have less problems with using auto in his case as the type is conveyed by the right hand side operand (even if I still prefer explicit types). > >> + > >> switch (bayerFormat_->order) { > >> case BayerFormat::BGGR: > >> cfa = properties::draft::BGGR; > >> diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp > >> index 8ea4423698..cabaa21635 100644 > >> --- a/src/libcamera/sensor/camera_sensor_raw.cpp > >> +++ b/src/libcamera/sensor/camera_sensor_raw.cpp > >> @@ -576,7 +576,7 @@ int CameraSensorRaw::initProperties() > >> const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); > >> if (orientation != controls.end()) { > >> int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); > >> - int32_t propertyValue; > >> + properties::LocationEnum propertyValue; > >> > >> switch (v4l2Orientation) { > >> default: > >> @@ -628,7 +628,7 @@ int CameraSensorRaw::initProperties() > >> properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); > >> > >> /* Color filter array pattern. */ > >> - uint32_t cfa; > >> + auto cfa = properties::draft::MONO; > >> > >> switch (cfaPattern_) { > >> case BayerFormat::BGGR: > >> @@ -644,7 +644,6 @@ int CameraSensorRaw::initProperties() > >> cfa = properties::draft::RGGB; > >> break; > >> case BayerFormat::MONO: > >> - default: > >> cfa = properties::draft::MONO; > >> break; > >> }
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index 64f36bd75d..0c46329c09 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -297,7 +297,7 @@ int AgcMeanLuminance::parseExposureModes(const YamlObject &tuningData) * possible before touching gain. */ if (availableExposureModes.empty()) { - int32_t exposureModeId = controls::ExposureNormal; + auto exposureModeId = controls::ExposureNormal; std::vector<std::pair<utils::Duration, double>> stages = { }; std::shared_ptr<ExposureModeHelper> helper = diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 8dfe35cc32..865035e578 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -1584,7 +1584,9 @@ void IpaBase::reportMetadata(unsigned int ipaContext) const AfStatus *afStatus = rpiMetadata.getLocked<AfStatus>("af.status"); if (afStatus) { - int32_t s, p; + controls::AfStateEnum s; + controls::AfPauseStateEnum p; + switch (afStatus->state) { case AfState::Scanning: s = controls::AfStateScanning; diff --git a/src/libcamera/sensor/camera_sensor_legacy.cpp b/src/libcamera/sensor/camera_sensor_legacy.cpp index f9e685a9ac..8b6df10ee6 100644 --- a/src/libcamera/sensor/camera_sensor_legacy.cpp +++ b/src/libcamera/sensor/camera_sensor_legacy.cpp @@ -571,7 +571,7 @@ int CameraSensorLegacy::initProperties() const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); if (orientation != controls.end()) { int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); - int32_t propertyValue; + properties::LocationEnum propertyValue; switch (v4l2Orientation) { default: @@ -624,7 +624,8 @@ int CameraSensorLegacy::initProperties() /* Color filter array pattern, register only for RAW sensors. */ if (bayerFormat_) { - int32_t cfa; + auto cfa = properties::draft::MONO; + switch (bayerFormat_->order) { case BayerFormat::BGGR: cfa = properties::draft::BGGR; diff --git a/src/libcamera/sensor/camera_sensor_raw.cpp b/src/libcamera/sensor/camera_sensor_raw.cpp index 8ea4423698..cabaa21635 100644 --- a/src/libcamera/sensor/camera_sensor_raw.cpp +++ b/src/libcamera/sensor/camera_sensor_raw.cpp @@ -576,7 +576,7 @@ int CameraSensorRaw::initProperties() const auto &orientation = controls.find(V4L2_CID_CAMERA_ORIENTATION); if (orientation != controls.end()) { int32_t v4l2Orientation = orientation->second.def().get<int32_t>(); - int32_t propertyValue; + properties::LocationEnum propertyValue; switch (v4l2Orientation) { default: @@ -628,7 +628,7 @@ int CameraSensorRaw::initProperties() properties_.set(properties::PixelArrayActiveAreas, { activeArea_ }); /* Color filter array pattern. */ - uint32_t cfa; + auto cfa = properties::draft::MONO; switch (cfaPattern_) { case BayerFormat::BGGR: @@ -644,7 +644,6 @@ int CameraSensorRaw::initProperties() cfa = properties::draft::RGGB; break; case BayerFormat::MONO: - default: cfa = properties::draft::MONO; break; }
The enumerated controls/properties use `int32_t` as their backing type. In multiple cases, when parsing such an enum value from a source, an integer type is used. Replace the integer type with the proper enum type where it is trivially doable. This change also brings `CameraSensorLegacy::initProperties()` in line with `CameraSensorRaw::initProperties()`, by defaulting the color filter arrangement to `MONO`. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/agc_mean_luminance.cpp | 2 +- src/ipa/rpi/common/ipa_base.cpp | 4 +++- src/libcamera/sensor/camera_sensor_legacy.cpp | 5 +++-- src/libcamera/sensor/camera_sensor_raw.cpp | 5 ++--- 4 files changed, 9 insertions(+), 7 deletions(-)