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

[Jacopo Mondi]

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 ?

>
> 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
>