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

[David Plowman]

Oops.

On Fri, 28 Aug 2020 at 16:14, David Plowman
<david.plowman at raspberrypi.com> wrote:
>
> 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

shifts. oh dear... :(

> 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