[PATCH 1/2] qcam: rotate the viewfinder output as per camera properties

Jacopo Mondi jacopo.mondi at ideasonboard.com
Fri Jun 21 10:27:23 CEST 2024


Hi Joel

On Thu, Jun 20, 2024 at 04:36:06PM GMT, Joel Selvaraj wrote:
> Devicetrees may specify the rotation at which the camera is
> mounted in the hardware. If available, this gets populated in camera
> properties. Rotate the viewfinder output accordingly in qcam qt and
> opengl renderers.
>
> Signed-off-by: Joel Selvaraj <joelselvaraj.oss at gmail.com>
> ---
>  include/libcamera/orientation.h |  2 ++
>  src/apps/qcam/main_window.cpp   |  9 ++++++-
>  src/apps/qcam/viewfinder.h      |  4 ++-
>  src/apps/qcam/viewfinder_gl.cpp | 46 ++++++++++++++++++++++++++++++---
>  src/apps/qcam/viewfinder_gl.h   |  4 ++-
>  src/apps/qcam/viewfinder_qt.cpp | 18 +++++++++++--
>  src/apps/qcam/viewfinder_qt.h   |  5 +++-
>  src/libcamera/orientation.cpp   | 34 ++++++++++++++++++++++++
>  8 files changed, 113 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/orientation.h b/include/libcamera/orientation.h
> index a3b40e63..e32f5eb8 100644
> --- a/include/libcamera/orientation.h
> +++ b/include/libcamera/orientation.h
> @@ -25,6 +25,8 @@ enum class Orientation {
>
>  Orientation orientationFromRotation(int angle, bool *success = nullptr);
>
> +int rotationFromOrientation(const Orientation &orientation, bool *success = nullptr);
> +
>  std::ostream &operator<<(std::ostream &out, const Orientation &orientation);
>
>  } /* namespace libcamera */
> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> index d515beed..18c94cf3 100644
> --- a/src/apps/qcam/main_window.cpp
> +++ b/src/apps/qcam/main_window.cpp
> @@ -363,6 +363,7 @@ void MainWindow::toggleCapture(bool start)
>  int MainWindow::startCapture()
>  {
>  	std::vector<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);
> +	Orientation orientation = Orientation::Rotate0;
>  	int ret;
>
>  	/* Verify roles are supported. */
> @@ -392,6 +393,12 @@ int MainWindow::startCapture()
>  		return -EINVAL;
>  	}
>
> +	/* Get orientation provided by camera kernel driver */
> +	const ControlList &properties = camera_->properties();
> +	const auto &rotation = properties.get(properties::Rotation);
> +	if (rotation)
> +		orientation = orientationFromRotation(*rotation);
> +
>  	StreamConfiguration &vfConfig = config_->at(0);
>
>  	/* Use a format supported by the viewfinder if available. */
> @@ -444,7 +451,7 @@ int MainWindow::startCapture()
>  	ret = viewfinder_->setFormat(vfConfig.pixelFormat,
>  				     QSize(vfConfig.size.width, vfConfig.size.height),
>  				     vfConfig.colorSpace.value_or(ColorSpace::Sycc),
> -				     vfConfig.stride);
> +				     vfConfig.stride, orientation);
>  	if (ret < 0) {
>  		qInfo() << "Failed to set viewfinder format";
>  		return ret;
> diff --git a/src/apps/qcam/viewfinder.h b/src/apps/qcam/viewfinder.h
> index 914f88ec..f17cc4a1 100644
> --- a/src/apps/qcam/viewfinder.h
> +++ b/src/apps/qcam/viewfinder.h
> @@ -14,6 +14,7 @@
>  #include <libcamera/color_space.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/framebuffer.h>
> +#include <libcamera/orientation.h>
>
>  class Image;
>
> @@ -26,7 +27,8 @@ public:
>
>  	virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size,
>  			      const libcamera::ColorSpace &colorSpace,
> -			      unsigned int stride) = 0;
> +			      unsigned int stride,
> +			      const libcamera::Orientation &orientation) = 0;
>  	virtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0;
>  	virtual void stop() = 0;
>
> diff --git a/src/apps/qcam/viewfinder_gl.cpp b/src/apps/qcam/viewfinder_gl.cpp
> index 9d2a6960..c4b10e1e 100644
> --- a/src/apps/qcam/viewfinder_gl.cpp
> +++ b/src/apps/qcam/viewfinder_gl.cpp
> @@ -77,7 +77,7 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const
>
>  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &size,
>  			    const libcamera::ColorSpace &colorSpace,
> -			    unsigned int stride)
> +			    unsigned int stride, const libcamera::Orientation &orientation)
>  {
>  	if (format != format_ || colorSpace != colorSpace_) {
>  		/*
> @@ -101,6 +101,7 @@ int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &s
>
>  	size_ = size;
>  	stride_ = stride;
> +	orientation_ = orientation;
>
>  	updateGeometry();
>  	return 0;
> @@ -511,7 +512,7 @@ void ViewFinderGL::initializeGL()
>  	glEnable(GL_TEXTURE_2D);
>  	glDisable(GL_DEPTH_TEST);
>
> -	static const GLfloat coordinates[2][4][2]{
> +	GLfloat coordinates[2][4][2]{
>  		{
>  			/* Vertex coordinates */
>  			{ -1.0f, -1.0f },
> @@ -528,6 +529,41 @@ void ViewFinderGL::initializeGL()
>  		},
>  	};
>
> +	switch (orientation_) {
> +	case libcamera::Orientation::Rotate90:
> +		coordinates[0][0][0] = -1.0f;
> +		coordinates[0][0][1] = +1.0f;
> +		coordinates[0][1][0] = +1.0f;
> +		coordinates[0][1][1] = +1.0f;
> +		coordinates[0][2][0] = +1.0f;
> +		coordinates[0][2][1] = -1.0f;
> +		coordinates[0][3][0] = -1.0f;
> +		coordinates[0][3][1] = -1.0f;
> +		break;
> +	case libcamera::Orientation::Rotate180:
> +		coordinates[0][0][0] = +1.0f;
> +		coordinates[0][0][1] = +1.0f;
> +		coordinates[0][1][0] = +1.0f;
> +		coordinates[0][1][1] = -1.0f;
> +		coordinates[0][2][0] = -1.0f;
> +		coordinates[0][2][1] = -1.0f;
> +		coordinates[0][3][0] = -1.0f;
> +		coordinates[0][3][1] = +1.0f;
> +		break;
> +	case libcamera::Orientation::Rotate270:
> +		coordinates[0][0][0] = +1.0f;
> +		coordinates[0][0][1] = -1.0f;
> +		coordinates[0][1][0] = -1.0f;
> +		coordinates[0][1][1] = -1.0f;
> +		coordinates[0][2][0] = -1.0f;
> +		coordinates[0][2][1] = +1.0f;
> +		coordinates[0][3][0] = +1.0f;
> +		coordinates[0][3][1] = +1.0f;
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	vertexBuffer_.create();
>  	vertexBuffer_.bind();
>  	vertexBuffer_.allocate(coordinates, sizeof(coordinates));
> @@ -831,5 +867,9 @@ void ViewFinderGL::resizeGL(int w, int h)
>
>  QSize ViewFinderGL::sizeHint() const
>  {
> -	return size_.isValid() ? size_ : QSize(640, 480);
> +	if (orientation_ == libcamera::Orientation::Rotate90 ||
> +	    orientation_ == libcamera::Orientation::Rotate270)
> +		return size_.isValid() ? size_.transposed() : QSize(480, 640);
> +	else
> +		return size_.isValid() ? size_ : QSize(640, 480);
>  }
> diff --git a/src/apps/qcam/viewfinder_gl.h b/src/apps/qcam/viewfinder_gl.h
> index 23744b41..1fc0f827 100644
> --- a/src/apps/qcam/viewfinder_gl.h
> +++ b/src/apps/qcam/viewfinder_gl.h
> @@ -39,7 +39,8 @@ public:
>
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size,
>  		      const libcamera::ColorSpace &colorSpace,
> -		      unsigned int stride) override;
> +		      unsigned int stride,
> +		      const libcamera::Orientation &orientation) override;
>  	void render(libcamera::FrameBuffer *buffer, Image *image) override;
>  	void stop() override;
>
> @@ -68,6 +69,7 @@ private:
>  	libcamera::FrameBuffer *buffer_;
>  	libcamera::PixelFormat format_;
>  	libcamera::ColorSpace colorSpace_;
> +	libcamera::Orientation orientation_;
>  	QSize size_;
>  	unsigned int stride_;
>  	Image *image_;
> diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp
> index 4821c27d..583a74f7 100644
> --- a/src/apps/qcam/viewfinder_qt.cpp
> +++ b/src/apps/qcam/viewfinder_qt.cpp
> @@ -57,7 +57,7 @@ const QList<libcamera::PixelFormat> &ViewFinderQt::nativeFormats() const
>
>  int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &size,
>  			    [[maybe_unused]] const libcamera::ColorSpace &colorSpace,
> -			    unsigned int stride)
> +			    unsigned int stride, const libcamera::Orientation &orientation)
>  {
>  	image_ = QImage();
>
> @@ -80,6 +80,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s
>
>  	format_ = format;
>  	size_ = size;
> +	orientation_ = orientation;
> +
> +	bool success;
> +	int angle = libcamera::rotationFromOrientation(orientation, &success);
> +	if (!success) {
> +		qWarning() << "Unsupported orientation";
> +		return -EINVAL;
> +	}
> +	transform_ = QTransform().rotate(angle);
>
>  	updateGeometry();
>  	return 0;
> @@ -148,6 +157,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *)
>
>  	/* If we have an image, draw it. */
>  	if (!image_.isNull()) {
> +		image_ = image_.transformed(transform_);
>  		painter.drawImage(rect(), image_, image_.rect());
>  		return;
>  	}
> @@ -179,5 +189,9 @@ void ViewFinderQt::paintEvent(QPaintEvent *)
>
>  QSize ViewFinderQt::sizeHint() const
>  {
> -	return size_.isValid() ? size_ : QSize(640, 480);
> +	if (orientation_ == libcamera::Orientation::Rotate90 ||
> +	    orientation_ == libcamera::Orientation::Rotate270)
> +		return size_.isValid() ? size_.transposed() : QSize(480, 640);
> +	else
> +		return size_.isValid() ? size_ : QSize(640, 480);
>  }
> diff --git a/src/apps/qcam/viewfinder_qt.h b/src/apps/qcam/viewfinder_qt.h
> index 4f4b9f11..309b39e5 100644
> --- a/src/apps/qcam/viewfinder_qt.h
> +++ b/src/apps/qcam/viewfinder_qt.h
> @@ -33,7 +33,8 @@ public:
>
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size,
>  		      const libcamera::ColorSpace &colorSpace,
> -		      unsigned int stride) override;
> +		      unsigned int stride,
> +		      const libcamera::Orientation &orientation) override;
>  	void render(libcamera::FrameBuffer *buffer, Image *image) override;
>  	void stop() override;
>
> @@ -51,6 +52,8 @@ private:
>
>  	libcamera::PixelFormat format_;
>  	QSize size_;
> +	QTransform transform_;
> +	libcamera::Orientation orientation_;
>
>  	/* Camera stopped icon */
>  	QSize vfSize_;
> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp
> index 47fd6a32..7afb37a8 100644
> --- a/src/libcamera/orientation.cpp
> +++ b/src/libcamera/orientation.cpp
> @@ -92,6 +92,40 @@ Orientation orientationFromRotation(int angle, bool *success)
>  	return Orientation::Rotate0;
>  }
>
> +/**
> + * \brief Return the rotation angle for a given orientation

I'm a bit unconfortable about having this in the API, as it might
result in something more complex than what it actually looks like.

All the 2d plane transforms expressed by Orientation include
an optional horizontal flip, something that cannot be expressed with
an angular rotation only.

Look at the example images we have in the Orientation class
documentation. If you only report the rotation angle (*)
you won't be able to identify if an image has been flipped or not, and
the result might not be what you expect.

(*) Also expressing the rotation angle only is less trivial than what
one might expect. First of all, if you want to return a rotation
angle, you have to specify what the rotation describes:

1) The image rotation in the memory buffers
2) The correction to be applied to display the image in its natural
orientation

The Orientation class describes the image rotation in memory buffers,
so I presume that's what you're expressing here, but it has to be made
explicit in the documentation of the function. The direction of the
rotation has to be specified as well. All of this is documented in the
Orientation class, but if this function is made part of the API, it
has to be specified here as well.

I would suggest to infer the rotation angle from Orientation in the
application and explicitly say that mirroring is not corrected there.

Thanks
  j

> + * \param[in] orientation The orientation to convert to a rotation angle
> + * \param[out] success Set to `true` if the given orientation is valid,
> + * otherwise `false`
> + * \return The rotation angle corresponding to the given orientation
> + * if \a success was set to `true`, otherwise 0.
> + */
> +int rotationFromOrientation(const Orientation &orientation, bool *success)
> +{
> +	if (success != nullptr)
> +		*success = true;
> +
> +	switch (orientation) {
> +	case Orientation::Rotate0:
> +	case Orientation::Rotate0Mirror:
> +		return 0;
> +	case Orientation::Rotate90:
> +	case Orientation::Rotate90Mirror:
> +		return 90;
> +	case Orientation::Rotate180:
> +	case Orientation::Rotate180Mirror:
> +		return 180;
> +	case Orientation::Rotate270:
> +	case Orientation::Rotate270Mirror:
> +		return 270;
> +	}
> +
> +	if (success != nullptr)
> +		*success = false;
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Prints human-friendly names for Orientation items
>   * \param[in] out The output stream
> --
> 2.45.2
>


More information about the libcamera-devel mailing list