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

Message ID 20200829115429.30010-5-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • 2D transforms
Related show

Commit Message

David Plowman Aug. 29, 2020, 11:54 a.m. UTC
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@raspberrypi.com>
---
 include/libcamera/internal/v4l2_pixelformat.h |  3 +
 src/libcamera/v4l2_pixelformat.cpp            | 94 ++++++++++++++++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Sept. 1, 2020, 12:25 a.m. UTC | #1
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@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 */
Laurent Pinchart Sept. 1, 2020, 12:28 a.m. UTC | #2
Hi David,

I forgot to review the documentation, sorry.

On Tue, Sep 01, 2020 at 03:25:23AM +0300, Laurent Pinchart wrote:
> 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@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.

No need for a trailing . for \brief and \param.
s/pixel format/V4L2 pixel format/

> > + *
> > + * Some sensors that produce Bayer V4L2PixelFormats may change their Bayer

s/V4L2PixelFormats/formats/ (we try not to pluralize class names)

> > + * 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 */

Patch

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>
 
 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
  * \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. */
+	if (!!(t & Transform::VFlip))
+		result = vflipTable.find(result)->second;
+
+	return result;
+}
+
 } /* namespace libcamera */