diff mbox

[FFmpeg-devel,1/4] kmsgrab: Fix 32-bit RGB DRM format definitions

Message ID 20170915205108.14049-1-sw@jkqxz.net
State Accepted
Commit 22afa87a8e90c83d736400bf8fd6bb19152defaf
Headers show

Commit Message

Mark Thompson Sept. 15, 2017, 8:51 p.m. UTC 
The 32-bit DRM formats are defined in terms of little-endian words, so
32-bit RGB formats like XRGB actually have the elements in the opposite
order in memory to the order they are in the name.

This does not affect YUYV and similar YUV 4:2:2 formats, which are in
the expected order.
---
 libavdevice/kmsgrab.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Carl Eugen Hoyos Sept. 15, 2017, 9:40 p.m. UTC | #1 
2017-09-15 22:51 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
> The 32-bit DRM formats are defined in terms of little-endian words, so
> 32-bit RGB formats like XRGB actually have the elements in the opposite
> order in memory to the order they are in the name.
>
> This does not affect YUYV and similar YUV 4:2:2 formats, which are in
> the expected order.
> ---
>  libavdevice/kmsgrab.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
> index 67a83ef84a..bcb6865f60 100644
> --- a/libavdevice/kmsgrab.c
> +++ b/libavdevice/kmsgrab.c
> @@ -210,14 +210,14 @@ static const struct {
>  #endif
>      { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
>      { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
> -    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
> -    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
> -    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
> -    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
> -    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
> -    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
> -    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
> -    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
> +    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
> +    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
> +    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
> +    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
> +    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
> +    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
> +    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
> +    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },

Is it possible to compile kmsgrab on big-endian hardware?

Thank you, Carl Eugen
Mark Thompson Sept. 15, 2017, 9:44 p.m. UTC | #2 
On 15/09/17 22:40, Carl Eugen Hoyos wrote:
> 2017-09-15 22:51 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>> The 32-bit DRM formats are defined in terms of little-endian words, so
>> 32-bit RGB formats like XRGB actually have the elements in the opposite
>> order in memory to the order they are in the name.
>>
>> This does not affect YUYV and similar YUV 4:2:2 formats, which are in
>> the expected order.
>> ---
>>  libavdevice/kmsgrab.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
>> index 67a83ef84a..bcb6865f60 100644
>> --- a/libavdevice/kmsgrab.c
>> +++ b/libavdevice/kmsgrab.c
>> @@ -210,14 +210,14 @@ static const struct {
>>  #endif
>>      { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
>>      { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
>> -    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
>> -    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
>> -    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
>> -    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
>> -    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
>> -    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
>> -    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
>> -    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
>> +    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
>> +    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
>> +    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
>> +    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
>> +    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
>> +    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
>> +    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
>> +    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },
> 
> Is it possible to compile kmsgrab on big-endian hardware?

Yes.

- Mark
Carl Eugen Hoyos Sept. 15, 2017, 9:49 p.m. UTC | #3 
2017-09-15 23:44 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
> On 15/09/17 22:40, Carl Eugen Hoyos wrote:
>> 2017-09-15 22:51 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>>> The 32-bit DRM formats are defined in terms of little-endian words, so
>>> 32-bit RGB formats like XRGB actually have the elements in the opposite
>>> order in memory to the order they are in the name.
>>>
>>> This does not affect YUYV and similar YUV 4:2:2 formats, which are in
>>> the expected order.
>>> ---
>>>  libavdevice/kmsgrab.c | 16 ++++++++--------
>>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
>>> index 67a83ef84a..bcb6865f60 100644
>>> --- a/libavdevice/kmsgrab.c
>>> +++ b/libavdevice/kmsgrab.c
>>> @@ -210,14 +210,14 @@ static const struct {
>>>  #endif
>>>      { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
>>>      { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
>>> -    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
>>> -    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
>>> -    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
>>> -    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
>>> -    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
>>> -    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
>>> -    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
>>> -    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
>>> +    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
>>> +    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
>>> +    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
>>> +    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
>>> +    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
>>> +    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
>>> +    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
>>> +    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },
>>
>> Is it possible to compile kmsgrab on big-endian hardware?
>
> Yes.

And you assume that on big-endian hardware, above formats
would still be little-endian?

Or that they would not be used at all and the big-endian bit would
be set?
In that case, it may be simpler to mask the most significant
bit away in the code and use the endianness-aware formats
for FFmpeg in the table above (it would reduce the table size
and make it more readable). Especially as we don't know if
the bit would also be set for the packed yuv formats.

Carl Eugen
Mark Thompson Sept. 15, 2017, 10:03 p.m. UTC | #4 
On 15/09/17 22:49, Carl Eugen Hoyos wrote:
> 2017-09-15 23:44 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>> On 15/09/17 22:40, Carl Eugen Hoyos wrote:
>>> 2017-09-15 22:51 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>>>> The 32-bit DRM formats are defined in terms of little-endian words, so
>>>> 32-bit RGB formats like XRGB actually have the elements in the opposite
>>>> order in memory to the order they are in the name.
>>>>
>>>> This does not affect YUYV and similar YUV 4:2:2 formats, which are in
>>>> the expected order.
>>>> ---
>>>>  libavdevice/kmsgrab.c | 16 ++++++++--------
>>>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
>>>> index 67a83ef84a..bcb6865f60 100644
>>>> --- a/libavdevice/kmsgrab.c
>>>> +++ b/libavdevice/kmsgrab.c
>>>> @@ -210,14 +210,14 @@ static const struct {
>>>>  #endif
>>>>      { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
>>>>      { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
>>>> -    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
>>>> -    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
>>>> -    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
>>>> -    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
>>>> -    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
>>>> -    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
>>>> -    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
>>>> -    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
>>>> +    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
>>>> +    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
>>>> +    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
>>>> +    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
>>>> +    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
>>>> +    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
>>>> +    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
>>>> +    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },
>>>
>>> Is it possible to compile kmsgrab on big-endian hardware?
>>
>> Yes.
> 
> And you assume that on big-endian hardware, above formats
> would still be little-endian?
> 
> Or that they would not be used at all and the big-endian bit would
> be set?
> In that case, it may be simpler to mask the most significant
> bit away in the code and use the endianness-aware formats
> for FFmpeg in the table above (it would reduce the table size
> and make it more readable). Especially as we don't know if
> the bit would also be set for the packed yuv formats.

I think that unlike with RGB565 and similar, the big-endian bit does not need to be set for any of these formats, because the opposite-endian form already exists for each one.  That is, you can always use DRM_FORMAT_XBGR8888 when the order of bytes in memory is RGBX, regardless of the host endianness.

(I think that's a consistent interpretation?)

- Mark
Carl Eugen Hoyos Sept. 15, 2017, 10:19 p.m. UTC | #5 
2017-09-16 0:03 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
> On 15/09/17 22:49, Carl Eugen Hoyos wrote:
>> 2017-09-15 23:44 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>>> On 15/09/17 22:40, Carl Eugen Hoyos wrote:
>>>> 2017-09-15 22:51 GMT+02:00 Mark Thompson <sw@jkqxz.net>:
>>>>> The 32-bit DRM formats are defined in terms of little-endian words, so
>>>>> 32-bit RGB formats like XRGB actually have the elements in the opposite
>>>>> order in memory to the order they are in the name.
>>>>>
>>>>> This does not affect YUYV and similar YUV 4:2:2 formats, which are in
>>>>> the expected order.
>>>>> ---
>>>>>  libavdevice/kmsgrab.c | 16 ++++++++--------
>>>>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
>>>>> index 67a83ef84a..bcb6865f60 100644
>>>>> --- a/libavdevice/kmsgrab.c
>>>>> +++ b/libavdevice/kmsgrab.c
>>>>> @@ -210,14 +210,14 @@ static const struct {
>>>>>  #endif
>>>>>      { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
>>>>>      { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
>>>>> -    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
>>>>> -    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
>>>>> -    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
>>>>> -    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
>>>>> -    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
>>>>> -    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
>>>>> -    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
>>>>> -    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
>>>>> +    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
>>>>> +    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
>>>>> +    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
>>>>> +    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
>>>>> +    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
>>>>> +    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
>>>>> +    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
>>>>> +    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },
>>>>
>>>> Is it possible to compile kmsgrab on big-endian hardware?
>>>
>>> Yes.
>>
>> And you assume that on big-endian hardware, above formats
>> would still be little-endian?
>>
>> Or that they would not be used at all and the big-endian bit would
>> be set?
>> In that case, it may be simpler to mask the most significant
>> bit away in the code and use the endianness-aware formats
>> for FFmpeg in the table above (it would reduce the table size
>> and make it more readable). Especially as we don't know if
>> the bit would also be set for the packed yuv formats.
>
> I think that unlike with RGB565 and similar, the big-endian bit does not need to be set for any of these formats, because the opposite-endian form already exists for each one.  That is, you can always use DRM_FORMAT_XBGR8888 when the order of bytes in memory is RGBX, regardless of the host endianness.
>
> (I think that's a consistent interpretation?)

If that is what the comments in the header file mean, it is
probably consistent.

Thank you, Carl Eugen
diff mbox

Patch

diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index 67a83ef84a..bcb6865f60 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -210,14 +210,14 @@  static const struct {
 #endif
     { AV_PIX_FMT_RGB24,    DRM_FORMAT_RGB888   },
     { AV_PIX_FMT_BGR24,    DRM_FORMAT_BGR888   },
-    { AV_PIX_FMT_0RGB,     DRM_FORMAT_XRGB8888 },
-    { AV_PIX_FMT_0BGR,     DRM_FORMAT_XBGR8888 },
-    { AV_PIX_FMT_RGB0,     DRM_FORMAT_RGBX8888 },
-    { AV_PIX_FMT_BGR0,     DRM_FORMAT_BGRX8888 },
-    { AV_PIX_FMT_ARGB,     DRM_FORMAT_ARGB8888 },
-    { AV_PIX_FMT_ABGR,     DRM_FORMAT_ABGR8888 },
-    { AV_PIX_FMT_RGBA,     DRM_FORMAT_RGBA8888 },
-    { AV_PIX_FMT_BGRA,     DRM_FORMAT_BGRA8888 },
+    { AV_PIX_FMT_0RGB,     DRM_FORMAT_BGRX8888 },
+    { AV_PIX_FMT_0BGR,     DRM_FORMAT_RGBX8888 },
+    { AV_PIX_FMT_RGB0,     DRM_FORMAT_XBGR8888 },
+    { AV_PIX_FMT_BGR0,     DRM_FORMAT_XRGB8888 },
+    { AV_PIX_FMT_ARGB,     DRM_FORMAT_BGRA8888 },
+    { AV_PIX_FMT_ABGR,     DRM_FORMAT_RGBA8888 },
+    { AV_PIX_FMT_RGBA,     DRM_FORMAT_ABGR8888 },
+    { AV_PIX_FMT_BGRA,     DRM_FORMAT_ARGB8888 },
     { AV_PIX_FMT_YUYV422,  DRM_FORMAT_YUYV     },
     { AV_PIX_FMT_YVYU422,  DRM_FORMAT_YVYU     },
     { AV_PIX_FMT_UYVY422,  DRM_FORMAT_UYVY     },