Message ID | 20201021143635.22846-8-jacopo@jmondi.org |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, Thank you for the patch. CC'ing Naush and David. On Wed, Oct 21, 2020 at 04:36:28PM +0200, Jacopo Mondi wrote: > Initialize the ControlInfoMap of controls supported by the Raspberry > pipeline handler and IPA using the list of the enumerated values instead > of specifying them manually. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/ipa/raspberrypi.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > index b23baf2f1330..4ca1528ad097 100644 > --- a/include/libcamera/ipa/raspberrypi.h > +++ b/include/libcamera/ipa/raspberrypi.h > @@ -50,13 +50,13 @@ static const ControlInfoMap Controls = { > { &controls::AeEnable, ControlInfo(false, true) }, > { &controls::ExposureTime, ControlInfo(0, 999999) }, > { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > - { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, > - { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, > - { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, > + { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) }, > + { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, > + { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > { &controls::AwbEnable, ControlInfo(false, true) }, > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > - { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, > + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, This looks good to me, but I haven't checked if the RPi IPA actually supports all possible values. David or Naush, would you be able to comment on this ? > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
Hi Laurent and Jacopo, On Thu, 22 Oct 2020 at 03:47, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Jacopo, > > Thank you for the patch. > > CC'ing Naush and David. > > On Wed, Oct 21, 2020 at 04:36:28PM +0200, Jacopo Mondi wrote: > > Initialize the ControlInfoMap of controls supported by the Raspberry > > pipeline handler and IPA using the list of the enumerated values instead > > of specifying them manually. > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/ipa/raspberrypi.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/include/libcamera/ipa/raspberrypi.h > b/include/libcamera/ipa/raspberrypi.h > > index b23baf2f1330..4ca1528ad097 100644 > > --- a/include/libcamera/ipa/raspberrypi.h > > +++ b/include/libcamera/ipa/raspberrypi.h > > @@ -50,13 +50,13 @@ static const ControlInfoMap Controls = { > > { &controls::AeEnable, ControlInfo(false, true) }, > > { &controls::ExposureTime, ControlInfo(0, 999999) }, > > { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, > > - { &controls::AeMeteringMode, ControlInfo(0, > static_cast<int32_t>(controls::MeteringModeMax)) }, > > - { &controls::AeConstraintMode, ControlInfo(0, > static_cast<int32_t>(controls::ConstraintModeMax)) }, > > - { &controls::AeExposureMode, ControlInfo(0, > static_cast<int32_t>(controls::ExposureModeMax)) }, > > + { &controls::AeMeteringMode, > ControlInfo(controls::AeMeteringModeValues) }, > > + { &controls::AeConstraintMode, > ControlInfo(controls::AeConstraintModeValues) }, > > + { &controls::AeExposureMode, > ControlInfo(controls::AeExposureModeValues) }, > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > { &controls::AwbEnable, ControlInfo(false, true) }, > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > - { &controls::AwbMode, ControlInfo(0, > static_cast<int32_t>(controls::AwbModeMax)) }, > > + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > This looks good to me, but I haven't checked if the RPi IPA actually > supports all possible values. David or Naush, would you be able to > comment on this ? > Yes, we do support all listed modes, so this should be fine. Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, > > { &controls::Saturation, ControlInfo(0.0f, 32.0f) }, > > -- > Regards, > > Laurent Pinchart >
diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h index b23baf2f1330..4ca1528ad097 100644 --- a/include/libcamera/ipa/raspberrypi.h +++ b/include/libcamera/ipa/raspberrypi.h @@ -50,13 +50,13 @@ static const ControlInfoMap Controls = { { &controls::AeEnable, ControlInfo(false, true) }, { &controls::ExposureTime, ControlInfo(0, 999999) }, { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) }, - { &controls::AeMeteringMode, ControlInfo(0, static_cast<int32_t>(controls::MeteringModeMax)) }, - { &controls::AeConstraintMode, ControlInfo(0, static_cast<int32_t>(controls::ConstraintModeMax)) }, - { &controls::AeExposureMode, ControlInfo(0, static_cast<int32_t>(controls::ExposureModeMax)) }, + { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) }, + { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, + { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, { &controls::AwbEnable, ControlInfo(false, true) }, { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, - { &controls::AwbMode, ControlInfo(0, static_cast<int32_t>(controls::AwbModeMax)) }, + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, { &controls::Contrast, ControlInfo(0.0f, 32.0f) }, { &controls::Saturation, ControlInfo(0.0f, 32.0f) },