plugins: linux-vaapi, Linux VA-API H.264 encoder #941
Conversation
Includes code from the reboot/obs-studio github project and the vaapi-h264 branch. This adds the ability to use libva to use the gpu to do hardware encoding. It is fully functional for both streaming and file recording. Tested with Intel Haswell chipset.
|
Unfortunately work is blocking the travis-ci build link, I'll have to check to see why that failed this weekend. |
kingargyle
changed the title
|
Hey David,
Let me know if if you have any questions about this code! I think the only
remaining was adding b-day support (more complicated logic for setting
which frames ref each other in the gop, since now it is pretty easy and
just increments with a single ref to ref frame)
…On Jun 9, 2017 7:10 AM, "David Carver" ***@***.***> wrote:
Unfortunately work is blocking the travis-ci build link, I'll have to
check to see why that failed this weekend.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOGagojgF6P4fJ-rOqi0HdGS-8K7quoks5sCTYqgaJpZM4N0fH8>
.
|
|
@kc5nra It is pretty functional as it is right now from my limited testing. I mainly wanted to get this back out to a wider audience, as it really made a huge difference on my dedicated streaming laptop I take to tournaments with me. I might not have time myself to work on it but figure the wider community could help out to finish it up. |
|
This looks like it is missing the libva-dev files installed on the travis build system. In file included from /home/travis/build/jp9000/obs-studio/plugins/linux-vaapi/surface-queue.c:1:0: It can't find the appropriate header files on the system. |
There was a problem hiding this comment.
Hi! Congrats on your first PR! To help out, I've reviewed this PR for code style, spelling, and your Travis build issue.
The commit message subject should be prefixed by the component. Use something like:
linux-vaapi: Add Linux VA-API H.264 encoder
The rest of the commit message (the body) should be wrapped at 72 characters.
I don't see a reason to include the doc-pak/* files in this PR. They should probably be removed from this PR.
In addition to the review comments I've made on handling missing dependencies (libva, etc.), you should add the following line to CI/install-dependencies-linux.sh on line 22 (bump libvlc down to line 23):
libva-dev \
Sorry for the lengthy review. Let me know if you have any questions!
| #include "bitstream.h" | ||
| #include <util/darray.h> | ||
|
|
||
| #define BS_ROUNDUP_DIV(x, sz) ((x+(sz-1))/sz) |
There was a problem hiding this comment.
Style: Use spaces around the arithmetic operators, unless it would somehow break the define.
| // nal_hrd_parameters_present_flag | ||
| bs_append_bool(bs, nal_hrd_parameters_present_flag); | ||
| if (nal_hrd_parameters_present_flag) { | ||
|
|
| for(size_t i = 0; i < enc->caps->attribs_cnt; i++) { | ||
| VAConfigAttrib attr = enc->caps->attribs[i]; | ||
| switch (enc->caps->attribs[i].type) | ||
| { |
There was a problem hiding this comment.
Style: This curly brace should be at the end of the preceding line (on the same line as the "switch" statement).
|
|
||
| enum nal_unit_type | ||
| { | ||
| NAL_UNKNOWN = 0, |
There was a problem hiding this comment.
Style: Lines 47-58 should be indented with a single tab instead of using spaces.
|
|
||
| enum nal_priority | ||
| { | ||
| NAL_PRIORITY_DISPOSABLE = 0, |
There was a problem hiding this comment.
Style: Lines 63-66 should be indented with a single tab instead of using spaces.
| obs_data_set_default_int (settings, "min_qp", 1); | ||
| obs_data_set_default_int (settings, "max_qp_delta", 4); | ||
| obs_data_set_default_bool (settings, "rc_type", VAAPI_RC_CBR); | ||
| obs_data_set_default_int (settings, "profile", |
There was a problem hiding this comment.
Style: I don't see a reason to line break this. It doesn't exceed 80 characters.
|
|
||
| static void vaapi_enc_defaults(obs_data_t *settings) | ||
| { | ||
| obs_data_set_default_int (settings, "device_type", VAAPI_DISPLAY_X); |
There was a problem hiding this comment.
This block (395-405) is odd. It places spaces between the called function name and the opening parenthesis. I'm pretty sure you aren't supposed to do that. If you're going to use spaces to align something, you could use them to align the second parameter in these calls.
| bool obs_module_load(void) | ||
| { | ||
| if (!vaapi_initialize()) { | ||
| VA_LOG(LOG_WARNING, "no VA-API supported devices found with " |
There was a problem hiding this comment.
Style: Don't break user-visible strings. See other comments about this as well.
Consistency note: I've seen "vaapi", "VAAPI", and "VA-API" throughout. For printed strings, I'd suggest picking one style and using that.
| @@ -0,0 +1,40 @@ | |||
| project(linux-vaapi) | |||
|
|
|||
There was a problem hiding this comment.
To allow disabling of this module, add this here:
if(DISABLE_VA)
message(STATUS "VAAPI plugin disabled")
return()
endif()
|
|
||
| find_package(LibVa) | ||
| find_package(X11 REQUIRED) | ||
|
|
There was a problem hiding this comment.
To allow graceful failure/omission of this module if libva or other dependencies are not found on the system, add this here:
if(NOT LIBVA_INCLUDES_FOUND AND ENABLE_VA)
message(FATAL_ERROR "LibVA includes not found but set as enabled")
elseif(NOT LIBVA_INCLUDES_FOUND)
message(STATUS "LibVA includes not found, VAAPI plugin disabled")
return()
endif()
|
Haha, thanks RytoEX. This code is old, though I appreciate you going through all this. It might make more sense if I reopen this PR with RytoEX's fixes so that the origin authorship is preserved in the git history. Let me know if this is something you are interested in! |
|
@kc5nra yeah, I'm fine with you re-opening the PR since you originally wrote it and have the commit history. I just didn't want this to get lost and found it useful enough to use in place of the x264 encoder. |
|
@kc5nra No problem. If you open a new PR for this and you want another review of that, just tag me. |
|
If you reopen the PR with the original branch don't forget to include the fix i made on https://github.com/reboot/obs-studio/commits/vaapi-h264. This PR is based on my repository, as far as i know, and should contain the fix. |
|
@reboot yes, it is based on your repository. |
|
What is currently holding this back? |
|
@pschichtel One build fails on travis: https://travis-ci.org/jp9000/obs-studio/jobs/240913031#L4248 (may be a reason) |
|
Actually, I think @kc5nra was going to re-submit the PR since he originally worked on it, and it would contain the full development history. That is the reason I haven't tried to address any of the issues above or the build failure. |
|
Ya, debating whether to submit it with libyami which would get vp8/9,264/5
hardware encoding, or fixing mine to work with bframes.
…On Jun 25, 2017 12:48 PM, "David Carver" ***@***.***> wrote:
Actually, I think @kc5nra <https://github.com/kc5nra> was going to
re-submit the PR since he originally worked on it, and it would contain the
full development history. That is the reason I haven't tried to address any
of the issues above or the build failure.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAOGaj0a8P5UOj3GskerHcoVAb8-n-U6ks5sHp1RgaJpZM4N0fH8>
.
|
|
@kc5nra I ran this code for 13+ hrs of recording this last weekend, and it worked well, expect for the occasional B-Frame glitch every once in a while. If I had the time I'd try and investigate it myself as this worked well enough for what I needed it to do (i.e. relieve the stress on the CPU so it could do other things besides encode the video). I'd encourage fixing the B-Frame and then working on the libyami implementation, but it is up to you. |
|
Hello. I tried using this branch, but whenever I start recording or streaming with hardware encoder, it fails with error: Still, h264_vaapi hardware encoder seems to work ok when converting streams in ffmpeg. |
|
@vsnrain what does the command |
|
@vsnrain after upgrading to 17.04 of Ubuntu, I'm seeing the same error. |
|
More information; info: [VAAPI encoder]: settings: info: libfdk_aac encoder created The issue appears that the display_path is returning NULL. I'll see if I can track this down. |
|
Further investigation seems to be a bug either in Ubuntu 17.04 or the libva implementation there. 1.7.3-2 causes the same problem. Ubuntu 16.10 which has 1.7.1 and corresponding intel drivers worked. I didn't have any issues until upgrading to 17.04. I've opened up an issue with the libva project, so maybe I can get some info from them |
|
I am using Arch, and I tried downgrading packages to and it is still does not work. Same behavior, but this time it does not even show error and end log with the video file is created, but only with single frame. |
|
@vsnrain I'm going to try and install a dual boot into ubuntu 16.10 later tonight which has 1.7.1-2 and I'll let you know what I see. I didn't start getting problems until I upgraded to 17.04. I'll let people know what I find out, and hopefully can get some extra info from the libva project itself. |
|
Locally I was able to get this working again, but I had to go back to Ubuntu 16.04 and it's native libva libraries. This is running 1.7.0 of the intel driver. I've tested it with FLV and MKV files. I do think long term that lib-yami is probably a better option as it wraps VA-API and takes much of the grunt work out of it from what I can tell. Something changed in the later versions, just not sure what yet. |
|
So answer I got from the LIBVA developers is as follows. "intra_period is set to 60 however ip_period is to 0 in your case, however 0 is invalid for ip_period when using CBR mode. The new driver adds some checks for intra_period/ip_period. Please set a valid ip_period, sorry for the inconvenience." So, it looks like with the newer drivers we need to ip_period to something other than 0. |
|
Putting this here, as a sample on on how to set the ip_period, (sample comes from the libva project). "You may refer to https://github.com/01org/libva-utils/blob/master/encode/h264encode.c for how to set ip_period in your application:" |
|
I also get this error when using CBR: So I tryed CQP, maybe I can at least use it for recording. A part of the console output after obs started: I made a 2 seconds test recording (because I don't want my computer to freeze again) and this is the log file of it: The recorded video is ok and plays fine in VLC. My OBS settings: I'm using this branch: https://github.com/kingargyle/obs-studio/tree/vaapi-h264 |
|
yeah, this code should be considered beta. I'm hoping to get some time soon to at least address the CBR issue that has been reported. I've had some success with VBR but again that it is on Ubuntu 16.04. |
|
@RytoEX haven't had time myself to work on things. I know what is causing the issue for the crashes some people are experiencing I just haven't had time to actually make the appropriate change in the code. I thought @kc5nra was going to either update this or create a different implementation. I've got a working system locally but would like to try and fix it for others using the newer vaapi drivers. Last implementation I did was just to sync my code up with master to make sure it still worked locally. |
|
Just making a sort of official note here -- unfortunately still not mergeable as of yet I feel. Also still has that cmake configuration issue where it still won't find va/va.h when compiling for example. The required package really needs to be put in cmake. Might be worth checking out libyami like fruitman was saying. Unfortunately I can't really dedicate the time to implement this myself -- I feel like I'll really have to delegate this to someone else who's able and willing. I have a backup of the old branch still locally as well just in case. |
|
@jp9000 I agree that libyami is probably the way forward. I'll be giving a presentation in a couple weeks at the Ohio Linuxfest on OBS Studio and I'll bring up there that the project is looking for someone to step up and create an Intel hardware encoder. Like others, my time unfortunately right now is limited and I have a version working for my needs. If you want to close the PR, feel free to do so I'll keep my branch up to date with master. |
|
Oh really? That's quite awesome, wish I could be there to see it in person and say hi -- if there's a video of it email it to me. Perfectly understandable though -- I'm very grateful regardless. Having it as a PR put it kind of in a more public spotlight than when it was rotting on a stale branch, and the testing on the PR has been interesting. VAAPI is certainly usable, although implementing it properly is definitely not a simple task. A library that implements it is likely the best option at this point. It's not really a critical or high priority thing these days as CPUs are more powerful and have more cores. x264 doesn't really have troubles these days like it used to 4 or 5 years ago. I suppose hardware encoding is mostly just a luxury more than anything, so definitely not a big deal either way. |
|
From what I have seen about libyami, this library seems focus on Intel's CPUs. Would this still work with other GPUs supporting vaapi ? |
|
@jp9000 actually, I use a laptop for most of my use of OBS and honestly, I had to have hardware encoding because using the x264 encoder would eventually cause dropped frames when the CPU usage would get up around 20% on the system. Using the VAAPI hardware encoder built into the laptop, keeps the CPU usage around 8% to 10% which doesn't cause any frame drops. So in my case, I need the hardware encoding as x264 even on the fastest setting possible uses too much CPU for the poor laptop. |
|
I suppose that's true, I have a bad tendency to sort of disregard laptops when it comes to serious streaming -- it's kind of a bad habit. |
|
@jp9000 No big deal, the use case I have, is recording Live events for miniature games, and the laptop was just the most portable solution. Ideally I get the time to build out a more robust mini desktop machine that can be taken, but until then the laptop with hardware encoding does the job. |
|
I have been wondering if there is any benefit of directly implementing a VAAPI encoder. FFmpeg has a h264_vaapi encoder that could probably be used just like it is already done for NVENC. |
|
Any way to use the ffmpeg h264_vaapi encoder in OBS? Or is that something that would require a pull request? |
|
The FFmpeg output in the advanced recording settings should allow any codec supported by FFmpeg. |
|
There isn't such a setting in the streaming tab |
|
@reboot I've tried to get the ffmpeg encoder with vaapi working in OBS and have failed miserably. None of the command line options seem to work in the ffmpeg encoder settings. If somebody wants to wrap a friendlier gui around it I'd be all for it, but until then, I think from a users point of view, having something native just makes it easier for those that don't want to have a PHD in FFMPeg. |
|
Well technically all you'd need to do it basically copy a significant portion of the NVENC code (which is implemented through FFmpeg similarly) and just adapt it to VAAPI, I should probably try that if I ever get time. Could probably make that code abstract at some point like I did for the audio encoders. |
|
Can one of the admins verify this patch? |
|
@ONSBalder there are some bugs as noted with this patch that would need to be addressed so that it works with the newer VAAPI versions. I think this is mainly staying open until either I get time to try and fix a few of the bugs, or somebody comes along and writes a better more complete implementation. |
|
@kingargyle |
|
Ah... thought that was the case after I checked it's activity. :) |
|
I took the code from w23 and updated it to work with the newest obs. Old source: https://github.com/w23/obs-studio/tree/ffmpeg-vaapi The only thing that you have to change is this line in ret = av_hwdevice_ctx_create(&enc->vadevice_ref, AV_HWDEVICE_TYPE_VAAPI, "/dev/dri/renderD129" /*FIXME*/, NULL, 0);for me it works with TODO:
BUGS:
|
|
Hi guys, I've tried the @kingargyle branch and this is what throws me into the console: |
|
Hi guys, as I do not see any new advances... I want to inform mine ...
Regards |
|
@eduardoc25 - If you're only interested in full screen recording, newer versions of ffmpeg have |
|
Is it dead? |
|
We're going to use FFmpeg's implementation rather than a custom implementation. Someone is working on it currently, just not complete yet. |
This comment has been minimized.
This comment has been minimized.
|
I tried building this for my x5-z8300 Intel Compute Stick using kingargyles sources and had the same issues as Bleuzen.. Going to try with his solution next..
Please NO! I cannot understand why people working with similar open source projects do not take the time to understand hardware encoding / decoding etc. In my opinion it is very important to understand that a lot of people are using very old hardware or in other words cheap hardware to run.. you've guessed it; open source software. This means one should forget modern hardware, such as the Intel Atoms like Bay Trail and more recently Cherry Trail CPUs which are all very capable computers for the price. And I mean all, ALL!!, it takes is to implement proper support for at least the basic hardware capabilities of the CPUs that exist! Imagine all the Atom-based TV-devices running 2W TDP (and older hardware, like the Intel HD2000, HD2500 and HD3000) going from potentially unusable 720p h264 encoding to something where you can actually encode 1080p on a 2W cpu with less than 70% cpu usage. Again, In my opinion it is a very big deal.
If I can ask, maybe in the future at least think about disregarding ALL low power cpus and older hardware, not just laptops, since the reality is you can reliably run OBS on this: ...On Windows. |
|
There is now a PR that uses the FFmpeg implementation of VAAPI here: #1255 |
|
Moved -> #1256 |
Includes code from the reboot/obs-studio github project and the vaapi-h264 branch.
This adds the ability to use libva to use the gpu to do hardware encoding. It is
functional for both streaming and file recording.
Tested with Intel Haswell chipset.
Note that I had contact @reboot in regards to this code as it came from his branch. According to him it was from another branch in the original obs-studio code, but that branch has since been deleted and the work was never included into the original code. It also appears that @kc5nra did a lot of the original work.
I'm re-submitting it for possible inclusion if somebody wants to take it over. I believe it addresses issue
https://obsproject.com/mantis/view.php?id=263