[libcamera-devel] [RFC PATCH 0/2] Resolve invalid attempts to set sensor flip controls

[Jacopo Mondi]

Hi Naush

On Thu, Nov 03, 2022 at 12:05:31PM +0000, Naushir Patuck wrote:
> Hi Jacopo,
>
>
> On Thu, 3 Nov 2022 at 11:50, Jacopo Mondi via libcamera-devel <
> libcamera-devel at lists.libcamera.org> wrote:
>
> > Hi David
> >
> > On Thu, Nov 03, 2022 at 10:40:25AM +0000, David Plowman via
> > libcamera-devel wrote:
> > > Hi everyone
> > >
> > > (Warning: may affect all pipeline handlers!)
> > >
> > > I wanted to start the ball rolling on this. The problem is that when
> > > we enumerate the cameras in the system, we try to set some of the
> > > controls (specifically the HFLIP and VFLIP ones, and also later
> > > HBLANK). But this will fail if we don't "own" the camera, for example,
> > > it is already in use elsewhere.
> >
> > Feels like I'm missing one point here..
> >
> > How do you get to CameraSensor::init() if your pipeline handler
> > acquires the unicam media device at the very beginning of match() ?
> >
> > I understand we allow multiple application acquire the same PH as the
> > ph might register multiple cameras, but why is this a problem at
> > match() time ?
> >
>
> In match(), the rpi, rkisp and ipu3 pipeline handlers call
> CameraSensor::init() as part of the Camera object registration.  This call
> is unconditional assuming we can enumerate and register the sensor.
>

Exactly, so CameraSensor::init() is called once, regardless of which
application will later "own" the camera. Sensor's controls are
reset indeed, is this the problem, that application do not expect to
have the controls reset ?

> Not sure if that answers your question though?
>
> Naush
>
>
>
> >
> > >
> > > These patches address just the flip bits and not the HBLANK, though I
> > > think the latter is a more straightforward problem. The flip bits are
> > > troublesome because they affect the reported Bayer orders.
> > >
> > > So here the idea is to store the formats in the sensor's native Bayer
> > > order, as we were doing before. Only we can't reset the flip bits now,
> > > we have to query them and transform any Bayer formats the sensor gives
> > > us back into the native order. Two patches implement this:
> > >
> > > 1. The first patch adds a function to turn BayerFormats back into mbus
> > > codes. I couldn't see that we had functions to do this already, but
> > > please correct me if I missed something!
> > >
> > > 2. The second patch queries the flip bits and transforms any
> > > BayerFormats back into the native order to be stored.
> > >
> > > As I said, this potentially affects all pipeline handlers, which is
> > > why I left it "RFC" for the moment. I did take a look through them all
> > > to see what we might need to do:
> > >
> > > raspberrypi.cpp
> > >
> > > I think this PH is OK. We assume we're given the native Bayer order
> > > and transform it according to the final sensor transform, setting the
> > > flip bits every time. We update the format in any raw stream to match.
> > >
> > > uvcvideo.cpp, simple.cpp
> > >
> > > These appear to hardcode the transform to the Identity. I think mostly
> > > they should be OK because the formats here won't be raw and you can't
> > > have raw streams (?). Probably they should clear the flip bits to zero
> > > where these controls exist, perhaps someone who knows more about them
> > > could comment.
> > >
> > > rkisp1.cpp:
> > >
> > > Seems to force the transform to the Identity as well. So I think this
> > > is like the above ones, it should be OK though it probably needs to
> > > clear the sensor flip bits as well.
> > >
> > > ipu3.cpp:
> > >
> > > This one does handle transforms and appears to set the flip bits, so
> > > that's good. I couldn't see it transforming the Bayer order for raw
> > > streams correctly, so that probably wants checking. (To be fair, this
> > > looks suspect already and is not made worse by these changes!)
> > >
> > > Thoughts and opinions gratefully received, as always.
> > >
> > > Thanks!
> > > David
> > >
> > > David Plowman (2):
> > >   libcamera: bayer_format: Add toMbusCode method
> > >   libcamera: camera_sensor: Do not clear camera flips when listing
> > >     formats
> > >
> > >  include/libcamera/internal/bayer_format.h |  1 +
> > >  src/libcamera/bayer_format.cpp            | 12 ++++++
> > >  src/libcamera/camera_sensor.cpp           | 51 +++++++++++++++++++----
> > >  3 files changed, 56 insertions(+), 8 deletions(-)
> > >
> > > --
> > > 2.30.2
> > >
> >