| Message ID | 20200821155641.11839-3-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the patch. On Fri, Aug 21, 2020 at 04:56:38PM +0100, David Plowman wrote: > Add a field to the CameraConfiguration (including the necessary > documentation) to represent a 2D transform requested by the > application. All pipeline handlers are amended to coerce this to the > Identity, marking the configuration as "adjusted" if something > different had been requested. > > Pipeline handlers that support Transforms can be amended subsequently. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > --- > include/libcamera/camera.h | 3 +++ > src/libcamera/camera.cpp | 16 +++++++++++++++- > src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++ > .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++ > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++++ > src/libcamera/pipeline/simple/simple.cpp | 5 +++++ > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++++ > src/libcamera/pipeline/vimc/vimc.cpp | 5 +++++ > 8 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 272c12c..a2ee4e7 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -17,6 +17,7 @@ > #include <libcamera/request.h> > #include <libcamera/signal.h> > #include <libcamera/stream.h> > +#include <libcamera/transform.h> > > namespace libcamera { > > @@ -61,6 +62,8 @@ public: > bool empty() const; > std::size_t size() const; > > + Transform transform; > + > protected: > CameraConfiguration(); > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 4a9c19c..e12c1a0 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -93,7 +93,7 @@ LOG_DECLARE_CATEGORY(Camera) > * \brief Create an empty camera configuration > */ > CameraConfiguration::CameraConfiguration() > - : config_({}) > + : transform(Transform::Identity), config_({}) > { > } > > @@ -250,6 +250,20 @@ std::size_t CameraConfiguration::size() const > return config_.size(); > } > > +/** > + * \var CameraConfiguration::transform > + * \brief User-specified transform to be applied to the image > + * > + * The transform is a user-specified 2D plane transform that will be applied > + * to the camera images by the processing pipeline before being handed to > + * the application. This is subsequent to any transform that is already > + * required to fix up any platform-defined rotation. I wonder if this is the right thing to do, or if we're trying to be too clever. Maybe the default configuration should set the transform field to the best value instead (that's mostly Rot180 if the sensor is mounted upside-down), and let the application overwrite it if desired, without having to think about how the user-specified transform will be composed with the platform-defined rotation. I don't think we specify anywhere that cameras will try to correct the image orientation behind the scene. There's no need to change this now, but I think it will be reworked before we stabilize the API. > + * > + * The usual 2D plane transforms are allowed here (horizontal/vertical > + * flips, multiple of 90-degree rotations etc.), but pipeline handlers may > + * adjust this field at their discretion if the selection is not supported. From an application point of view, pipeline handlers should be irrelevant, they're an internal implementation "detail". I would say "... but the validate() function may adjust this field at its discretion ...". With this addressed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + > /** > * \var CameraConfiguration::config_ > * \brief The vector of stream configurations > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 019e50b..0f5ad73 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -138,6 +138,11 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > IPU3_MAX_STREAMS) { > config_.resize(IPU3_MAX_STREAMS); > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index eeaf335..236aa5c 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -400,6 +400,11 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0; > std::pair<int, Size> outSize[2]; > Size maxSize; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 32fdaed..f846733 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -478,6 +478,11 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > 1) { > config_.resize(1); > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index eb72e3b..10223a9 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -438,6 +438,11 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > 1) { > config_.resize(1); > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index bc892ec..fd14248 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -108,6 +108,11 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > 1) { > config_.resize(1); > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp > index cf244f1..bb791d6 100644 > --- a/src/libcamera/pipeline/vimc/vimc.cpp > +++ b/src/libcamera/pipeline/vimc/vimc.cpp > @@ -130,6 +130,11 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() > if (config_.empty()) > return Invalid; > > + if (transform != Transform::Identity) { > + transform = Transform::Identity; > + status = Adjusted; > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > 1) { > config_.resize(1);
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 272c12c..a2ee4e7 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -17,6 +17,7 @@ #include <libcamera/request.h> #include <libcamera/signal.h> #include <libcamera/stream.h> +#include <libcamera/transform.h> namespace libcamera { @@ -61,6 +62,8 @@ public: bool empty() const; std::size_t size() const; + Transform transform; + protected: CameraConfiguration(); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 4a9c19c..e12c1a0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -93,7 +93,7 @@ LOG_DECLARE_CATEGORY(Camera) * \brief Create an empty camera configuration */ CameraConfiguration::CameraConfiguration() - : config_({}) + : transform(Transform::Identity), config_({}) { } @@ -250,6 +250,20 @@ std::size_t CameraConfiguration::size() const return config_.size(); } +/** + * \var CameraConfiguration::transform + * \brief User-specified transform to be applied to the image + * + * The transform is a user-specified 2D plane transform that will be applied + * to the camera images by the processing pipeline before being handed to + * the application. This is subsequent to any transform that is already + * required to fix up any platform-defined rotation. + * + * The usual 2D plane transforms are allowed here (horizontal/vertical + * flips, multiple of 90-degree rotations etc.), but pipeline handlers may + * adjust this field at their discretion if the selection is not supported. + */ + /** * \var CameraConfiguration::config_ * \brief The vector of stream configurations diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 019e50b..0f5ad73 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -138,6 +138,11 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + /* Cap the number of entries to the available streams. */ if (config_.size() > IPU3_MAX_STREAMS) { config_.resize(IPU3_MAX_STREAMS); diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index eeaf335..236aa5c 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -400,6 +400,11 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + unsigned int rawCount = 0, outCount = 0, count = 0, maxIndex = 0; std::pair<int, Size> outSize[2]; Size maxSize; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 32fdaed..f846733 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -478,6 +478,11 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + /* Cap the number of entries to the available streams. */ if (config_.size() > 1) { config_.resize(1); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index eb72e3b..10223a9 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -438,6 +438,11 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + /* Cap the number of entries to the available streams. */ if (config_.size() > 1) { config_.resize(1); diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index bc892ec..fd14248 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -108,6 +108,11 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + /* Cap the number of entries to the available streams. */ if (config_.size() > 1) { config_.resize(1); diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index cf244f1..bb791d6 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -130,6 +130,11 @@ CameraConfiguration::Status VimcCameraConfiguration::validate() if (config_.empty()) return Invalid; + if (transform != Transform::Identity) { + transform = Transform::Identity; + status = Adjusted; + } + /* Cap the number of entries to the available streams. */ if (config_.size() > 1) { config_.resize(1);
Add a field to the CameraConfiguration (including the necessary documentation) to represent a 2D transform requested by the application. All pipeline handlers are amended to coerce this to the Identity, marking the configuration as "adjusted" if something different had been requested. Pipeline handlers that support Transforms can be amended subsequently. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- include/libcamera/camera.h | 3 +++ src/libcamera/camera.cpp | 16 +++++++++++++++- src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++ .../pipeline/raspberrypi/raspberrypi.cpp | 5 +++++ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++++ src/libcamera/pipeline/simple/simple.cpp | 5 +++++ src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++++ src/libcamera/pipeline/vimc/vimc.cpp | 5 +++++ 8 files changed, 48 insertions(+), 1 deletion(-)