[libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel formats to be transformed

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Sep 1 02:25:16 CEST 2020


Hi David,

Thank you for the patch.

On Sat, Aug 29, 2020 at 12:54:25PM +0100, David Plowman wrote:
> Add a transform method to the V4L2PixelFormat class which allows Bayer
> formats to be re-ordered into a different Bayer order when horizontal
> or vertical flips are requested from the sensor.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/internal/v4l2_pixelformat.h |  3 +
>  src/libcamera/v4l2_pixelformat.cpp            | 94 ++++++++++++++++++-
>  2 files changed, 96 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index 9bfd81a..cddac86 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -14,6 +14,7 @@
>  #include <linux/videodev2.h>
>  
>  #include <libcamera/pixel_format.h>
> +#include <libcamera/transform.h>

You can use a forward declaration of enum class Transform in this file,
and include transform.h in v4l2_pixelformat.cpp.

>  
>  namespace libcamera {
>  
> @@ -40,6 +41,8 @@ public:
>  	static V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,
>  					       bool multiplanar);
>  
> +	V4L2PixelFormat transform(Transform t) const;
> +
>  private:
>  	uint32_t fourcc_;
>  };
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 30c94bb..28bcd51 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -101,6 +101,62 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), formats::MJPEG },
>  };
>  
> +/* Table giving the result of applying an hflip to each Bayer pixel format. */
> +const std::map<V4L2PixelFormat, V4L2PixelFormat> hflipTable{
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
> +};
> +
> +/* Table giving the result of applying a vflip to each Bayer pixel format. */
> +const std::map<V4L2PixelFormat, V4L2PixelFormat> vflipTable{
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
> +};
> +
>  } /* namespace */
>  
>  /**
> @@ -113,7 +169,7 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  
>  /**
>   * \fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc)
> - * \brief Construct a V4L2PixelFormat from a FourCC value
> +p * \brief Construct a V4L2PixelFormat from a FourCC value

Probably an unwanted addition ?

>   * \param[in] fourcc The pixel format FourCC numerical value
>   */
>  
> @@ -205,4 +261,40 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,
>  	return info.v4l2Format;
>  }
>  
> +/**
> + * \brief Transform a Bayer V4L2PixelFormat.
> + * \param[in] t The Transform to be applied to the pixel format.
> + *
> + * Some sensors that produce Bayer V4L2PixelFormats may change their Bayer
> + * order when the V4L2_CID_HFLIP or V4L2_CID_VFLIP controls are applied to the
> + * device. For such sensors, this function computes the new V4L2PixelFormat
> + * for the transformed Bayer order.
> + *
> + * Transposes are ignored as sensors do not implement them.
> + *
> + * \return The Bayer V4L2PixelFormat resulting from applying transform \a t
> + * to the given V4L2PixelFormat.
> + */
> +V4L2PixelFormat V4L2PixelFormat::transform(Transform t) const
> +{
> +	V4L2PixelFormat result = *this;
> +
> +	if (!!(t & Transform::HFlip)) {
> +		const auto iter = hflipTable.find(*this);
> +		if (iter == hflipTable.end()) {
> +			LOG(V4L2, Warning)
> +				<< "Cannot transpose V4L2 pixel format "
> +				<< toString();
> +			return V4L2PixelFormat();
> +		}
> +		result = iter->second;
> +	}
> +
> +	/* This will be found as the tables have the same keys. */

Except that if HFlip isn't set, the above check will not have run.

In order to simplify this a bit, and avoid two lookups, could we merge
the two tables ? We can store a

struct BayerFlipResult {
	V4L2PixelFormat hflip;
	V4L2PixelFormat vflip;
	V4L2PixelFormat hvflip;
};

in the table, and this function could then become

	t &= ~Transform::Transpose;
	if (!t)
		return *this;

	const auto iter = bayerFlipTable.find(*this);
	if (iter == bayerFlipTable.end()) {
		LOG(V4L2, Warning)
			<< "Cannot transpose V4L2 pixel format " << toString();
		return *this;
	}

	const BayerFlipResult &flip = iter->second;

	switch (t) {
	case Transform::HFlip:
		return flip.hflip;
	case Transform::VFlip:
		return flip.vflip;
	case Transform::HVFlip:
	default:
		return flip.hvflip;
	}

We would need a

constexpr Transform operator~(Transform t)
{
	return static_cast<Transform>(~static_cast<int>(t) & 7);
}

Alternatively the first line could become

	t &= (Transform::HFlip | TransformVFlip);

The downside is that the table would become a bit less readable:

const std::map<V4L2PixelFormat, BayerFlipResult> bayerFlipTable{
	{
		V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),
		{
			.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),
			.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),
			.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),
		},
	}, {
		V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),
		{
			.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),
			.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),
			.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),
		},
	}, {
	...
};

but that's not that bad I think.

Note that in the future we are considering not expressing the bayer
pattern as part of the DRM fourcc, but using DRM_FORMAT_R8 and conveying
the Bayer pattern separately. It would be nice if we could handle all
this at the PixelFormat level instead, we could avoid a large table, but
I fear that won't be possible as we can't change the V4L2 side.

> +	if (!!(t & Transform::VFlip))
> +		result = vflipTable.find(result)->second;
> +
> +	return result;
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list