[FFmpeg-devel] [PATCH 5/5] lavd: Add KMS frame grabber

[Carl Eugen Hoyos]

2017-09-15 16:37 GMT+02:00 Mark Thompson <sw at jkqxz.net>:
> On 15/09/17 13:09, Carl Eugen Hoyos wrote:
>> 2017-09-15 0:37 GMT+02:00 Mark Thompson <sw at jkqxz.net>:
>>> On 14/09/17 22:30, Carl Eugen Hoyos wrote:
>>>> 2017-09-07 23:56 GMT+02:00 Mark Thompson <sw at 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