| Message ID | CAB0OVGp3Miwxcbaw54Rw1sOm4iQ=_OiyTf0TXcCfMY6StynhpA@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 15/09/17 13:09, Carl Eugen Hoyos wrote: > 2017-09-15 0:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >> On 14/09/17 22:30, Carl Eugen Hoyos wrote: >>> 2017-09-07 23:56 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>> >>>> +static const struct { >>>> + enum AVPixelFormat pixfmt; >>>> + uint32_t drm_format; >>>> +} kmsgrab_formats[] = { >>>> + { AV_PIX_FMT_GRAY8, DRM_FORMAT_R8 }, >>>> + { AV_PIX_FMT_GRAY16LE, DRM_FORMAT_R16 }, >>>> + { 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_YUYV422, DRM_FORMAT_YUYV }, >>>> + { AV_PIX_FMT_YVYU422, DRM_FORMAT_YVYU }, >>>> + { AV_PIX_FMT_UYVY422, DRM_FORMAT_UYVY }, >>>> + { AV_PIX_FMT_NV12, DRM_FORMAT_NV12 }, >>>> + { AV_PIX_FMT_YUV420P, DRM_FORMAT_YUV420 }, >>>> + { AV_PIX_FMT_YUV422P, DRM_FORMAT_YUV422 }, >>>> + { AV_PIX_FMT_YUV444P, DRM_FORMAT_YUV444 }, >>> >>> Which of those were you able to test? >> >> Only the 32-bit RGB0-type ones (all of them are testable because you just get the colours in a different order). > > So RGB0, BGR0, 0RGB and 0BGR all work fine? Yes. I've verified YUYV/UYVY directly on Intel as well now. >> Intel at least should work with others in near-future versions (e.g. <https://lists.freedesktop.org/archives/intel-gfx/2017-July/132642.html>), though I haven't actually tested this. It seemed sensible to include all core DRM formats which map to ffmpeg pixfmts to account for other drivers (possibly future) which I can't test now. > > Good idea, twelve more are attached. Looks sensible. I think the ordering of the packed-within-bytes formats (565, etc.) should be verified before applying them, though, just in case there is something funny going on here. I had a look at RGB565, which is usable for output on Intel, but I can't easily get the result anywhere (map fails, VAAPI doesn't like the format). On BIG_ENDIAN, I'm not sure whether it has any use or testing at all - none of the libdrm test programs allow it, and it is suspiciously absent from all but the most generic parts of drivers/gpu/drm in Linux. >> I've tested on amdgpu, exynos, i915 and rockchip. It should work on other KMS drivers, though if the output is tiled or not-CPU-mappable it can be hard to get the output somewhere where it can actually be used. (The ARM ones are fine and allow hwdownload directly. Intels I've tried are mappable but tiled so hwdownload is messed up, but they interoperate cleanly with VAAPI. The AMD I have makes the objects completely unmappable from the CPU, but they can be imported to other GPU things with Mesa.) >> >>> I find the comments in the header file very misleading: >>> What is "little-endian 8:8:8:8 ARGB"? >> >> It is just A-R-G-B in memory in that order as you might expect >> without the comment. > > So the comment is simply wrong for the 8:8:8:8 RGB formats? > Iirc, the same comment in the SDL sources defines another > order (or OpenGL, I don't remember atm), as does FFmpeg > through its RGB32 formats. Hmm. Maybe this is actually wrong in my code. The format is provided by the user, because there is no way to retrieve that information from the framebuffer itself, and therefore we are always doing the mapping in both directions - the default of AV_PIX_FMT_BGR0 is mapped to DRM_FORMAT_BGRX8888 and then back to AV_PIX_FMT_BGR0 for hwdownload or hwmap. If the sense were actually the opposite and the framebuffer was in fact DRM_FORMAT_XRGB8888, this would still work identically and have correct output, because the intermediate doesn't matter as long as it's reversible. I think I need to test this with an explicit program to do the modeset and set framebuffer formats directly and then match them to the output pixel values, because there is no other way to tell. Thanks, - Mark
2017-09-15 16:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: > On 15/09/17 13:09, Carl Eugen Hoyos wrote: >> 2017-09-15 0:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>> On 14/09/17 22:30, Carl Eugen Hoyos wrote: >>>> 2017-09-07 23:56 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>>> >>>>> +static const struct { >>>>> + enum AVPixelFormat pixfmt; >>>>> + uint32_t drm_format; >>>>> +} kmsgrab_formats[] = { >>>>> + { AV_PIX_FMT_GRAY8, DRM_FORMAT_R8 }, >>>>> + { AV_PIX_FMT_GRAY16LE, DRM_FORMAT_R16 }, >>>>> + { 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_YUYV422, DRM_FORMAT_YUYV }, >>>>> + { AV_PIX_FMT_YVYU422, DRM_FORMAT_YVYU }, >>>>> + { AV_PIX_FMT_UYVY422, DRM_FORMAT_UYVY }, >>>>> + { AV_PIX_FMT_NV12, DRM_FORMAT_NV12 }, >>>>> + { AV_PIX_FMT_YUV420P, DRM_FORMAT_YUV420 }, >>>>> + { AV_PIX_FMT_YUV422P, DRM_FORMAT_YUV422 }, >>>>> + { AV_PIX_FMT_YUV444P, DRM_FORMAT_YUV444 }, >>>> >>>> Which of those were you able to test? >>> >>> Only the 32-bit RGB0-type ones (all of them are testable because you just get the colours in a different order). >> >> So RGB0, BGR0, 0RGB and 0BGR all work fine? > > Yes. > > I've verified YUYV/UYVY directly on Intel as well now. > >>> Intel at least should work with others in near-future versions (e.g. <https://lists.freedesktop.org/archives/intel-gfx/2017-July/132642.html>), though I haven't actually tested this. It seemed sensible to include all core DRM formats which map to ffmpeg pixfmts to account for other drivers (possibly future) which I can't test now. >> Good idea, twelve more are attached. > > Looks sensible. May I commit? > I think the ordering of the packed-within-bytes formats (565, etc.) should be verified before applying them, though, just in case there is something funny going on here. I had a look at RGB565, which is usable for output on Intel, but I can't easily get the result anywhere (map fails, VAAPI doesn't like the format). > > On BIG_ENDIAN, I'm not sure whether it has any use or testing at all - none of the libdrm test programs allow it, and it is suspiciously absent from all but the most generic parts of drivers/gpu/drm in Linux. > >>> I've tested on amdgpu, exynos, i915 and rockchip. It should work on other KMS drivers, though if the output is tiled or not-CPU-mappable it can be hard to get the output somewhere where it can actually be used. (The ARM ones are fine and allow hwdownload directly. Intels I've tried are mappable but tiled so hwdownload is messed up, but they interoperate cleanly with VAAPI. The AMD I have makes the objects completely unmappable from the CPU, but they can be imported to other GPU things with Mesa.) >>> >>>> I find the comments in the header file very misleading: >>>> What is "little-endian 8:8:8:8 ARGB"? >>> >>> It is just A-R-G-B in memory in that order as you might expect >>> without the comment. >> >> So the comment is simply wrong for the 8:8:8:8 RGB formats? >> Iirc, the same comment in the SDL sources defines another >> order (or OpenGL, I don't remember atm), as does FFmpeg >> through its RGB32 formats. > > Hmm. Maybe this is actually wrong in my code. The format is provided by the user, because there is no way to retrieve that information from the framebuffer itself, and therefore we are always doing the mapping in both directions - the default of AV_PIX_FMT_BGR0 is mapped to DRM_FORMAT_BGRX8888 and then back to AV_PIX_FMT_BGR0 for hwdownload or hwmap. If the sense were actually the opposite and the framebuffer was in fact DRM_FORMAT_XRGB8888, this would still work identically and have correct output, because the intermediate doesn't matter as long as it's reversible. > > I think I need to test this with an explicit program to do the modeset and set framebuffer formats directly and then match them to the output pixel values, because there is no other way to tell. Thank you! Could we agree, just for the wording (you know that I am all for commit early), that you were - so far - not able to test any of above formats? Carl Eugen
On 15/09/17 16:11, Carl Eugen Hoyos wrote: > 2017-09-15 16:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >> On 15/09/17 13:09, Carl Eugen Hoyos wrote: >>> 2017-09-15 0:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>>> On 14/09/17 22:30, Carl Eugen Hoyos wrote: >>>>> 2017-09-07 23:56 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>>>> >>>>>> +static const struct { >>>>>> + enum AVPixelFormat pixfmt; >>>>>> + uint32_t drm_format; >>>>>> +} kmsgrab_formats[] = { >>>>>> + { AV_PIX_FMT_GRAY8, DRM_FORMAT_R8 }, >>>>>> + { AV_PIX_FMT_GRAY16LE, DRM_FORMAT_R16 }, >>>>>> + { 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_YUYV422, DRM_FORMAT_YUYV }, >>>>>> + { AV_PIX_FMT_YVYU422, DRM_FORMAT_YVYU }, >>>>>> + { AV_PIX_FMT_UYVY422, DRM_FORMAT_UYVY }, >>>>>> + { AV_PIX_FMT_NV12, DRM_FORMAT_NV12 }, >>>>>> + { AV_PIX_FMT_YUV420P, DRM_FORMAT_YUV420 }, >>>>>> + { AV_PIX_FMT_YUV422P, DRM_FORMAT_YUV422 }, >>>>>> + { AV_PIX_FMT_YUV444P, DRM_FORMAT_YUV444 }, >>>>> >>>>> Which of those were you able to test? >>>> >>>> Only the 32-bit RGB0-type ones (all of them are testable because you just get the colours in a different order). >>> >>> So RGB0, BGR0, 0RGB and 0BGR all work fine? >> >> Yes. >> >> I've verified YUYV/UYVY directly on Intel as well now. >> >>>> Intel at least should work with others in near-future versions (e.g. <https://lists.freedesktop.org/archives/intel-gfx/2017-July/132642.html>), though I haven't actually tested this. It seemed sensible to include all core DRM formats which map to ffmpeg pixfmts to account for other drivers (possibly future) which I can't test now. > >>> Good idea, twelve more are attached. >> >> Looks sensible. > > May I commit? Are you able to test at all? I believe I should be able to test at least 565 properly a bit later today (and settle the ordering question - I do think now that my interpretation is wrong and that it was working because the same method was used in both directions). >> I think the ordering of the packed-within-bytes formats (565, etc.) should be verified before applying them, though, just in case there is something funny going on here. I had a look at RGB565, which is usable for output on Intel, but I can't easily get the result anywhere (map fails, VAAPI doesn't like the format). >> >> On BIG_ENDIAN, I'm not sure whether it has any use or testing at all - none of the libdrm test programs allow it, and it is suspiciously absent from all but the most generic parts of drivers/gpu/drm in Linux. >> >>>> I've tested on amdgpu, exynos, i915 and rockchip. It should work on other KMS drivers, though if the output is tiled or not-CPU-mappable it can be hard to get the output somewhere where it can actually be used. (The ARM ones are fine and allow hwdownload directly. Intels I've tried are mappable but tiled so hwdownload is messed up, but they interoperate cleanly with VAAPI. The AMD I have makes the objects completely unmappable from the CPU, but they can be imported to other GPU things with Mesa.) >>>> >>>>> I find the comments in the header file very misleading: >>>>> What is "little-endian 8:8:8:8 ARGB"? >>>> >>>> It is just A-R-G-B in memory in that order as you might expect >>>> without the comment. >>> >>> So the comment is simply wrong for the 8:8:8:8 RGB formats? >>> Iirc, the same comment in the SDL sources defines another >>> order (or OpenGL, I don't remember atm), as does FFmpeg >>> through its RGB32 formats. >> >> Hmm. Maybe this is actually wrong in my code. The format is provided by the user, because there is no way to retrieve that information from the framebuffer itself, and therefore we are always doing the mapping in both directions - the default of AV_PIX_FMT_BGR0 is mapped to DRM_FORMAT_BGRX8888 and then back to AV_PIX_FMT_BGR0 for hwdownload or hwmap. If the sense were actually the opposite and the framebuffer was in fact DRM_FORMAT_XRGB8888, this would still work identically and have correct output, because the intermediate doesn't matter as long as it's reversible. >> >> I think I need to test this with an explicit program to do the modeset and set framebuffer formats directly and then match them to the output pixel values, because there is no other way to tell. > > Thank you! > > Could we agree, just for the wording (you know that I am all for > commit early), that you were - so far - not able to test any of > above formats? The ones in your list, no. The things I have actually tested are all permutations of RGBX (though not necessarily being able to distinguish between them), and then also YUYV and UYVY (4:2:2). - Mark
2017-09-15 18:19 GMT+02:00 Mark Thompson <sw@jkqxz.net>: > On 15/09/17 16:11, Carl Eugen Hoyos wrote: >> May I commit? > > Are you able to test at all? No, I am sorry if I gave a wrong impression! Carl Eugen
On 15/09/17 17:19, Mark Thompson wrote: > On 15/09/17 16:11, Carl Eugen Hoyos wrote: >> 2017-09-15 16:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>> On 15/09/17 13:09, Carl Eugen Hoyos wrote: >>>> 2017-09-15 0:37 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>>>> On 14/09/17 22:30, Carl Eugen Hoyos wrote: >>>>>> 2017-09-07 23:56 GMT+02:00 Mark Thompson <sw@jkqxz.net>: >>>>>> >>>>>>> +static const struct { >>>>>>> + enum AVPixelFormat pixfmt; >>>>>>> + uint32_t drm_format; >>>>>>> +} kmsgrab_formats[] = { >>>>>>> + { AV_PIX_FMT_GRAY8, DRM_FORMAT_R8 }, >>>>>>> + { AV_PIX_FMT_GRAY16LE, DRM_FORMAT_R16 }, >>>>>>> + { 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_YUYV422, DRM_FORMAT_YUYV }, >>>>>>> + { AV_PIX_FMT_YVYU422, DRM_FORMAT_YVYU }, >>>>>>> + { AV_PIX_FMT_UYVY422, DRM_FORMAT_UYVY }, >>>>>>> + { AV_PIX_FMT_NV12, DRM_FORMAT_NV12 }, >>>>>>> + { AV_PIX_FMT_YUV420P, DRM_FORMAT_YUV420 }, >>>>>>> + { AV_PIX_FMT_YUV422P, DRM_FORMAT_YUV422 }, >>>>>>> + { AV_PIX_FMT_YUV444P, DRM_FORMAT_YUV444 }, >>>>>> >>>>>> Which of those were you able to test? >>>>> >>>>> Only the 32-bit RGB0-type ones (all of them are testable because you just get the colours in a different order). >>>> >>>> So RGB0, BGR0, 0RGB and 0BGR all work fine? >>> >>> Yes. >>> >>> I've verified YUYV/UYVY directly on Intel as well now. >>> >>>>> Intel at least should work with others in near-future versions (e.g. <https://lists.freedesktop.org/archives/intel-gfx/2017-July/132642.html>), though I haven't actually tested this. It seemed sensible to include all core DRM formats which map to ffmpeg pixfmts to account for other drivers (possibly future) which I can't test now. >> >>>> Good idea, twelve more are attached. >>> >>> Looks sensible. >> >> May I commit? > > Are you able to test at all? I believe I should be able to test at least 565 properly a bit later today (and settle the ordering question - I do think now that my interpretation is wrong and that it was working because the same method was used in both directions). Ok, I've done a lot more format testing now, with explicitly created framebuffers on Rockchip: * The four-element RGB formats are indeed in the order stated in the file, not the order in the name. This means they are reversed wrt everything else in ffmpeg, so I'm fixing that. * RGB24 and BGR24 /are/ correct already, the endianness comments in drm_fourcc are misleading for them. * The multiple-plane formats are not usable, because the planes need not be contiguous (you get garbage in the chroma plane), so I'm going to remove support for them. There is currently no way to query the offsets of later planes, but if that were added then they could be supported. * RGB565 and BGR565 work correctly, the rest of formats in your patch are untested. Patches follow, including yours. Thanks, - Mark
diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c index 67a83ef..2b8affa 100644 --- a/libavdevice/kmsgrab.c +++ b/libavdevice/kmsgrab.c @@ -207,7 +207,17 @@ static const struct { #endif #ifdef DRM_FORMAT_R16 { AV_PIX_FMT_GRAY16LE, DRM_FORMAT_R16 }, + { AV_PIX_FMT_GRAY16BE, DRM_FORMAT_R16 | DRM_FORMAT_BIG_ENDIAN }, #endif + { AV_PIX_FMT_BGR8, DRM_FORMAT_BGR233 }, + { AV_PIX_FMT_RGB555LE, DRM_FORMAT_XRGB1555 }, + { AV_PIX_FMT_RGB555BE, DRM_FORMAT_XRGB1555 | DRM_FORMAT_BIG_ENDIAN }, + { AV_PIX_FMT_BGR555LE, DRM_FORMAT_XBGR1555 }, + { AV_PIX_FMT_BGR555BE, DRM_FORMAT_XBGR1555 | DRM_FORMAT_BIG_ENDIAN }, + { AV_PIX_FMT_RGB565LE, DRM_FORMAT_RGB565 }, + { AV_PIX_FMT_RGB565BE, DRM_FORMAT_RGB565 | DRM_FORMAT_BIG_ENDIAN }, + { AV_PIX_FMT_BGR565LE, DRM_FORMAT_BGR565 }, + { AV_PIX_FMT_BGR565BE, DRM_FORMAT_BGR565 | DRM_FORMAT_BIG_ENDIAN }, { AV_PIX_FMT_RGB24, DRM_FORMAT_RGB888 }, { AV_PIX_FMT_BGR24, DRM_FORMAT_BGR888 }, { AV_PIX_FMT_0RGB, DRM_FORMAT_XRGB8888 }, @@ -222,6 +232,8 @@ static const struct { { AV_PIX_FMT_YVYU422, DRM_FORMAT_YVYU }, { AV_PIX_FMT_UYVY422, DRM_FORMAT_UYVY }, { AV_PIX_FMT_NV12, DRM_FORMAT_NV12 }, + { AV_PIX_FMT_YUV410P, DRM_FORMAT_YUV410 }, + { AV_PIX_FMT_YUV411P, DRM_FORMAT_YUV411 }, { AV_PIX_FMT_YUV420P, DRM_FORMAT_YUV420 }, { AV_PIX_FMT_YUV422P, DRM_FORMAT_YUV422 }, { AV_PIX_FMT_YUV444P, DRM_FORMAT_YUV444 },