Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Use DRM as a fallback if not on X11 for VAAPI #10922
Conversation
BrandonSchaefer
commented
Nov 12, 2016
|
im pretty sure this would be consider a breaking change since (marked as such) ... ideally nothing changes but now the VAAPI will be using DRM soo there could be some unforeseen edge cases ive yet to hit in test. |
BrandonSchaefer
commented
Nov 13, 2016
•
Edited 1 time
-
BrandonSchaefer
Nov 13, 2016
-
Nov 13, 2016
|
I bring comparisons using 3840x2160p60 video: So for mir on its KMS platform fullscreen: Mir using no hardware accel: X11 same DRM VAAPI code: Not 100% sure why mir's drop is 60 perfectly with the FPS. But the X11 version (when I didnt switch workspaces) was very good. |
BrandonSchaefer
referenced this pull request
Nov 13, 2016
Merged
Mir windowing system for Kodi #10898
BrandonSchaefer
commented
Nov 13, 2016
|
Seems to be hard to get good drop/skip numbers when you switch VTs or move workspaces. Though they seem very close in numbers besides X11 have a 0 drop when never moving a workspace (soo very good!) Mir I can seem to get ~20 drop but I have to switch VT to send the debug message not sure if thats causing extra drop doing that. |
BrandonSchaefer
commented
Nov 13, 2016
|
Ok, so turning on debug by default for Mir finally solves the dropping (since i needed to switch a vt to send the PlayerDebug event) http://i.imgur.com/HkIpbg2.jpg Drop: 0 Skip: 0... yay! |
BrandonSchaefer
commented
Nov 13, 2016
|
Some more testing running Big Buck Bunny 3840x2160p60 on both Mir and X11 with the same code from start 0:00 until 8:18 (about when the credits start). Kodi is compiled with player debug enabled so no extra WM stuff getting in the way. Mir: drop 0 skip 3 If theres a better way to test this sort of thing let me know! |
|
Hi Brandon, I would map the PlayerDebug to ctrl-o in a custom keyboard.xml, therefore create: with the following content Then just press ctrl-o from time to time. Keeping it on screen produces further load, so just check every some seconds / minutes. |
|
The code is fine. I don't see a problem when comparing also our render part with: https://dvdhrm.wordpress.com/tag/drm/ . We succesfully use: EGL_LINUX_DMA_BUF_EXT to display it afterwards. On thing that could be tested on MIR though is: Go to setting and disable Prefer VAAPI-Output - then a cpu intensive SSE4 copy is used that is based on vaDeriveImage and vaMapBuffer. It should just continue to work. If not - that's on the list to be dropped anyways. Edit: This path will only be used for < 1080p input, as it does not scale performance wise for larger images. Don't forget to enable Prefer VAAPI-Output again after testing or performance will suck. |
| @@ -123,14 +128,44 @@ bool CVAAPIContext::EnsureContext(CVAAPIContext **ctx, CDecoder *decoder) | ||
| return true; | ||
| } | ||
| +namespace | ||
| +{ | ||
| +int find_open_render_node_fd() |
FernetMenta
Nov 13, 2016
Member
I think this is fine as a fallback when running headless (future case), but with UI we should not bypass vaapi api for getting the display handle. Consider the case with multiple cards and monitors connected.
https://cgit.freedesktop.org/libva/tree/va
^^ there should be code to get vaDisplay for MIR. Would you know if anybody intents to write and submit this code?
btw: we prefer camel case for function names.
fritsch
Nov 13, 2016
Member
In fact, that's not bypassing VAAPI-API, see the original commit done by Gwenole: http://libva.freedesktop.narkive.com/d5UZ8R3b/patch-0-2-add-support-for-drm-render-nodes
In kodi case I see it that the only thing that should have anything to do with a running Xserver is the display part. Decoding + Rendering should use the future API which is DRM + EGL.
I don't see the point depending a decoder on a display server when it is not needed.
FernetMenta
Nov 13, 2016
Member
I don't see the point depending a decoder on a display server when it is not needed.
makes no sense using the gpu one one gfx card when you have the display on the other one. vaapi provides the API for getting the display on the various systems and this has to be used.
FernetMenta
Nov 13, 2016
Member
further have fun creating an EGL context for interop with info from windowing. that means there is already direct link between decoder and windowing that cannot be removed.
fritsch
Nov 13, 2016
Member
Which is no reason to depend the decoder on a specific display API (like X11, etc.). As you see from the patches in the other PR, it will solely depend on EGL as a standard. If your X11 supports it, your MIR, your wayland whatever - you easily get exactly that egl display, which is totally DisplayServer unaware and just speaks the EGL API.
RAOF
Nov 14, 2016
So, since presentation is (mostly) unimportant, the most sensible approach would be to initialise VA on all available rendernodes and then pick the one with the best format support?
FernetMenta
Nov 14, 2016
Member
Who said that presentation would be unimportant? Kodi maps OpenGL textures to vaSurfaces (zero-copy). Please elaborate how this works when having decoding on one gpu and the GL context on the other.
RAOF
Nov 14, 2016
As far as I can tell from VAAPI.cpp, Kodi uses eglCreateImageKHR with the EGL_EXT_image_dma_buf_import extension to import the buffers identified by the dma-buf fds as textures. Those dma-buf fds are cross-GPU handles.
The dma-buf fds you get from the vaSurfaces will be references to memory on the decoder-GPU; if the GL context is not on the same GPU, then eglCreateImageKHR will implicitly perform a GPU→GPU migration.
This means that decoding on one GPU and presenting on a different GPU is not zero-copy, but the copy is hidden in the driver and will generally be performed by a hardware DMA engine. As this is only happening at most 3 times per frame, it shouldn't be a particularly noticeable performance impact.
FernetMenta
Nov 15, 2016
Member
I wrote this code in order to get zero-copy and I certainly won't break this strategy for something I don't see any benefit in. Introducing a hidden any highly brittle copy operation done by the driver does not make any sense to me.
As this is only happening at most 3 times per frame, it shouldn't be a particularly noticeable performance impact
I seriously doubt. Why don't you try with a 4k @ 60 fps movie and post the numbers.
BrandonSchaefer
changed the title from
* Move VAAPI to use DRM vs directly using X11
to
Use DRM as a fallback if not on X11 for VAAPI
Nov 13, 2016
BrandonSchaefer
commented
Nov 18, 2016
|
Either way it goes, I think its still a good idea to get a fallback for render nodes/DRM here. Mainly if the windowing system (ie. Mir in this case) doesnt support a direct GetVaDisplay. Mir requires some work to allow direct access to its buffers. So DRM will be far better then software rendering. This will still pick X11 VaDisplay vs render nodes when on X11. Let me know when you want me to squash the commits. |
| -bool CVAAPIContext::CreateContext() | ||
| +namespace | ||
| +{ | ||
| +VADisplay FindValidDRMVaDisplayFromRenderNode() |
BrandonSchaefer
Nov 19, 2016
Its in an unnamed namespace, so it'll have internal linkage. It also doesnt depend on anything in the class it self (soo I tend to like pure functions :). But fine with me, changing!
| VADisplay m_display; | ||
| int m_refCount; | ||
| int m_attributeCount; | ||
| VADisplayAttribute *m_attributes; | ||
| int m_profileCount; | ||
| VAProfile *m_profiles; | ||
| std::vector<CDecoder*> m_decoders; | ||
| + int m_fd{-1}; |
| { | ||
| - { CSingleLock lock(g_graphicsContext); | ||
| - if (!m_X11dpy) | ||
| - m_X11dpy = XOpenDisplay(NULL); |
BrandonSchaefer
Nov 19, 2016
I removed this, because it wasnt referenced anywhere, but I can put it back (not sure if there were plans for it later). Ill add back!
FernetMenta
Nov 19, 2016
Member
It is referenced in current code:
{ CSingleLock lock(g_graphicsContext);
if (!m_X11dpy)
m_X11dpy = XOpenDisplay(NULL);
}
m_display = vaGetDisplay(m_X11dpy);
wsnipex
added
Improvement
Linux
Rendering Video
Video
labels
Nov 19, 2016
| +#if HAVE_X11 | ||
| + Display *x11_display{nullptr}; | ||
| + { CSingleLock lock(g_graphicsContext); | ||
| + x11_display = XOpenDisplay(nullptr); |
FernetMenta
Nov 19, 2016
•
Edited 1 time
-
FernetMenta
Nov 19, 2016
Member
-
Nov 19, 2016
this opens a display connection every time we start some video. this is not what we want here. we want to
open it once for the lifetime of the application.
| + } | ||
| + } | ||
| + | ||
| + CLog::Log(LOGERROR, "FAILED To find any open render nodes in /dev/dri/renderD<num>\n"); |
| VADisplay m_display; | ||
| int m_refCount; | ||
| int m_attributeCount; | ||
| VADisplayAttribute *m_attributes; | ||
| int m_profileCount; | ||
| VAProfile *m_profiles; | ||
| std::vector<CDecoder*> m_decoders; | ||
| + int m_render_node_fd{-1}; |
BrandonSchaefer
commented
Nov 19, 2016
•
Edited 1 time
-
BrandonSchaefer
Nov 19, 2016
-
Nov 19, 2016
|
Opps has to be a static fixing... |
FernetMenta
removed
the
Rendering Video
label
Nov 19, 2016
|
looks good now, thanks very much! |
| + } | ||
| + } | ||
| + | ||
| + CLog::Log(LOGERROR, "FAILED To find any open render nodes in /dev/dri/renderD<num>"); |
| + | ||
| + if (m_display == nullptr) | ||
| + { | ||
| + CLog::Log(LOGERROR, "FAILED To find any a VaDisplay for this system\n"); |
hudokkow
Nov 19, 2016
Member
Sorry for nitpicking. It's either "any VaDisplay" or "a VaDisplay". But English is not my native language...
BrandonSchaefer
Nov 19, 2016
Yeah you're right, english is my native language and im not super good at it :)
|
jenkins build this please |
FernetMenta
merged commit 19a13a8
into
xbmc:master
Nov 30, 2016
hudokkow
added this to the L 18.0-alpha1 milestone
Nov 30, 2016
hudokkow
added
the
v18 Leia
label
Nov 30, 2016
|
With this change, I'm getting a build failure: It looks like "-lva-drm" is missing. |
|
yes, seems jenkins does not build vaapi then |
|
@BrandonSchaefer could you submit a fix for this? |
BrandonSchaefer
commented
Dec 2, 2016
|
Opps yeah I didnt add finding libva-drm to the FindVaapi.cmake. Ill get something up tomorrow (a bit late for me) |
BrandonSchaefer
commented
Dec 2, 2016
|
Though whats strange, it builds fine for me as well as here: How did you compile this? I should be adding libva-drm to the FindVAAPI.cmake but just curious how I cant get it to fail nor does it fail when ran through travis. |
|
I'm using auto tools/make, not cmake, not sure if that matters. |
BrandonSchaefer
commented
Dec 2, 2016
|
Autotools are planned to be removed soon, annnd I didnt test it out there :). You can try cmake in kodi/project/cmake which looks to be working. |
if you are using autotools, you are on your own. if it builds with cmake, all fine. |
BrandonSchaefer commentedNov 12, 2016
•
Edited 1 time
-
BrandonSchaefer
Nov 13, 2016
Use DRM in VAAPI as a fallback if X11 is not present.
Description
If we are not on the X11 windowing system lets fall back to using DRM for VAAPI. Which will use vaGetDisplayDRM using render nodes. This requires opening any device in /dev/dri/renderD.
Motivation and Context
This will allow hardware acceleration in other display servers that use DRM/EGL such as mir (and most likely wayland, untested).
How Has This Been Tested?
This has been tested on X11 and my Mir branch here: #10898 on ubuntu 16.10 (yakkety)
This change does require at lease a kernel >= 3.15.
Screenshots (if appropriate):
http://i.imgur.com/OGtB2W7.jpg (x11)
Types of change
Checklist: