[libcamera-devel,1/2] libcamera: Fix V4L2 pixel format mapping for XRGB32
diff mbox series

Message ID 20210618200247.624987-2-vedantparanjape160201@gmail.com
State Superseded
Headers show
Series
  • Fixed mismatch of pixel formats of V4L2 and libcamera
Related show

Commit Message

Vedant Paranjape June 18, 2021, 8:02 p.m. UTC 
Fixed the mismatch of V4L2 and Libcamera pixelformats.

Matched V4L2_PIX_FMT_XRGB32 with formats::BGRX8888. Additionally, fixes
mapping of V4L2_PIX_FMT_RGBX32 to formats::XBGR8888 in formats.cpp

Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
---
 src/libcamera/formats.cpp          | 12 +++++++++++-
 src/libcamera/v4l2_pixelformat.cpp |  2 +-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Paul Elder June 22, 2021, 4:26 a.m. UTC | #1 
Hi Vedant,

On Sat, Jun 19, 2021 at 01:32:46AM +0530, Vedant Paranjape wrote:
> Fixed the mismatch of V4L2 and Libcamera pixelformats.
> 
> Matched V4L2_PIX_FMT_XRGB32 with formats::BGRX8888. Additionally, fixes
> mapping of V4L2_PIX_FMT_RGBX32 to formats::XBGR8888 in formats.cpp

The points that you did in this series are:
- add the BGRX8888 format
- fix the mapping of XRGB32 to BGRX8888
- fix the mapping of RGBX32 to XBGR8888

The last two you mention, but I think you should mention the first as
well.

> 
> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> ---
>  src/libcamera/formats.cpp          | 12 +++++++++++-
>  src/libcamera/v4l2_pixelformat.cpp |  2 +-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 55822f4c..e41bfc05 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -198,13 +198,23 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>  	{ formats::XBGR8888, {
>  		.name = "XBGR8888",
>  		.format = formats::XBGR8888,
> -		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
>  		.bitsPerPixel = 32,
>  		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
>  		.packed = false,
>  		.pixelsPerGroup = 1,
>  		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
>  	} },
> +	{ formats::BGRX8888, {
> +		.name = "BGRX8888",
> +		.format = formats::BGRX8888,
> +		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),

Hm, maybe this is too intertwined. Here you add BGRX8888 = XRGB32, which
means that you have to fix the mapping above to RGBX32, but the
corresponding mapping in the v4l2 pixel format map is only added later
in 2/2.

I think it's okay to squash 2/2 into this.


Paul

> +		.bitsPerPixel = 32,
> +		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
> +		.packed = false,
> +		.pixelsPerGroup = 1,
> +		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> +	} },	
>  	{ formats::ABGR8888, {
>  		.name = "ABGR8888",
>  		.format = formats::ABGR8888,
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 166d93cf..950cbda4 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -51,7 +51,7 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), formats::BGR888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_BGR24), formats::RGB888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_XBGR32), formats::XRGB8888 },
> -	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::XBGR8888 },
> +	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::BGRX8888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_RGBA32), formats::ABGR8888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_ABGR32), formats::ARGB8888 },
>  	{ V4L2PixelFormat(V4L2_PIX_FMT_ARGB32), formats::BGRA8888 },
> -- 
> 2.25.1
>
Vedant Paranjape June 22, 2021, 7:07 a.m. UTC | #2 
Hi Paul,

Ok, will do that.

Regards,
Vedant Paranjape

On Tue, Jun 22, 2021 at 9:56 AM <paul.elder@ideasonboard.com> wrote:

