[libcamera-devel] [PATCH] pipeline: simple: Validate transform

[Kieran Bingham]

Quoting Jacopo Mondi via libcamera-devel (2023-04-27 12:36:21)
> Hi Robert,
> 
> On Thu, Apr 27, 2023 at 12:53:56PM +0200, Robert Mader via libcamera-devel wrote:
> > Just like we do for other pipeline handlers already.
> > This ensures we corretly pass on transforms that are not handled by the
> > sensor - e.g. rotations - back to the app via the config, which is
> > required on devices like the Pinephone.
> >
> > Signed-off-by: Robert Mader <robert.mader at collabora.com>
> > Tested-by: Arnav Singh <me at arnavion.dev>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 2423ec10..abfb4c87 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -211,7 +211,8 @@ public:
> >       int init();
> >       int setupLinks();
> >       int setupFormats(V4L2SubdeviceFormat *format,
> > -                      V4L2Subdevice::Whence whence);
> > +                      V4L2Subdevice::Whence whence,
> > +                      Transform transform = Transform::Identity);
> >       void bufferReady(FrameBuffer *buffer);
> >
> >       unsigned int streamIndex(const Stream *stream) const
> > @@ -292,6 +293,7 @@ public:
> >       }
> >
> >       bool needConversion() const { return needConversion_; }
> > +     const Transform &combinedTransform() { return combinedTransform_; }
> 
>         const Transform &combinedTransform() const { return combinedTransform_; }
> 
> >
> >  private:
> >       /*
> > @@ -304,6 +306,7 @@ private:
> >
> >       const SimpleCameraData::Configuration *pipeConfig_;
> >       bool needConversion_;
> > +     Transform combinedTransform_;
> >  };
> >
> >  class SimplePipelineHandler : public PipelineHandler
> > @@ -664,7 +667,8 @@ int SimpleCameraData::setupLinks()
> >  }
> >
> >  int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> > -                                V4L2Subdevice::Whence whence)
> > +                                V4L2Subdevice::Whence whence,
> > +                                Transform transform)
> >  {
> >       SimplePipelineHandler *pipe = SimpleCameraData::pipe();
> >       int ret;
> > @@ -673,7 +677,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> >        * Configure the format on the sensor output and propagate it through
> >        * the pipeline.
> >        */
> > -     ret = sensor_->setFormat(format);
> > +     ret = sensor_->setFormat(format, transform);
> >       if (ret < 0)
> >               return ret;
> >
> > @@ -877,15 +881,16 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,
> >
> >  CameraConfiguration::Status SimpleCameraConfiguration::validate()
> >  {
> > +     const CameraSensor *sensor = data_->sensor_.get();
> >       Status status = Valid;
> >
> >       if (config_.empty())
> >               return Invalid;
> >
> > -     if (transform != Transform::Identity) {
> > -             transform = Transform::Identity;
> > +     Transform requestedTransform = transform;
> > +     combinedTransform_ = sensor->validateTransform(&transform);
> > +     if (transform != requestedTransform)
> >               status = Adjusted;
> > -     }
> >
> >       /* Cap the number of entries to the available streams. */
> >       if (config_.size() > data_->streams_.size()) {
> > @@ -1116,7 +1121,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> >       format.mbus_code = pipeConfig->code;
> >       format.size = pipeConfig->sensorSize;
> >
> > -     ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);
> > +     ret = data->setupFormats(&format,
> > +                              V4L2Subdevice::ActiveFormat,
> > +                              config->combinedTransform());
> 
> Probably fits in 2 lines only
> 
> Minor nits, can be fixed when applying
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Minors addressed and applied.
--
Kieran

> 
> >       if (ret < 0)
> >               return ret;
> >
> > --
> > 2.40.0
> >