[libcamera-devel,2/2] libcamera: formats: add missing RGBX8888 info
diff mbox series

Message ID 20220518122014.54506-2-tomi.valkeinen@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,1/2] libcamera: formats: fix warning print
Related show

Commit Message

Tomi Valkeinen May 18, 2022, 12:20 p.m. UTC 
Add missing RGBX8888 PixelFormatInfo.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 src/libcamera/formats.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Laurent Pinchart May 18, 2022, 11:20 p.m. UTC | #1 
Hi Tomi,

Thank you for the patch.

On Wed, May 18, 2022 at 03:20:14PM +0300, Tomi Valkeinen wrote:
> Add missing RGBX8888 PixelFormatInfo.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  src/libcamera/formats.cpp | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 3e60ec7d..1c710541 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -231,6 +231,19 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>  		.pixelsPerGroup = 1,
>  		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
>  	} },
> +	{ formats::RGBX8888, {
> +		.name = "RGBX8888",
> +		.format = formats::RGBX8888,
> +		.v4l2Formats = {
> +			.single = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),

I think this should be BGRX32. Furthermore, you need to also update the
vpf2pf map in v4l2_pixelformat.cpp.

> +			.multi = V4L2PixelFormat(),
> +		},
> +		.bitsPerPixel = 32,
> +		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
> +		.packed = false,
> +		.pixelsPerGroup = 1,
> +		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> +	} },
>  	{ formats::BGRX8888, {
>  		.name = "BGRX8888",
>  		.format = formats::BGRX8888,
Tomi Valkeinen May 19, 2022, 6:52 a.m. UTC | #2 
On 19/05/2022 02:20, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Wed, May 18, 2022 at 03:20:14PM +0300, Tomi Valkeinen wrote:
>> Add missing RGBX8888 PixelFormatInfo.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   src/libcamera/formats.cpp | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
>> index 3e60ec7d..1c710541 100644
>> --- a/src/libcamera/formats.cpp
>> +++ b/src/libcamera/formats.cpp
>> @@ -231,6 +231,19 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>>   		.pixelsPerGroup = 1,
>>   		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
>>   	} },
>> +	{ formats::RGBX8888, {
>> +		.name = "RGBX8888",
>> +		.format = formats::RGBX8888,
>> +		.v4l2Formats = {
>> +			.single = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
> 
> I think this should be BGRX32. Furthermore, you need to also update the

Thanks. Yes, it is obviously wrong, as there was another 
V4L2_PIX_FMT_RGBX32 in the file already. I thought I checked that, but 
apparently not...

I have to say I don't get the V4L2 formats. Why is

V4L2_PIX_FMT_XBGR32  BGRX-8-8-8-8

so the X is not beside the R in the define. But

V4L2_PIX_FMT_XRGB32  XRGB-8-8-8-8

X is beside the R. Well, doesn't matter.

> vpf2pf map in v4l2_pixelformat.cpp.

Ok.

  Tomi
Laurent Pinchart May 19, 2022, 7:07 a.m. UTC | #3 
Hi Tomi,

On Thu, May 19, 2022 at 09:52:44AM +0300, Tomi Valkeinen wrote:
> On 19/05/2022 02:20, Laurent Pinchart wrote:
> > On Wed, May 18, 2022 at 03:20:14PM +0300, Tomi Valkeinen wrote:
> >> Add missing RGBX8888 PixelFormatInfo.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >>   src/libcamera/formats.cpp | 13 +++++++++++++
> >>   1 file changed, 13 insertions(+)
> >>
> >> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> >> index 3e60ec7d..1c710541 100644
> >> --- a/src/libcamera/formats.cpp
> >> +++ b/src/libcamera/formats.cpp
> >> @@ -231,6 +231,19 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> >>   		.pixelsPerGroup = 1,
> >>   		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> >>   	} },
> >> +	{ formats::RGBX8888, {
> >> +		.name = "RGBX8888",
> >> +		.format = formats::RGBX8888,
> >> +		.v4l2Formats = {
> >> +			.single = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
> > 
> > I think this should be BGRX32. Furthermore, you need to also update the
> 
> Thanks. Yes, it is obviously wrong, as there was another 
> V4L2_PIX_FMT_RGBX32 in the file already. I thought I checked that, but 
> apparently not...
> 
> I have to say I don't get the V4L2 formats. Why is
> 
> V4L2_PIX_FMT_XBGR32  BGRX-8-8-8-8
> 
> so the X is not beside the R in the define. But
> 
> V4L2_PIX_FMT_XRGB32  XRGB-8-8-8-8
> 
> X is beside the R. Well, doesn't matter.

I think those are called historical mistakes. They tend to appear here
and there. And again :-)

> > vpf2pf map in v4l2_pixelformat.cpp.
> 
> Ok.
Tomi Valkeinen May 19, 2022, 7:09 a.m. UTC | #4 
On 19/05/2022 10:07, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Thu, May 19, 2022 at 09:52:44AM +0300, Tomi Valkeinen wrote:
>> On 19/05/2022 02:20, Laurent Pinchart wrote:
>>> On Wed, May 18, 2022 at 03:20:14PM +0300, Tomi Valkeinen wrote:
>>>> Add missing RGBX8888 PixelFormatInfo.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>>> ---
>>>>    src/libcamera/formats.cpp | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
>>>> index 3e60ec7d..1c710541 100644
>>>> --- a/src/libcamera/formats.cpp
>>>> +++ b/src/libcamera/formats.cpp
>>>> @@ -231,6 +231,19 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
>>>>    		.pixelsPerGroup = 1,
>>>>    		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
>>>>    	} },
>>>> +	{ formats::RGBX8888, {
>>>> +		.name = "RGBX8888",
>>>> +		.format = formats::RGBX8888,
>>>> +		.v4l2Formats = {
>>>> +			.single = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
>>>
>>> I think this should be BGRX32. Furthermore, you need to also update the
>>
>> Thanks. Yes, it is obviously wrong, as there was another
>> V4L2_PIX_FMT_RGBX32 in the file already. I thought I checked that, but
>> apparently not...
>>
>> I have to say I don't get the V4L2 formats. Why is
>>
>> V4L2_PIX_FMT_XBGR32  BGRX-8-8-8-8
>>
>> so the X is not beside the R in the define. But
>>
>> V4L2_PIX_FMT_XRGB32  XRGB-8-8-8-8
>>
>> X is beside the R. Well, doesn't matter.
> 
> I think those are called historical mistakes. They tend to appear here
> and there. And again :-)

I guessed as much. Well, good that we don't make those. If only the 
other people would learn also.

  Tomi

Patch
diff mbox series 

diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
index 3e60ec7d..1c710541 100644
--- a/src/libcamera/formats.cpp
+++ b/src/libcamera/formats.cpp
@@ -231,6 +231,19 @@  const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
 		.pixelsPerGroup = 1,
 		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
 	} },
+	{ formats::RGBX8888, {
+		.name = "RGBX8888",
+		.format = formats::RGBX8888,
+		.v4l2Formats = {
+			.single = V4L2PixelFormat(V4L2_PIX_FMT_RGBX32),
+			.multi = V4L2PixelFormat(),
+		},
+		.bitsPerPixel = 32,
+		.colourEncoding = PixelFormatInfo::ColourEncodingRGB,
+		.packed = false,
+		.pixelsPerGroup = 1,
+		.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
+	} },
 	{ formats::BGRX8888, {
 		.name = "BGRX8888",
 		.format = formats::BGRX8888,