> Hi Vedant,
>
> On Sat, Jun 19, 2021 at 01:32:46AM +0530, Vedant Paranjape wrote:
> > Fixed the mismatch of V4L2 and Libcamera pixelformats.
> >
> > Matched V4L2_PIX_FMT_XRGB32 with formats::BGRX8888. Additionally, fixes
> > mapping of V4L2_PIX_FMT_RGBX32 to formats::XBGR8888 in formats.cpp
>
> The points that you did in this series are:
> - add the BGRX8888 format
> - fix the mapping of XRGB32 to BGRX8888
> - fix the mapping of RGBX32 to XBGR8888
>
> The last two you mention, but I think you should mention the first as
> well.
>
> >
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> > ---
> >  src/libcamera/formats.cpp          | 12 +++++++++++-
> >  src/libcamera/v4l2_pixelformat.cpp |  2 +-
> >  2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 55822f4c..e41bfc05 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -198,13 +198,23 @@ const std::map<PixelFormat, PixelFormatInfo>
> pixelFormatInfo{
> >       { formats::XBGR8888, {
> >               .name = "XBGR8888",
> >               .format = formats::XBGR8888,
> > -             .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
> > +             .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
> >               .bitsPerPixel = 32,
> >               .colourEncoding = PixelFormatInfo::ColourEncodingRGB,
> >               .packed = false,
> >               .pixelsPerGroup = 1,
> >               .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> >       } },
> > +     { formats::BGRX8888, {
> > +             .name = "BGRX8888",
> > +             .format = formats::BGRX8888,
> > +             .v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
>
> Hm, maybe this is too intertwined. Here you add BGRX8888 = XRGB32, which
> means that you have to fix the mapping above to RGBX32, but the
> corresponding mapping in the v4l2 pixel format map is only added later
> in 2/2.
>
> I think it's okay to squash 2/2 into this.
>
>
> Paul
>
> > +             .bitsPerPixel = 32,
> > +             .colourEncoding = PixelFormatInfo::ColourEncodingRGB,
> > +             .packed = false,
> > +             .pixelsPerGroup = 1,
> > +             .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> > +     } },
> >       { formats::ABGR8888, {
> >               .name = "ABGR8888",
> >               .format = formats::ABGR8888,
> > diff --git a/src/libcamera/v4l2_pixelformat.cpp
> b/src/libcamera/v4l2_pixelformat.cpp
> > index 166d93cf..950cbda4 100644
> > --- a/src/libcamera/v4l2_pixelformat.cpp
> > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > @@ -51,7 +51,7 @@ const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
> >       { V4L2PixelFormat(V4L2_PIX_FMT_RGB24), formats::BGR888 },
> >       { V4L2PixelFormat(V4L2_PIX_FMT_BGR24), formats::RGB888 },
> >       { V4L2PixelFormat(V4L2_PIX_FMT_XBGR32), formats::XRGB8888 },
> > -     { V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::XBGR8888 },
> > +     { V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::BGRX8888 },
> >       { V4L2PixelFormat(V4L2_PIX_FMT_RGBA32), formats::ABGR8888 },
> >       { V4L2PixelFormat(V4L2_PIX_FMT_ABGR32), formats::ARGB8888 },
> >       { V4L2PixelFormat(V4L2_PIX_FMT_ARGB32), formats::BGRA8888 },
> > --
> > 2.25.1
> >
>

Patch
diff mbox series 

diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
index 55822f4c..e41bfc05 100644
--- a/src/libcamera/formats.cpp
+++ b/src/libcamera/formats.cpp
@@ -198,13 +198,23 @@  const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
 	{ formats::XBGR8888, {
 		.name = "XBGR8888",
 		.format = formats::XBGR8888,
-		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
 		.bitsPerPixel = 32,
 		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
 		.packed = false,
 		.pixelsPerGroup = 1,
 		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
 	} },
+	{ formats::BGRX8888, {
+		.name = "BGRX8888",
+		.format = formats::BGRX8888,
+		.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_XRGB32),
+		.bitsPerPixel = 32,
+		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
+		.packed = false,
+		.pixelsPerGroup = 1,
+		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
+	} },	
 	{ formats::ABGR8888, {
 		.name = "ABGR8888",
 		.format = formats::ABGR8888,
diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
index 166d93cf..950cbda4 100644
--- a/src/libcamera/v4l2_pixelformat.cpp
+++ b/src/libcamera/v4l2_pixelformat.cpp
@@ -51,7 +51,7 @@  const std::map<V4L2PixelFormat, PixelFormat> vpf2pf{
 	{ V4L2PixelFormat(V4L2_PIX_FMT_RGB24), formats::BGR888 },
 	{ V4L2PixelFormat(V4L2_PIX_FMT_BGR24), formats::RGB888 },
 	{ V4L2PixelFormat(V4L2_PIX_FMT_XBGR32), formats::XRGB8888 },
-	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::XBGR8888 },
+	{ V4L2PixelFormat(V4L2_PIX_FMT_XRGB32), formats::BGRX8888 },
 	{ V4L2PixelFormat(V4L2_PIX_FMT_RGBA32), formats::ABGR8888 },
 	{ V4L2PixelFormat(V4L2_PIX_FMT_ABGR32), formats::ARGB8888 },
 	{ V4L2PixelFormat(V4L2_PIX_FMT_ARGB32), formats::BGRA8888 },