diff mbox

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

Message ID 20170907215619.3182-5-sw@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson Sept. 7, 2017, 9:56 p.m. UTC
---
Now sets the trusted packet flag; otherwise unchanged.


 configure                |   1 +
 libavdevice/Makefile     |   1 +
 libavdevice/alldevices.c |   1 +
 libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 458 insertions(+)
 create mode 100644 libavdevice/kmsgrab.c

Comments

Mark Thompson Sept. 11, 2017, 10:09 p.m. UTC | #1
On 07/09/17 22:56, Mark Thompson wrote:
> ---
> Now sets the trusted packet flag; otherwise unchanged.
> 
> 
>  configure                |   1 +
>  libavdevice/Makefile     |   1 +
>  libavdevice/alldevices.c |   1 +
>  libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 458 insertions(+)
>  create mode 100644 libavdevice/kmsgrab.c

Series approved by atomnuker on IRC.  Does anyone else want to comment on the wrapped_avframe / AV_PKT_FLAG_TRUSTED setup (or anything else)?  If not, I'll push this the day after tomorrow.

Thanks,

- Mark
Paul B Mahol Sept. 11, 2017, 10:15 p.m. UTC | #2
On 9/12/17, Mark Thompson <sw@jkqxz.net> wrote:
> On 07/09/17 22:56, Mark Thompson wrote:
>> ---
>> Now sets the trusted packet flag; otherwise unchanged.
>>
>>
>>  configure                |   1 +
>>  libavdevice/Makefile     |   1 +
>>  libavdevice/alldevices.c |   1 +
>>  libavdevice/kmsgrab.c    | 455
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 458 insertions(+)
>>  create mode 100644 libavdevice/kmsgrab.c
>
> Series approved by atomnuker on IRC.  Does anyone else want to comment on
> the wrapped_avframe / AV_PKT_FLAG_TRUSTED setup (or anything else)?  If not,
> I'll push this the day after tomorrow.

Missing docs.
James Almer Sept. 14, 2017, 4:35 a.m. UTC | #3
On 9/7/2017 6:56 PM, Mark Thompson wrote:
> +    pkt->data  = (uint8_t*)frame;
> +    pkt->size  = sizeof(*frame);
> +    pkt->pts   = now;
> +    pkt->flags = AV_PKT_FLAG_TRUSTED;

pkt->flags |= AV_PKT_FLAG_TRUSTED;

I know pkt->flags is zeroed by default, but that may change in the
future. And ORing/XORing is the proper way to set flags anyway.
Mark Thompson Sept. 14, 2017, 8:27 a.m. UTC | #4
On 14/09/17 05:35, James Almer wrote:
> On 9/7/2017 6:56 PM, Mark Thompson wrote:
>> +    pkt->data  = (uint8_t*)frame;
>> +    pkt->size  = sizeof(*frame);
>> +    pkt->pts   = now;
>> +    pkt->flags = AV_PKT_FLAG_TRUSTED;
> 
> pkt->flags |= AV_PKT_FLAG_TRUSTED;
> 
> I know pkt->flags is zeroed by default, but that may change in the
> future. And ORing/XORing is the proper way to set flags anyway.

Fixed.

Thanks!

- Mark
Carl Eugen Hoyos Sept. 14, 2017, 9:30 p.m. UTC | #5
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?
I find the comments in the header file very misleading:
What is "little-endian 8:8:8:8 ARGB"?

Thank you, Carl Eugen
Mark Thompson Sept. 14, 2017, 10:37 p.m. UTC | #6
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).  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.

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.  Not sure why it says little-endian - maybe to emphasise that it doesn't change based on the host architecture?

Thanks,

- Mark
Andy Furniss Sept. 14, 2017, 11:08 p.m. UTC | #7
Mark Thompson wrote:
> ---
> Now sets the trusted packet flag; otherwise unchanged.
> 
> 
>   configure                |   1 +
>   libavdevice/Makefile     |   1 +
>   libavdevice/alldevices.c |   1 +
>   libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 458 insertions(+)
>   create mode 100644 libavdevice/kmsgrab.c
> 
> diff --git a/configure b/configure
> index 6581c53c1a..76a7591ceb 100755
> --- a/configure
> +++ b/configure
> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>   iec61883_indev_deps="libiec61883"
>   jack_indev_deps="jack"
>   jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
> +kmsgrab_indev_deps="libdrm"

