[libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose transformation

[Laurent Pinchart]

Hi David,

Would you have a bit of time to review this ?

On Wed, Jan 27, 2021 at 01:07:38AM +0200, Laurent Pinchart wrote:
> Hi Sebastian,
> 
> Thank you for the patch.
> 
> On Tue, Jan 26, 2021 at 07:48:53PM +0100, Sebastian Fricke wrote:
> > To transpose a BayerFormat means to flip it over its main-diagonal.
> 
> s/main-diagonal/main diagonal/
> 
> > 
> > For example:
> > G B    G R
> >     ->
> > R G    B G
> > 
> > The main-diagonal goes from the top left to the bottom right.
> 
> Same here.
> 
> > This means, that the only two orders affected by a transpose
> > are GBRG & GRBG. When a transpose is used in combination with horizontal
> > and/or vertical flips it is performed with the lowest priority.
> > Therefore add the functionality by switching GBRG (index 1) with GRBG
> > (index 2), after the flips have been applied.
> > 
> > Signed-off-by: Sebastian Fricke <sebastian.fricke at posteo.net>
> > ---
> >  src/libcamera/bayer_format.cpp | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> > index d4e7f142..e06cd6e8 100644
> > --- a/src/libcamera/bayer_format.cpp
> > +++ b/src/libcamera/bayer_format.cpp
> > @@ -272,10 +272,6 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)
> >   * The transformed image would have a GRBG order. The bit depth and modifiers
> >   * are not affected.
> >   *
> > - * Note that transpositions are ignored as the order of a transpose with
> > - * respect to the flips would have to be defined, and sensors are not expected
> > - * to support transposition.
> > - *
> 
> This needs to now document the order in which the flips and transpose
> are applied. Maybe something along the lines of
> 
>   * Horizontal and vertical flips are applied before transpose.
> 
> With this change,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> but I'd like to get David's feedback on this.
> 
> >   * \return The transformed Bayer format
> >   */
> >  BayerFormat BayerFormat::transform(Transform t) const
> > @@ -292,6 +288,11 @@ BayerFormat BayerFormat::transform(Transform t) const
> >  	if (!!(t & Transform::VFlip))
> >  		result.order = static_cast<Order>(result.order ^ 2);
> >  
> > +	if (!!(t & Transform::Transpose) && result.order == 1)
> > +		result.order = static_cast<Order>(2);
> > +	else if (!!(t & Transform::Transpose) && result.order == 2)
> > +		result.order = static_cast<Order>(1);
> > +
> >  	return result;
> >  }
> >  

-- 
Regards,

Laurent Pinchart