<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 15 Sept 2021 at 09:29, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Sep 15, 2021 at 08:53:50AM +0100, Naushir Patuck wrote:<br>
> On Wed, 15 Sept 2021 at 02:49, Laurent Pinchart wrote:<br>
> > On Mon, Sep 13, 2021 at 07:20:06PM +0900, Paul Elder wrote:<br>
> > > Remove the NoiseReductionMode values that the raspberrypi IPA does not<br>
> > > support. The ControlInfo values that the IPA reports will be used for<br>
> > > capability detection, so values that it does not support shall be<br>
> > > removed.<br>
> > ><br>
> > > Signed-off-by: Paul Elder <<a href="mailto:paul.elder@ideasonboard.com" target="_blank">paul.elder@ideasonboard.com</a>><br>
> > > ---<br>
> > >  include/libcamera/ipa/raspberrypi.h | 8 +++++++-<br>
> > >  src/ipa/raspberrypi/raspberrypi.cpp | 2 --<br>
> > >  2 files changed, 7 insertions(+), 3 deletions(-)<br>
> > ><br>
> > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h<br>
> > > index e0dc6f5e..6e97ef53 100644<br>
> > > --- a/include/libcamera/ipa/raspberrypi.h<br>
> > > +++ b/include/libcamera/ipa/raspberrypi.h<br>
> > > @@ -18,6 +18,12 @@ namespace libcamera {<br>
> > ><br>
> > >  namespace RPi {<br>
> > ><br>
> > > +const std::array<const ControlValue, 3> RPiNoiseReductionModeValues = {<br>
> ><br>
> > Can this be constexpr ?<br>
> ><br>
> > > +     static_cast<int32_t>(controls::NoiseReductionModeOff),<br>
> > > +     static_cast<int32_t>(controls::NoiseReductionModeFast),<br>
> > > +     static_cast<int32_t>(controls::NoiseReductionModeHighQuality),<br>
> > > +};<br>
> ><br>
> > I'm wondering, now that we have the ability to pass control info from<br>
> > the IPA to the pipeline handler, could we move the control info map to<br>
> > the IPA before adding more static data ?<br>
> ><br>
> > Naush, is this something you have looked at by any chance ?<br>
> <br>
> I haven't looked into this, but it makes sense to do it this way.<br>
> It would be almost trivial to pass the control info from ipa::init().<br>
<br>
Sounds good to me. Let's avoid Paul and you implementing this in<br>
parallel, is this something you plan to work on ?<br></blockquote><div><br></div><div>I could do the change (hopefully) by the end of the week.</div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> > > +<br>
> > >  /*<br>
> > >   * List of controls handled by the Raspberry Pi IPA<br>
> > >   *<br>
> > > @@ -45,7 +51,7 @@ static const ControlInfoMap Controls({<br>
> > >               { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f,<br>
> > 16.0f) },<br>
> > >               { &controls::ScalerCrop, ControlInfo(Rectangle{},<br>
> > Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },<br>
> > >               { &controls::FrameDurationLimits,<br>
> > ControlInfo(INT64_C(1000), INT64_C(1000000000)) },<br>
> > > -             { &controls::NoiseReductionMode,<br>
> > ControlInfo(controls::NoiseReductionModeValues) }<br>
> > > +             { &controls::NoiseReductionMode,<br>
> > ControlInfo(RPiNoiseReductionModeValues) }<br>
> > >       }, controls::controls);<br>
> > ><br>
> > >  } /* namespace RPi */<br>
> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> > b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> > > index 8d44ab0a..daef1c2d 100644<br>
> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> > > @@ -608,8 +608,6 @@ static const std::map<int32_t,<br>
> > RPiController::DenoiseMode> DenoiseModeTable = {<br>
> > >       { controls::NoiseReductionModeOff, RPiController::DenoiseMode::Off<br>
> > },<br>
> > >       { controls::NoiseReductionModeFast,<br>
> > RPiController::DenoiseMode::ColourFast },<br>
> > >       { controls::NoiseReductionModeHighQuality,<br>
> > RPiController::DenoiseMode::ColourHighQuality },<br>
> > > -     { controls::NoiseReductionModeMinimal,<br>
> > RPiController::DenoiseMode::ColourOff },<br>
> > > -     { controls::NoiseReductionModeZSL,<br>
> > RPiController::DenoiseMode::ColourHighQuality },<br>
> > >  };<br>
> > ><br>
> > >  void IPARPi::queueRequest(const ControlList &controls)<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>