Doesn't get built for me = doesn't show up as indev after configure 
anything special needed?
Andy Furniss Sept. 14, 2017, 11:15 p.m. UTC | #8
Andy Furniss wrote:
> Mark Thompson wrote:
>> ---
>> Now sets the trusted packet flag; otherwise unchanged.
>>
>>
>>   configure                |   1 +
>>   libavdevice/Makefile     |   1 +
>>   libavdevice/alldevices.c |   1 +
>>   libavdevice/kmsgrab.c    | 455 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 458 insertions(+)
>>   create mode 100644 libavdevice/kmsgrab.c
>>
>> diff --git a/configure b/configure
>> index 6581c53c1a..76a7591ceb 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>>   iec61883_indev_deps="libiec61883"
>>   jack_indev_deps="jack"
>>   jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
>> +kmsgrab_indev_deps="libdrm"
> 
> Doesn't get built for me = doesn't show up as indev after configure 
> anything special needed?

Never mind I found --enable-libdrm (had tried --enable-kmsgrab)
Mark Thompson Sept. 14, 2017, 11:59 p.m. UTC | #9
On 15/09/17 00:15, Andy Furniss wrote:
> Andy Furniss wrote:
>> Mark Thompson wrote:
>>> ---
>>> Now sets the trusted packet flag; otherwise unchanged.
>>>
>>>
>>>   configure                |   1 +
>>>   libavdevice/Makefile     |   1 +
>>>   libavdevice/alldevices.c |   1 +
>>>   libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 458 insertions(+)
>>>   create mode 100644 libavdevice/kmsgrab.c
>>>
>>> diff --git a/configure b/configure
>>> index 6581c53c1a..76a7591ceb 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>>>   iec61883_indev_deps="libiec61883"
>>>   jack_indev_deps="jack"
>>>   jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
>>> +kmsgrab_indev_deps="libdrm"
>>
>> Doesn't get built for me = doesn't show up as indev after configure anything special needed?
> 
> Never mind I found --enable-libdrm (had tried --enable-kmsgrab)

I assume you're going to try AMD + Mesa + VAAPI.

VAAPI_DISABLE_INTERLACE=1 ./ffmpeg_g -y -format bgr0 -device /dev/dri/card1 -f kmsgrab -i - -vsync 0 -init_hw_device vaapi=v:/dev/dri/renderD129 -filter_hw_device v -vf 'hwmap,scale_vaapi=w=1920:h=1080:format=nv12' -c:v h264_vaapi -profile 578 -bf 0 out.mp4

Three Mesa issues I had to get around:

* Device derivation doesn't work because the Mesa driver doesn't want to initialise on the DRM master device for some reason; making the matching device separately does work.
* Against current git, you need to reapply the VAAPI_DISABLE_INTERLACE patch and use it - if not, the colour conversion just barfs because it wants interlaced surfaces.
* The postproc scaler seems to only write the luma plane when converting from RGB - this is also visible when uploading normal RGB images, so just a bug somewhere.

With that, it works to record the screen in greyscale...

I have some other Mesa stuff to do queued up (libva2 with VAAPI export for EGL import on AMD), so I'll pursue these further soonish.

- Mark
Andy Furniss Sept. 15, 2017, 9:26 a.m. UTC | #10
Mark Thompson wrote:
> On 15/09/17 00:15, Andy Furniss wrote:
>> Andy Furniss wrote:
>>> Mark Thompson wrote:
>>>> ---
>>>> Now sets the trusted packet flag; otherwise unchanged.
>>>>
>>>>
>>>>    configure                |   1 +
>>>>    libavdevice/Makefile     |   1 +
>>>>    libavdevice/alldevices.c |   1 +
>>>>    libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 458 insertions(+)
>>>>    create mode 100644 libavdevice/kmsgrab.c
>>>>
>>>> diff --git a/configure b/configure
>>>> index 6581c53c1a..76a7591ceb 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>>>>    iec61883_indev_deps="libiec61883"
>>>>    jack_indev_deps="jack"
>>>>    jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
>>>> +kmsgrab_indev_deps="libdrm"
>>>
>>> Doesn't get built for me = doesn't show up as indev after configure anything special needed?
>>
>> Never mind I found --enable-libdrm (had tried --enable-kmsgrab)
> 
> I assume you're going to try AMD + Mesa + VAAPI.
> 
> VAAPI_DISABLE_INTERLACE=1 ./ffmpeg_g -y -format bgr0 -device /dev/dri/card1 -f kmsgrab -i - -vsync 0 -init_hw_device vaapi=v:/dev/dri/renderD129 -filter_hw_device v -vf 'hwmap,scale_vaapi=w=1920:h=1080:format=nv12' -c:v h264_vaapi -profile 578 -bf 0 out.mp4
> 
> Three Mesa issues I had to get around:
> 
> * Device derivation doesn't work because the Mesa driver doesn't want to initialise on the DRM master device for some reason; making the matching device separately does work.
> * Against current git, you need to reapply the VAAPI_DISABLE_INTERLACE patch and use it - if not, the colour conversion just barfs because it wants interlaced surfaces.
> * The postproc scaler seems to only write the luma plane when converting from RGB - this is also visible when uploading normal RGB images, so just a bug somewhere.
> 
> With that, it works to record the screen in greyscale...
> 
> I have some other Mesa stuff to do queued up (libva2 with VAAPI export for EGL import on AMD), so I'll pursue these further soonish.

