[libcamera-devel] [PATCH v4 2/7] libcamera: pipeline: raspberrypi: Set sensor orientation during validate

[David Plowman]

Hi Kieran

On Fri, 28 Aug 2020 at 16:02, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi David,
>
> On 28/08/2020 15:41, David Plowman wrote:
> > We set the sensor orientation (h and v flips) during validate as this
> > will in general affect the Bayer order output by the sensor. Doing it
> > here means that the correct raw format gets advertised in any raw
> > streams that the application requested.
>
> Eeep - I'm not sure if we could do this in validate().
>
> Validation should not actually make any change to the hardware, but it
> should check that the configuration can be applied correctly, and make
> any changes that would be necessary to support a correct (and 'valid')
> configuration to be applied through the ->configure()
>

Yes, that's an interesting one. The reason for doing it here is so
that the Bayer format comes out correctly for any raw streams that
were requested, and we're relying on the camera driver to give us the
true Bayer order.

Of course, the camera driver doesn't *have* to change the Bayer order
when you transform it (it might do 1-pixel shits to maintain the
original Bayer order), so the puzzle then is...  how would you know?

Another solution I toyed with - and indeed implemented first - was to
do it in the configure() method, but then I had to dig around and find
any raw stream configurations and update the pixelFormat post facto.
This involves changing stream formats after validate(), which seemed
bad to me too... but do we prefer it?

David

>
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  .../pipeline/raspberrypi/raspberrypi.cpp       | 18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 42c9caa..7aace71 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -400,6 +400,17 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >       if (config_.empty())
> >               return Invalid;
> >
> > +     /*
> > +      * Configure the H/V flip controls based on the sensor rotation. We do
> > +      * this here so that the sensor has the correct Bayer format that will
> > +      * get advertised in the configuration of any raw streams.
> > +      */
> > +     ControlList ctrls(data_->unicam_[Unicam::Image].dev()->controls());
> > +     int32_t rotation = data_->sensor_->properties().get(properties::Rotation);
> > +     ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
> > +     ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
> > +     data_->unicam_[Unicam::Image].dev()->setControls(&ctrls);
> > +
> >       unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0;
> >       std::pair<int, Size> outSize[2];
> >       Size maxSize;
> > @@ -1164,13 +1175,6 @@ int RPiCameraData::configureIPA()
> >                                             { V4L2_CID_EXPOSURE, result.data[1] } });
> >                       sensorMetadata_ = result.data[2];
> >               }
> > -
> > -             /* Configure the H/V flip controls based on the sensor rotation. */
> > -             ControlList ctrls(unicam_[Unicam::Image].dev()->controls());
> > -             int32_t rotation = sensor_->properties().get(properties::Rotation);
> > -             ctrls.set(V4L2_CID_HFLIP, static_cast<int32_t>(!!rotation));
> > -             ctrls.set(V4L2_CID_VFLIP, static_cast<int32_t>(!!rotation));
> > -             unicam_[Unicam::Image].dev()->setControls(&ctrls);
> >       }
> >
> >       if (result.operation & RPI_IPA_CONFIG_SENSOR) {
> >
>
> --
> Regards
> --
> Kieran