[libcamera-devel] [PATCH] libcamera: camera_sensor: Do not initialise unsupported test patterns

[Naushir Patuck]

Hi Kieran,

On Sat, 28 May 2022 at 23:52, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Naushir Patuck (2022-05-25 12:05:49)
> > Hi all,
> >
> > On Tue, 22 Feb 2022 at 06:11, Laurent Pinchart <
> > laurent.pinchart at ideasonboard.com> wrote:
> >
> > > Hi Kieran,
> > >
> > > Thank you for the patch.
> > >
> > > On Sat, Feb 12, 2022 at 11:34:07PM +0000, Kieran Bingham wrote:
> > > > After the CameraSensor class has initialised properties, a default
> > > > setting is applied to ensure that test pattern modes are disabled
> unless
> > > > explicitly requested.
> > > >
> > > > This does not currently check to see if test patterns are supported
> on
> > > > the CameraSensor and the applyTestPatternMode call will report a
> failure
> > > > if it attempts to set a mode when not supported.
> > > >
> > > > Move the initialisation of the test pattern mode to the
> implementation
> > > > of the control mappings to ensure that it is correctly reset at start
> > > > up, while the code path will have already completed early if not
> > > > supported.
> > > >
> > > > Bug: https://github.com/raspberrypi/libcamera-apps/issues/249
> > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > > ---
> > > >  src/libcamera/camera_sensor.cpp | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/libcamera/camera_sensor.cpp
> > > b/src/libcamera/camera_sensor.cpp
> > > > index 345b4a170d47..3a6a1a6bca77 100644
> > > > --- a/src/libcamera/camera_sensor.cpp
> > > > +++ b/src/libcamera/camera_sensor.cpp
> > > > @@ -172,7 +172,7 @@ int CameraSensor::init()
> > > >       if (ret)
> > > >               return ret;
> > > >
> > > > -     return
> > >
> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> > > > +     return 0;
> > > >  }
> > > >
> > > >  int CameraSensor::validateSensorDriver()
> > > > @@ -372,6 +372,9 @@ void CameraSensor::initTestPatternModes()
> > > >
> > > >               testPatternModes_.push_back(it->second);
> > > >       }
> > > > +
> > > > +     /* Initialise the sensor with test patterns disabled. */
> > > > +
> > >
> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> > >
> > > Errors when applyting the test pattern are not propagated up anymore.
> > >
> > > A different approach if you don't want to turn all void functions in
> the
> > > call stack to return an int is to move the error message when
> > > testPatternModes_.empty() from CameraSensor::applyTestPatternMode() to
> > > CameraSensor::setTestPatternMode(). You will need to keep the check in
> > > CameraSensor::applyTestPatternMode() in order to return 0, but
> > > CameraSensor::setTestPatternMode() could return a proper error code.
> > >
> >
> > Have we come to a resolution on this?  It's winding up some of our users,
> > and we could do with suppressing these warnings :-)
>
> I have pretty low availabilty for the next couple of weeks with
> holiday.
>
> Would you be able to send a patch with Laurents suggestion above?
> Hopefully that could be merged quicker.
>

Sure, I can give it a go.  It will probably have to wait for next week as I
am
off for the week after today.  I'll post a patch when it's done.

Regards,
Naush


>
> Of course adding the test pattern controls to the IMX477 is another
> option too?
>
--
> Regards
>
> Kieran
>
>
> >
> > Regards,
> > Naush
> >
> >
> >
> > >
> > > >  }
> > > >
> > > >  int CameraSensor::initProperties()
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220530/ab118980/attachment-0001.htm>