Thanks for the info, as it happens I am running some testing patches 
from Leo currently that fix up postproc deinterlace now the env is gone.

I'll point Leo at this mail and test this evening.


> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Andy Furniss Sept. 19, 2017, 9:21 p.m. UTC | #11
Mark Thompson wrote:
> On 15/09/17 00:15, Andy Furniss wrote:
>> Andy Furniss wrote:
>>> Mark Thompson wrote:
>>>> ---
>>>> Now sets the trusted packet flag; otherwise unchanged.
>>>>
>>>>
>>>>    configure                |   1 +
>>>>    libavdevice/Makefile     |   1 +
>>>>    libavdevice/alldevices.c |   1 +
>>>>    libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 458 insertions(+)
>>>>    create mode 100644 libavdevice/kmsgrab.c
>>>>
>>>> diff --git a/configure b/configure
>>>> index 6581c53c1a..76a7591ceb 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>>>>    iec61883_indev_deps="libiec61883"
>>>>    jack_indev_deps="jack"
>>>>    jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
>>>> +kmsgrab_indev_deps="libdrm"
>>>
>>> Doesn't get built for me = doesn't show up as indev after configure anything special needed?
>>
>> Never mind I found --enable-libdrm (had tried --enable-kmsgrab)
> 
> I assume you're going to try AMD + Mesa + VAAPI.
> 
> VAAPI_DISABLE_INTERLACE=1 ./ffmpeg_g -y -format bgr0 -device /dev/dri/card1 -f kmsgrab -i - -vsync 0 -init_hw_device vaapi=v:/dev/dri/renderD129 -filter_hw_device v -vf 'hwmap,scale_vaapi=w=1920:h=1080:format=nv12' -c:v h264_vaapi -profile 578 -bf 0 out.mp4
> 
> Three Mesa issues I had to get around:
> 
> * Device derivation doesn't work because the Mesa driver doesn't want to initialise on the DRM master device for some reason; making the matching device separately does work.
> * Against current git, you need to reapply the VAAPI_DISABLE_INTERLACE patch and use it - if not, the colour conversion just barfs because it wants interlaced surfaces.
> * The postproc scaler seems to only write the luma plane when converting from RGB - this is also visible when uploading normal RGB images, so just a bug somewhere.
> 
> With that, it works to record the screen in greyscale...
> 
> I have some other Mesa stuff to do queued up (libva2 with VAAPI export for EGL import on AMD), so I'll pursue these further soonish.

Leo just posted a patchset on the mesa list that makes vaapi work better 
with this and postproc generally.

You don't need the env with those and kmsgrab is now working for me - up 
to a point ...

That point being around 7k frames it will run out of something.

[AVHWFramesContext @ 0x31ed880] Failed to create surface from DRM 
object: 2 (resource allocation failed).
[Parsed_hwmap_0 @ 0x3114c40] Failed to map frame: -5.

I see that memory is reducing before this although I still have spare - 
is this the same issue you explained on users WRT leaking on decode?
Mark Thompson Sept. 20, 2017, 12:14 a.m. UTC | #12
On 19/09/17 22:21, Andy Furniss wrote:
> Mark Thompson wrote:
>> On 15/09/17 00:15, Andy Furniss wrote:
>>> Andy Furniss wrote:
>>>> Mark Thompson wrote:
>>>>> ---
>>>>> Now sets the trusted packet flag; otherwise unchanged.
>>>>>
>>>>>
>>>>>    configure                |   1 +
>>>>>    libavdevice/Makefile     |   1 +
>>>>>    libavdevice/alldevices.c |   1 +
>>>>>    libavdevice/kmsgrab.c    | 455 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>>    4 files changed, 458 insertions(+)
>>>>>    create mode 100644 libavdevice/kmsgrab.c
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 6581c53c1a..76a7591ceb 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -3040,6 +3040,7 @@ gdigrab_indev_select="bmp_decoder"
>>>>>    iec61883_indev_deps="libiec61883"
>>>>>    jack_indev_deps="jack"
>>>>>    jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
>>>>> +kmsgrab_indev_deps="libdrm"
>>>>
>>>> Doesn't get built for me = doesn't show up as indev after configure anything special needed?
>>>
>>> Never mind I found --enable-libdrm (had tried --enable-kmsgrab)
>>
>> I assume you're going to try AMD + Mesa + VAAPI.
>>
>> VAAPI_DISABLE_INTERLACE=1 ./ffmpeg_g -y -format bgr0 -device /dev/dri/card1 -f kmsgrab -i - -vsync 0 -init_hw_device vaapi=v:/dev/dri/renderD129 -filter_hw_device v -vf 'hwmap,scale_vaapi=w=1920:h=1080:format=nv12' -c:v h264_vaapi -profile 578 -bf 0 out.mp4
>>
>> Three Mesa issues I had to get around:
>>
>> * Device derivation doesn't work because the Mesa driver doesn't want to initialise on the DRM master device for some reason; making the matching device separately does work.
>> * Against current git, you need to reapply the VAAPI_DISABLE_INTERLACE patch and use it - if not, the colour conversion just barfs because it wants interlaced surfaces.
>> * The postproc scaler seems to only write the luma plane when converting from RGB - this is also visible when uploading normal RGB images, so just a bug somewhere.
>>
>> With that, it works to record the screen in greyscale...
>>
>> I have some other Mesa stuff to do queued up (libva2 with VAAPI export for EGL import on AMD), so I'll pursue these further soonish.
> 
> Leo just posted a patchset on the mesa list that makes vaapi work better with this and postproc generally.
> 
> You don't need the env with those and kmsgrab is now working for me - up to a point ...

Yep, that works for me now too - with colour!  (The export to GL for playback is still messed up by interlacing without patching it out, but that looked orthogonal anyway.)

> That point being around 7k frames it will run out of something.
> 
> [AVHWFramesContext @ 0x31ed880] Failed to create surface from DRM object: 2 (resource allocation failed).
> [Parsed_hwmap_0 @ 0x3114c40] Failed to map frame: -5.
> 
> I see that memory is reducing before this although I still have spare - is this the same issue you explained on users WRT leaking on decode?

Yeah, I also run out of ... something ... at around 7200 frames.  It's not fds or memory.  I don't think it's the buffer problem (which, incidentally, should finally be fixable sensibly in libva2 soon), because that ended up manifesting as leaking memory.  It's also not a problem for Intel (I've already been running that for a long time to test).  Maybe some other sort of handle on the Mesa side?  I'll investigate further tomorrow.

Thanks,

- Mark
Andy Furniss Sept. 20, 2017, 4:10 p.m. UTC | #13
Mark Thompson wrote:
> On 19/09/17 22:21, Andy Furniss wrote:

>> That point being around 7k frames it will run out of something.
>>
>> [AVHWFramesContext @ 0x31ed880] Failed to create surface from DRM object: 2 (resource allocation failed).
>> [Parsed_hwmap_0 @ 0x3114c40] Failed to map frame: -5.
>>
>> I see that memory is reducing before this although I still have spare - is this the same issue you explained on users WRT leaking on decode?
> 
> Yeah, I also run out of ... something ... at around 7200 frames.  It's not fds or memory.  I don't think it's the buffer problem (which, incidentally, should finally be fixable sensibly in libva2 soon), because that ended up manifesting as leaking memory.  It's also not a problem for Intel (I've already been running that for a long time to test).  Maybe some other sort of handle on the Mesa side?  I'll investigate further tomorrow.

Leo has fixed the leak.
Mark Thompson Sept. 20, 2017, 9:07 p.m. UTC | #14
On 20/09/17 17:10, Andy Furniss wrote:
> Mark Thompson wrote:
>> On 19/09/17 22:21, Andy Furniss wrote:
> 
>>> That point being around 7k frames it will run out of something.
>>>
>>> [AVHWFramesContext @ 0x31ed880] Failed to create surface from DRM object: 2 (resource allocation failed).
>>> [Parsed_hwmap_0 @ 0x3114c40] Failed to map frame: -5.
>>>
>>> I see that memory is reducing before this although I still have spare - is this the same issue you explained on users WRT leaking on decode?
>>
>> Yeah, I also run out of ... something ... at around 7200 frames.  It's not fds or memory.  I don't think it's the buffer problem (which, incidentally, should finally be fixable sensibly in libva2 soon), because that ended up manifesting as leaking memory.  It's also not a problem for Intel (I've already been running that for a long time to test).  Maybe some other sort of handle on the Mesa side?  I'll investigate further tomorrow.
> 
> Leo has fixed the leak.

Yep, checked with the updated Mesa postproc patches + libva2 fixes and it all looks good now.  (And colours are even correct, yay!  I still need to look into why the default comes out wrong with the postproc bits for that on Intel...)

Thanks,

- Mark
Andy Furniss Sept. 24, 2017, 10:08 a.m. UTC | #15
Mark Thompson wrote:
> On 20/09/17 17:10, Andy Furniss wrote:
>> Mark Thompson wrote:
>>> On 19/09/17 22:21, Andy Furniss wrote:
>>
>>>> That point being around 7k frames it will run out of something.
>>>>
>>>> [AVHWFramesContext @ 0x31ed880] Failed to create surface from DRM object: 2 (resource allocation failed).
>>>> [Parsed_hwmap_0 @ 0x3114c40] Failed to map frame: -5.
>>>>
>>>> I see that memory is reducing before this although I still have spare - is this the same issue you explained on users WRT leaking on decode?
>>>
>>> Yeah, I also run out of ... something ... at around 7200 frames.  It's not fds or memory.  I don't think it's the buffer problem (which, incidentally, should finally be fixable sensibly in libva2 soon), because that ended up manifesting as leaking memory.  It's also not a problem for Intel (I've already been running that for a long time to test).  Maybe some other sort of handle on the Mesa side?  I'll investigate further tomorrow.
>>
>> Leo has fixed the leak.
> 
> Yep, checked with the updated Mesa postproc patches + libva2 fixes and it all looks good now.  (And colours are even correct, yay!  I still need to look into why the default comes out wrong with the postproc bits for that on Intel...)

One thing that comes out in testing the patches WRT postproc is that for 
deinterlace 1080i25 -> 1080p50 there is an issue with the surface size 
being 1088.
This means the result gets scaled to 1088. It is possible to put scale 
after to get 1080, but this seems sub-optimal, costs about 8% perf and 
hypothetical quality issue (not that I could see it).

Apparently vaapi_scale does things differently and the driver is set for 
that way, so changing in the driver will break scale.

AFAICT this is orthogonal to the 1080/1088 encode issue discussed long 
ago where ffmpeg is doing the right thing, that should be fixable in the 
driver.

This case can be demonstrated with 1920x1080 nv12 hwupload -> deint -> 
hwdownload, so no chance for h26x cropping info being used.
diff mbox

Patch

diff --git a/configure b/configure
index 6581c53c1a..76a7591ceb 100755
--- a/configure
+++ b/configure
@@ -3040,6 +3040,7 @@  gdigrab_indev_select="bmp_decoder"
 iec61883_indev_deps="libiec61883"
 jack_indev_deps="jack"
 jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
+kmsgrab_indev_deps="libdrm"
 lavfi_indev_deps="avfilter"
 libcdio_indev_deps="libcdio"
 libdc1394_indev_deps="libdc1394"
diff --git a/libavdevice/Makefile b/libavdevice/Makefile
index 2a27d20388..f40f4d5298 100644
--- a/libavdevice/Makefile
+++ b/libavdevice/Makefile
@@ -32,6 +32,7 @@  OBJS-$(CONFIG_FBDEV_OUTDEV)              += fbdev_enc.o \
 OBJS-$(CONFIG_GDIGRAB_INDEV)             += gdigrab.o
 OBJS-$(CONFIG_IEC61883_INDEV)            += iec61883.o
 OBJS-$(CONFIG_JACK_INDEV)                += jack.o timefilter.o
+OBJS-$(CONFIG_KMSGRAB_INDEV)             += kmsgrab.o
 OBJS-$(CONFIG_LAVFI_INDEV)               += lavfi.o
 OBJS-$(CONFIG_OPENAL_INDEV)              += openal-dec.o
 OBJS-$(CONFIG_OPENGL_OUTDEV)             += opengl_enc.o
diff --git a/libavdevice/alldevices.c b/libavdevice/alldevices.c
index 38010e288a..b31558bcb5 100644
--- a/libavdevice/alldevices.c
+++ b/libavdevice/alldevices.c
@@ -53,6 +53,7 @@  static void register_all(void)
     REGISTER_INDEV   (GDIGRAB,          gdigrab);
     REGISTER_INDEV   (IEC61883,         iec61883);
     REGISTER_INDEV   (JACK,             jack);
+    REGISTER_INDEV   (KMSGRAB,          kmsgrab);
     REGISTER_INDEV   (LAVFI,            lavfi);
     REGISTER_INDEV   (OPENAL,           openal);
     REGISTER_OUTDEV  (OPENGL,           opengl);
diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
new file mode 100644
index 0000000000..d0b9cf5001
--- /dev/null
+++ b/libavdevice/kmsgrab.c
@@ -0,0 +1,455 @@ 
+/*
+ * KMS/DRM input device
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <drm.h>
+#include <drm_fourcc.h>
+#include <drm_mode.h>
+#include <xf86drm.h>
+#include <xf86drmMode.h>
+
+#include "libavutil/hwcontext.h"
+#include "libavutil/hwcontext_drm.h"
+#include "libavutil/internal.h"
+#include "libavutil/mathematics.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixfmt.h"
+#include "libavutil/pixdesc.h"
+#include "libavutil/time.h"
+
+#include "libavformat/avformat.h"
+#include "libavformat/internal.h"
+
+typedef struct KMSGrabContext {
+    const AVClass *class;
+
+    AVBufferRef *device_ref;
+    AVHWDeviceContext *device;
+    AVDRMDeviceContext *hwctx;
+
+    AVBufferRef *frames_ref;
+    AVHWFramesContext *frames;
+
+    uint32_t plane_id;
+    uint32_t drm_format;
+    unsigned int width;
+    unsigned int height;
+
+    int64_t frame_delay;
+    int64_t frame_last;
+
+    const char *device_path;
+    enum AVPixelFormat format;
+    int64_t drm_format_modifier;
+    int64_t source_plane;
+    int64_t source_crtc;
+    AVRational framerate;
+} KMSGrabContext;
+
+static void kmsgrab_free_desc(void *opaque, uint8_t *data)
+{
+    AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor*)data;
+
+    close(desc->objects[0].fd);
+
+    av_free(desc);
+}
+
+static void kmsgrab_free_frame(void *opaque, uint8_t *data)
+{
+    AVFrame *frame = (AVFrame*)data;
+
+    av_frame_free(&frame);
+}
+
+static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt)
+{
+    KMSGrabContext *ctx = avctx->priv_data;
+    drmModePlane *plane;
+    drmModeFB *fb;
+    AVDRMFrameDescriptor *desc;
+    AVFrame *frame;
+    int64_t now;
+    int err, fd;
+
+    now = av_gettime();
+    if (ctx->frame_last) {
+        int64_t delay;
+        while (1) {
+            delay = ctx->frame_last + ctx->frame_delay - now;
+            if (delay <= 0)
+                break;
+            av_usleep(delay);
+            now = av_gettime();
+        }
+    }
+    ctx->frame_last = now;
+
+    plane = drmModeGetPlane(ctx->hwctx->fd, ctx->plane_id);
+    if (!plane) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to get plane "
+               "%"PRIu32".\n", ctx->plane_id);
+        return AVERROR(EIO);
+    }
+    if (!plane->fb_id) {
+        av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" no longer has "
+               "an associated framebuffer.\n", ctx->plane_id);
+        return AVERROR(EIO);
+    }
+
+    fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id);
+    if (!fb) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to get framebuffer "
+               "%"PRIu32".\n", plane->fb_id);
+        return AVERROR(EIO);
+    }
+    if (fb->width != ctx->width || fb->height != ctx->height) {
+        av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer "
+               "dimensions changed: now %"PRIu32"x%"PRIu32".\n",
+               ctx->plane_id, fb->width, fb->height);
+        return AVERROR(EIO);
+    }
+    if (!fb->handle) {
+        av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer.\n");
+        return AVERROR(EIO);
+    }
+
+    err = drmPrimeHandleToFD(ctx->hwctx->fd, fb->handle, O_RDONLY, &fd);
+    if (err < 0) {
+        err = errno;
+        av_log(avctx, AV_LOG_ERROR, "Failed to get PRIME fd from "
+               "framebuffer handle: %s.\n", strerror(errno));
+        return AVERROR(err);
+    }
+
+    desc = av_mallocz(sizeof(*desc));
+    if (!desc)
+        return AVERROR(ENOMEM);
+
+    *desc = (AVDRMFrameDescriptor) {
+        .nb_objects = 1,
+        .objects[0] = {
+            .fd               = fd,
+            .size             = fb->height * fb->pitch,
+            .format_modifier  = ctx->drm_format_modifier,
+        },
+        .nb_layers = 1,
+        .layers[0] = {
+            .format           = ctx->drm_format,
+            .nb_planes        = 1,
+            .planes[0] = {
+                .object_index = 0,
+                .offset       = 0,
+                .pitch        = fb->pitch,
+            },
+        },
+    };
+
+    frame = av_frame_alloc();
+    if (!frame)
+        return AVERROR(ENOMEM);
+
+    frame->hw_frames_ctx = av_buffer_ref(ctx->frames_ref);
+    if (!frame->hw_frames_ctx)
+        return AVERROR(ENOMEM);
+
+    frame->buf[0] = av_buffer_create((uint8_t*)desc, sizeof(*desc),
+                                     &kmsgrab_free_desc, avctx, 0);
+    if (!frame->buf[0])
+        return AVERROR(ENOMEM);
+
+    frame->data[0] = (uint8_t*)desc;
+    frame->format  = AV_PIX_FMT_DRM_PRIME;
+    frame->width   = fb->width;
+    frame->height  = fb->height;
+
+    drmModeFreeFB(fb);
+    drmModeFreePlane(plane);
+
+    pkt->buf = av_buffer_create((uint8_t*)frame, sizeof(*frame),
+                                &kmsgrab_free_frame, avctx, 0);
+    if (!pkt->buf)
+        return AVERROR(ENOMEM);
+
+    pkt->data  = (uint8_t*)frame;
+    pkt->size  = sizeof(*frame);
+    pkt->pts   = now;
+    pkt->flags = AV_PKT_FLAG_TRUSTED;
+
+    return 0;
+}
+
+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   },
+};
+
+static av_cold int kmsgrab_read_header(AVFormatContext *avctx)
+{
+    KMSGrabContext *ctx = avctx->priv_data;
+    drmModePlaneRes *plane_res = NULL;
+    drmModePlane *plane = NULL;
+    drmModeFB *fb = NULL;
+    AVStream *stream;
+    int err, i;
+
+    for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) {
+        if (kmsgrab_formats[i].pixfmt == ctx->format) {
+            ctx->drm_format = kmsgrab_formats[i].drm_format;
+            break;
+        }
+    }
+    if (i >= FF_ARRAY_ELEMS(kmsgrab_formats)) {
+        av_log(avctx, AV_LOG_ERROR, "Unsupported format %s.\n",
+               av_get_pix_fmt_name(ctx->format));
+        return AVERROR(EINVAL);
+    }
+
+    err = av_hwdevice_ctx_create(&ctx->device_ref, AV_HWDEVICE_TYPE_DRM,
+                                 ctx->device_path, NULL, 0);
+    if (err < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to open DRM device.\n");
+        return err;
+    }
+    ctx->device = (AVHWDeviceContext*) ctx->device_ref->data;
+    ctx->hwctx  = (AVDRMDeviceContext*)ctx->device->hwctx;
+
+    err = drmSetClientCap(ctx->hwctx->fd,
+                          DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+    if (err < 0) {
+        av_log(avctx, AV_LOG_WARNING, "Failed to set universal planes "
+               "capability: primary planes will not be usable.\n");
+    }
+
+    if (ctx->source_plane > 0) {
+        plane = drmModeGetPlane(ctx->hwctx->fd, ctx->source_plane);
+        if (!plane) {
+            err = errno;
+            av_log(avctx, AV_LOG_ERROR, "Failed to get plane %"PRId64": "
+                   "%s.\n", ctx->source_plane, strerror(err));
+            err = AVERROR(err);
+            goto fail;
+        }
+
+        if (plane->fb_id == 0) {
+            av_log(avctx, AV_LOG_ERROR, "Plane %"PRId64" does not have "
+                   "an attached framebuffer.\n", ctx->source_plane);
+            err = AVERROR(EINVAL);
+            goto fail;
+        }
+    } else {
+        plane_res = drmModeGetPlaneResources(ctx->hwctx->fd);
+        if (!plane_res) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to get plane "
+                   "resources: %s.\n", strerror(errno));
+            err = AVERROR(EINVAL);
+            goto fail;
+        }
+
+        for (i = 0; i < plane_res->count_planes; i++) {
+            plane = drmModeGetPlane(ctx->hwctx->fd,
+                                    plane_res->planes[i]);
+            if (!plane) {
+                err = errno;
+                av_log(avctx, AV_LOG_VERBOSE, "Failed to get "
+                       "plane %"PRIu32": %s.\n",
+                       plane_res->planes[i], strerror(err));
+                continue;
+            }
+
+            av_log(avctx, AV_LOG_DEBUG, "Plane %"PRIu32": "
+                   "CRTC %"PRIu32" FB %"PRIu32".\n",
+                   plane->plane_id, plane->crtc_id, plane->fb_id);
+
+            if ((ctx->source_crtc > 0 &&
+                 plane->crtc_id != ctx->source_crtc) ||
+                plane->fb_id == 0) {
+                // Either not connected to the target source CRTC
+                // or not active.
+                drmModeFreePlane(plane);
+                plane = NULL;
+                continue;
+            }
+
+            break;
+        }
+
+        if (i == plane_res->count_planes) {
+            if (ctx->source_crtc > 0) {
+                av_log(avctx, AV_LOG_ERROR, "No usable planes found on "
+                       "CRTC %"PRId64".\n", ctx->source_crtc);
+            } else {
+                av_log(avctx, AV_LOG_ERROR, "No usable planes found.\n");
+            }
+            err = AVERROR(EINVAL);
+            goto fail;
+        }
+
+        av_log(avctx, AV_LOG_INFO, "Using plane %"PRIu32" to "
+               "locate framebuffers.\n", plane->plane_id);
+    }
+
+    ctx->plane_id = plane->plane_id;
+
+    fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id);
+    if (!fb) {
+        err = errno;
+        av_log(avctx, AV_LOG_ERROR, "Failed to get "
+               "framebuffer %"PRIu32": %s.\n",
+               plane->fb_id, strerror(err));
+        err = AVERROR(err);
+        goto fail;
+    }
+
+    av_log(avctx, AV_LOG_INFO, "Template framebuffer is %"PRIu32": "
+           "%"PRIu32"x%"PRIu32" %"PRIu32"bpp %"PRIu32"b depth.\n",
+           fb->fb_id, fb->width, fb->height, fb->bpp, fb->depth);
+
+    ctx->width  = fb->width;
+    ctx->height = fb->height;
+
+    if (!fb->handle) {
+        av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer: "
+               "maybe you need some additional capabilities?\n");
+        err = AVERROR(EINVAL);
+        goto fail;
+    }
+
+    stream = avformat_new_stream(avctx, NULL);
+    if (!stream) {
+        err = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    stream->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+    stream->codecpar->codec_id   = AV_CODEC_ID_WRAPPED_AVFRAME;
+    stream->codecpar->width      = fb->width;
+    stream->codecpar->height     = fb->height;
+    stream->codecpar->format     = AV_PIX_FMT_DRM_PRIME;
+
+    avpriv_set_pts_info(stream, 64, 1, 1000000);
+
+    ctx->frames_ref = av_hwframe_ctx_alloc(ctx->device_ref);
+    if (!ctx->frames_ref) {
+        err = AVERROR(ENOMEM);
+        goto fail;
+    }
+    ctx->frames = (AVHWFramesContext*)ctx->frames_ref->data;
+
+    ctx->frames->format    = AV_PIX_FMT_DRM_PRIME;
+    ctx->frames->sw_format = ctx->format,
+    ctx->frames->width     = fb->width;
+    ctx->frames->height    = fb->height;
+
+    err = av_hwframe_ctx_init(ctx->frames_ref);
+    if (err < 0) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to initialise "
+               "hardware frames context: %d.\n", err);
+        goto fail;
+    }
+
+    ctx->frame_delay = av_rescale_q(1, (AVRational) { ctx->framerate.den,
+                ctx->framerate.num }, AV_TIME_BASE_Q);
+
+    err = 0;
+fail:
+    if (plane_res)
+        drmModeFreePlaneResources(plane_res);
+    if (plane)
+        drmModeFreePlane(plane);
+    if (fb)
+        drmModeFreeFB(fb);
+
+    return err;
+}
+
+static av_cold int kmsgrab_read_close(AVFormatContext *avctx)
+{
+    KMSGrabContext *ctx = avctx->priv_data;
+
+    av_buffer_unref(&ctx->frames_ref);
+    av_buffer_unref(&ctx->device_ref);
+
+    return 0;
+}
+
+#define OFFSET(x) offsetof(KMSGrabContext, x)
+#define FLAGS AV_OPT_FLAG_DECODING_PARAM
+static const AVOption options[] = {
+    { "device", "DRM device path",
+      OFFSET(device_path), AV_OPT_TYPE_STRING,
+      { .str = "/dev/dri/card0" }, 0, 0, FLAGS },
+    { "format", "Pixel format for framebuffer",
+      OFFSET(format), AV_OPT_TYPE_PIXEL_FMT,
+      { .i64 = AV_PIX_FMT_BGR0 }, 0, UINT32_MAX, FLAGS },
+    { "format_modifier", "DRM format modifier for framebuffer",
+      OFFSET(drm_format_modifier), AV_OPT_TYPE_INT64,
+      { .i64 = DRM_FORMAT_MOD_NONE }, 0, INT64_MAX, FLAGS },
+    { "crtc_id", "CRTC ID to define capture source",
+      OFFSET(source_crtc), AV_OPT_TYPE_INT64,
+      { .i64 = 0 }, 0, UINT32_MAX, FLAGS },
+    { "plane_id", "Plane ID to define capture source",
+      OFFSET(source_plane), AV_OPT_TYPE_INT64,
+      { .i64 = 0 }, 0, UINT32_MAX, FLAGS },
+    { "framerate", "Framerate to capture at",
+      OFFSET(framerate), AV_OPT_TYPE_RATIONAL,
+      { .dbl = 30.0 }, 0, 1000, FLAGS },
+    { NULL },
+};
+
+static const AVClass kmsgrab_class = {
+    .class_name = "kmsgrab indev",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_kmsgrab_demuxer = {
+    .name           = "kmsgrab",
+    .long_name      = NULL_IF_CONFIG_SMALL("KMS screen capture"),
+    .priv_data_size = sizeof(KMSGrabContext),
+    .read_header    = &kmsgrab_read_header,
+    .read_packet    = &kmsgrab_read_packet,
+    .read_close     = &kmsgrab_read_close,
+    .flags          = AVFMT_NOFILE,
+    .priv_class     = &kmsgrab_class,
+};