New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

obs-ffmpeg: add linux VAAPI h.264 encoding support #1256

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@GloriousEggroll
Copy link
Contributor

GloriousEggroll commented Apr 8, 2018

Alright lets try this again. This allows obs to encode on a selected gpu using VAAPI via ffmpeg. Details:
https://wiki.archlinux.org/index.php/Hardware_video_acceleration

VAAPI is an open source gpu hardware encoding and decoding library for linux, which has built in compatibility with mesa. This works natively with intel and amd open source drivers via mesa.

In a nutshell it's the linux open source alternative to Quicksync and AMD AMF.
This is in reference to this forum post:
https://obsproject.com/forum/threads/experimental-ffmpeg-vaapi-plugin.61529/

@jp9000

This comment has been minimized.

Copy link
Member

jp9000 commented Apr 8, 2018

You didn't have to close the pull request. A pull request is just a remote branch. All you need to do to modify a pull request is push or force push on to that remote branch.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Apr 8, 2018

yeah I got your message and replied before I closed it, I wanted to move the changes to a completely seperate branch so that I could add onto it later if I need to and keep the master branch in sync with upstream. my old changes were committed directly to master because I did them in a hurry

@jp9000

This comment has been minimized.

Copy link
Member

jp9000 commented Apr 8, 2018

Ah, I see. Alright, that was probably for the best then, you generally don't want to PR something on your fork's master branch.

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Apr 12, 2018

I'll try and give this patch a test in the next day or two.

It appears you've significantly reworked profile handling since one of your much older patches? (dated November 26, 2017) - That patch had a bunch of codec-specific profiles (such as h264-main) which did not match the naming conventions for profiles in OBS' "Enforce streaming service encoder settings" feature, which would choose a profile of "main" for Facebook for example - which would get rejected by the driver as invalid causing a fairly generic "unimplemented" error.

I'll give this patchset a whirl soon to see if that issue is resolved, I'm guessing from the significant architectural changes of this patch it may be.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Apr 12, 2018

@Entropy512 I removed quite a bit of unimplemented/unfinished features and left strictly h264 and strictly profile 578 (constrained baseline) after further researching why certain other profiles would not work. Mesa devs confirmed in particular that 578 should be used. This profile is also the only profile that would render properly on twitch for both intel and amd. I assume the same is for facebook. I also fixed the device selection as the menu worked but it would not actually select the proper device. Should work properly now.

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Apr 12, 2018

I'm pretty sure h264-main was what I was using on Facebook without issues, but sticking with Main should probably be OK.

I know there was one thing in your old patchset (deleting the patch when I was prepping my repo for your new patch was probably a mistake...) regarding something that had to be set to 0 for AMD but was OK to be set to a nonzero value on Intel. Apparently a special/ultra-recent version of Mesa was needed on AMD chipsets? A mostly unmodified (I was only running PPAs for kdenlive and gnuradio-related items I'm fairly certain) Ubuntu 17.10 install seemed to work fine with your old patchset on a Kaby Lake machine. I've built your latest patchset and fired up OBS, but not actually tried doing any streaming yet. I might get a chance tomorrow. Unfortunately most local venues don't have a good enough network connection for livestreaming, but the one that does has a weekly Thursday show of their house band.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Apr 12, 2018

@Entropy512 b-frames are not supported on amd, this needs to be set to 0 on amd (which is noted in the interface). some settings did work for intel but not for amd for example the high profile worked for recording on intel and not on amd. amd was able to record using main profile as well, but the profile was not accepted when streaming to twitch. I removed the extra options and limited only availability that works on both platforms. constrained baseline (578) should work for both intel and amd for all streaming+recording purposes. As this is an initial patch that just allows base functionality, extra features can be added later on

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Apr 12, 2018

Thanks for the clarification. Makes sense to stick with the basics for now - it would be nice if there were a way to query for allowable/supported values, permitting bframes to be enabled on Intel devices. Perhaps after the initial patch gets accepted with basic support - which is still a MASSIVE improvement on a Kaby Lake laptop.

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Apr 13, 2018

I'm testing the current patch on an Intel Kaby Lake machine (i5-7200U) using an Elgato Cam Link (USB3 HDMI-to-UVC) fed by a Sony A7III - it's working very well. I forgot to turn "Enforce streaming service encoder settings" back on though - I need to verify that, but overall the patch seems to be solid so far.

@gilles-duboscq

This comment has been minimized.

Copy link

gilles-duboscq commented Apr 21, 2018

Thank you @GloriousEggroll for putting this together.

I gave it a quick try on a RX 580 and it worked fine.
Note however that the cmake files don't check for the presence of va/va.h.

@RytoEX
Copy link
Contributor

RytoEX left a comment

Hi! I've done a review of this PR, mostly for compliance with this project's style guidelines.

Additionally, could you explain the changes in the Linux dependencies that you made in CI/install-dependencies-linux.sh so that we know what changed and why?

Lastly, could you amend your commit message to this to fix some capitalization issues?

obs-ffmpeg: Add Linux VAAPI H.264 encoding support
Show resolved Hide resolved plugins/obs-ffmpeg/CMakeLists.txt
Show resolved Hide resolved plugins/obs-ffmpeg/CMakeLists.txt
Show resolved Hide resolved plugins/obs-ffmpeg/CMakeLists.txt
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg-vaapi.c
Show resolved Hide resolved plugins/obs-ffmpeg/obs-ffmpeg.c
@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Apr 23, 2018

@RytoEX

"Additionally, could you explain the changes in the Linux dependencies that you made in CI/install-dependencies-linux.sh so that we know what changed and why?"

This was explained in my previous PR: #1255

exact answer: #1255 (comment)

the new ppa is a backport of ffmpeg-3 to 14.04

I'll see if I can fix those styling nitpicks when I get some time

@juvester

This comment has been minimized.

Copy link
Contributor

juvester commented Apr 24, 2018

@GloriousEggroll @RytoEX
Something to keep in mind is that the change would not be limited to just the CI builds, the PPA for OBS itself would be affected too. Current users on Trusty would need to purge the old FFmpeg PPA from their system and add the new one. Also users on Xenial would now need to add it as well. Probably not a big deal but definitely something to keep in mind.

I believe one way to get rid of the whole 3rd party PPA issue would be to just host the relevant FFmpeg packages in OBS' own repository. Not even build them or anything, just copy them from ppa:jonathonf/ffmpeg-3 to ppa:obsproject/obs-studio. Not sure if there's any drawbacks to this but according to https://help.launchpad.net/Packaging/PPA/Copying it seems to be at least allowed.

edit.
Also tagging @Gol-D-Ace since he's the PPA guy.

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Apr 28, 2018

Out of curiosity, what is the policy on how many releases back a PPA supports?

Obviously to the most recent LTS makes sense, which explains why one would have to go back to xenial prior to the release of 18.04 - but now that bionic is out, what's the new policy on a PPA supporting old revisions? Will the ci system be moved to bionic as that is an LTS release?

Pulling in ffmpeg from a PPA was not necessary on artful and I assume it isn't on bionic either (I have not tried rebuilding OBS since upgrading.)

@kkartaltepe

This comment has been minimized.

Copy link
Contributor

kkartaltepe commented May 9, 2018

Should add in another cmake module for finding libva, dependencies were updated in the readme but CMakeLists was not. libva should be distributed with pkg-config so a simple fallback should do ala

include(FindPkgConfig)
pkg_check_modules(LIBVA REQUIRED libva>=1.0.0)

Reference https://github.com/obsproject/obs-studio/blob/master/cmake/Modules/FindLibv4l2.cmake for a simple cmake find module with pkg-config and a reasonable attempt at detecting should that fail.

@kkartaltepe

This comment has been minimized.

Copy link
Contributor

kkartaltepe commented May 9, 2018

compiled and running beautifully on 18.04 without any external PPA's on a 7100U.
-- Edit
One question, is there any current issue preventing the hardware surface from being used as the encoding input? FFmpeg supports decoding vaapi -> encoding vaapi without downloading frames as explained https://trac.ffmpeg.org/wiki/Hardware/VAAPI#Encoding. Assuming the user has not selected an incompatible pixfmt for obs, and the main texture is on the correct device, is this possible as a future improvement?

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented May 10, 2018

I feel I need to clear some confusion on the PPAs. The PPA change is only for the buildbot, because it uses older ubuntu 14.04. I explained already why this change is needed for ubuntu 14.0.4 but I'll do so again:

the build system for travis-ci uses ubuntu 14.04.
libav requires libavutil-dev because the PPA used does not include these:
libavutil/hwcontext.h
libavutil/hwcontext_vaapi.h

and I can't change the packages to the non-ffmpeg versions, because the ffmpeg packages all require
libavutil-ffmpeg-dev, AND..
libswresample-dev doesn't exist in trusty, because ubuntu and debian decided they wanted to be special at some point in that time and fork ffmpeg, and they used libavresample with a different prefix on all the libraries.

so in order for travis-ci not to complain the PPA needs to be changed to one with a more recent version of ffmpeg backported (which is what I did). again, this is only for much older versions of ubuntu.

@juvester is correct that people using these older versions of ubuntu would have to purge their current ffmpeg+ ffmpeg ppa and add+install from the new one. I also agree that hosting it on obs's own ppa would be a good idea

@kkartaltepe I'm unsure on the answer to your question regarding the hardware surface. This patch was originally created by forum user w23, I just cleaned it up and fixed it so it allowed the user to select which encoding device to use and fixed the profiles used so that it's compatible with both intel and amd hardware. If you have any changes you're more than welcome to submit a pull request on my repo

@rkantos

This comment has been minimized.

Copy link

rkantos commented May 11, 2018

One question, is there any current issue preventing the hardware surface from being used as the encoding input? FFmpeg supports decoding vaapi -> encoding vaapi without downloading frames as explained https://trac.ffmpeg.org/wiki/Hardware/VAAPI#Encoding. Assuming the user has not selected an incompatible pixfmt for obs, and the main texture is on the correct device, is this possible as a future improvement?

While I would appreciate all hardware encoding since it would make OBS usable on nearly anything running with an Intel integrated GPU; Isn't this something that would need to be addressed much more fundamentally in OBS in relation to FFmpeg? It would be nice to be able to select hardware decoding as per input source too. However as far as I know it isn't enabled anywhere, and furthermore isn't really related to this PR (hw enc on Linux) either.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented May 30, 2018

Just a note for those running on AMD hardware - I did some tests digging a bit further into the "encoding overload" issue. From what I can tell, weaker cards seem much less capable of maintaining 60fps.
I got my hands on an RX 550, and was able to run some tests:

1080p60 fps, encoder overload no matter what settings
1080p30fps with no problem
900p60fps encoder overload no matter what settings
900p30fps no problem
720p60fps encoder overload no matter what settings
720p30fps no problem

I really do believe it comes down to the strength of the card, as I did not have these problems with my Vega. I was unable to test my RX 580 because I currently have it passed through a VM for windows. It may also be some hidden firmware that is limiting the cards. I'm not sure. I would also like to test more on intel integrated gpus, however I do not have one at the moment

@dribbleondo

This comment has been minimized.

Copy link

dribbleondo commented Jun 3, 2018

I should probably make you aware of the FFMPEG-4 PPA which, on my test bench of an Ryzen 5 1500x and a RX 470, fixed the issue of VA-API either not loading or constantly stammering, no matter the resolution, which is what I experienced using the FFMPEG-3 PPA. I still can't record 1080@60fps footage, but I can record 1080p footage if I downscale to 1760x990, which is a sort-of workaround i'm willing to share.

I did some 1080@60fps downscaled to 720p@60fps recording on twitch while playing Hitman to test if Twitch would even accept the stream settings. Interestingly Rise Of The Tomb Raider absolutely refused to be recorded with VA-API at any resolution or frame rate.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Jun 6, 2018

I found something interesting.

My encoder overloads with window capture (xcomposite) and any capture device I use like my magewell, however i was able to use screen capture (xshm) and record at 1080p60. The encoder would not give me the overloaded warning, but frames would drop under heavy load, especially with tessellation on or very high textures. Eventually I DID get it to overload during a texture change from high to very high in rise of the tomb raider. Frames dropped under heavy load but i didnt get a warning for it unlike the other two mentioned.

@jp9000 any idea why window capture or video capture device might cause the issue but screen capture doesnt?

@Readek

This comment has been minimized.

Copy link

Readek commented Jun 6, 2018

Can't replicate. Encoder still overloads, and well, it was overloading anyways with no sources added (1080@60fps only, RX480)

@p4block

This comment has been minimized.

Copy link

p4block commented Jun 9, 2018

On Vega10 hardware, only HEVC encoder is present (at least for now, but it may be due to hardware reasons)

OBS built with this patch will refuse to work as (obviously) h264 encoding is not present.

However, the option is still incorrectly shown. Would it be possible to add HEVC encode, or check if the hardware supports h264 first?

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Jun 9, 2018

@p4block

This comment has been minimized.

Copy link

p4block commented Jun 10, 2018

Sorry for the badly worded comment, what I meant is that with the current vaapi stack, vega10 only has an HVEC encoder available, (no h264 entrypoint slice). When using obs with this patch, h264 encoding is shown as available for the vaapi encoder when it's impossible for it to work.

2400G doesn't have the same VCE as desktop vega, it's actually much newer.

@GloriousEggroll

This comment has been minimized.

Copy link
Contributor Author

GloriousEggroll commented Jun 10, 2018

@kkartaltepe

This comment has been minimized.

Copy link
Contributor

kkartaltepe commented Jun 10, 2018

Highly recommend running perf and just looking at the flamegraph of obs after a streaming session where you get encoder overload this will help you locate where the actual time is being spent. If it is wasted in memcpy it's probably not the encoders fault.

(or otherwise instrumenting the process, you could also use OBS's existing performance tooling to simply break down areas you are interested as well).

@jp9000

This comment has been minimized.

Copy link
Member

jp9000 commented Jul 18, 2018

Sorry for having delayed this. I did actually end up looking at it, but I had an issue with compiling and headers, despite having the required package -- I'll give a detailed report when I get back to this, but have been a bit overworked recently.

@cRaZy-bisCuiT

This comment has been minimized.

Copy link

cRaZy-bisCuiT commented Jul 25, 2018

Any news on this? Could we expect a merge into mainline any time soon? It would be amazing to have an official release including vaapi support for Linux soon. :)

@dodgepong

This comment has been minimized.

Copy link
Contributor

dodgepong commented Jul 25, 2018

Jim's comment from last week is the latest news on this issue.

@ericfont

This comment has been minimized.

Copy link

ericfont commented Jul 25, 2018

I couldn't compile because of two compile errors in: UI/frontend-plugins/frontend-tools/scripts.cpp

obs-studio/UI/frontend-plugins/frontend-tools/scripts.cpp:551:8: error: invalid use of incomplete type ‘class QAction’ action->connect(action, &QAction::triggered, cb);

obs-studio/UI/frontend-plugins/frontend-tools/scripts.cpp:551:36: error: incomplete type ‘QAction’ used in nested name specifier action->connect(action, &QAction::triggered, cb);

however, that was easily fixed by adding #include <QAction> to the top of scripts.cpp and then compiled and ran fine.

I've tested streaming and recording and I have observed that my GPU is being utilized when I select the VAAPI FFMPEG encoder. Good work...I'm looking forward to this PR being merged.

@RytoEX

This comment has been minimized.

Copy link
Contributor

RytoEX commented Jul 25, 2018

@ericfont That's a Qt compile error when compiling with Qt 5.11+, which OBS hasn't switched to just yet. That will be addressed separate from this PR. Actually, that specific change is already in UI/frontend-plugins/frontend-tools/scripts.cpp on master, but does need to be addressed elsewhere.

@ericfont

This comment has been minimized.

Copy link

ericfont commented Jul 27, 2018

Thanks. Twitch w/ vaapi 264 worked for me, but Facebook didn't...obs gave me a strange error message about being unsupported even though it is still 264 format. I'm guessing that is related to a comment 4 months ago:

"That patch had a bunch of codec-specific profiles (such as h264-main) which did not match the naming conventions for profiles in OBS' "Enforce streaming service encoder settings" feature, which would choose a profile of "main" for Facebook for example - which would get rejected by the driver as invalid causing a fairly generic "unimplemented" error."

If it indeed such a simple problem, then I hope that can be resolved...

@Entropy512

This comment has been minimized.

Copy link

Entropy512 commented Jul 27, 2018

@ericfont Last time I had looked at the most recent patchset, it had been simplified significantly to the point where it should have resolved that issue - but I need to look again. At least for Facebook, the workaround is to disable that "Enforce encoder settings" option - this patch defaults to a "main" profile accepted by Facebook anyway.

I haven't tested this patchset in a while. Will need to try again next time I'm in a scenario favorable for livestreaming - the problem is that the music venue I used to stream a lot from due to it having a combination of frequent good acts AND amazing upstream bandwidth now has a fairly nasty upstream cap, apparently their deal with a local ISP that offered fiber and wireless service to get amazing fiber bandwidth in exchange for roof space to put that ISP's WISP antennas expired. Now if anyone livestreams from their phone, the bandwidth cap gets hit and everyone's streams start flaking out.

@cRaZy-bisCuiT

This comment has been minimized.

Copy link

cRaZy-bisCuiT commented Aug 13, 2018

Is there a major blocker not to merge this patch right now?

@dribbleondo

This comment has been minimized.

Copy link

dribbleondo commented Aug 13, 2018

I think the compiling issue was fixed when the QT compile error was fixed a few days ago.

@kkartaltepe

This comment has been minimized.

Copy link
Contributor

kkartaltepe commented Aug 13, 2018

It did not get picked up for 22, so thats why nothing has happened for the past few weeks. Hopefully we can see some renewed responses after 22 is released.

@cRaZy-bisCuiT

This comment has been minimized.

Copy link

cRaZy-bisCuiT commented Aug 14, 2018

I think it would be pretty nice to have a merge into mainline soon. @GloriousEggroll is a very talented programmer (if you check out his projects on his Gitlab) so I'm sure he'll maintain / improve this while he can. :)

@dodgepong

This comment has been minimized.

Copy link
Contributor

dodgepong commented Aug 14, 2018

I know you want this really badly, but this is too big of a change for it to be merged in before v22. Give Jim a chance to release the new version, at least, and eventually he will have time to take a closer look at this PR.

@dribbleondo

This comment has been minimized.

Copy link

dribbleondo commented Aug 16, 2018

I posted a while ago an alternative PPA to provide FFMPEG 4 (which fixed a few issues with video encoding so long as it isn't 1080p or you're playing ROTTR). In a similar vein, here's Nate Carlson's FFMPEG 4 PPA for Bionic Distro's. as I had problems getting FFMPEG to work via manually compiling.

admshao added a commit to admshao/obs-studio that referenced this pull request Aug 19, 2018

UI: Fix mixer context menu toggling layout on kde
This is a weird one. On KDE just clicking in the options or right
clicking the empty space areas of the mixer dock would trigger a layout
change.

This fixes mantis obsproject#1256
@nexfwall

This comment has been minimized.

Copy link

nexfwall commented Sep 11, 2018

I made a test build for local testing. Added this PR.patch to RPMFusion obs-studio package, without any changes and built for Fedora 28. This PR was applied to 22.0.3 version of OBS without any errors and rpm package built successful from the first time.

The results are amazing, but it wasn't easy to find how to use it.

I don't know why it's still delayed while it's working well. For such results in encoding speed on Linux these PR deserves some love. It kills two birds with one stone. Adding Intel QSV and AMD GPU acceleration at once.

@kkartaltepe

This comment has been minimized.

Copy link
Contributor

kkartaltepe commented Sep 12, 2018

I'm going to count this PR as unmaintained at this point. But I have opened another PR #1482 to take care of any outstanding issues which may come up.

@jp9000

This comment has been minimized.

Copy link
Member

jp9000 commented Sep 25, 2018

Hey there GloriousEggroll, apologies for pining you on here, just wanted to let you know what was going on before I merge your code. I finally managed to get my network sorted so I can use my linux machine properly again. Just wanted to let you know that kkartaltepe didn't mean any harm -- he didn't try to change any of your code, he just cleaned the styling, fixed a magic number, and that's about it.

As for the header issue I had, it was because ubuntu 16.04 only had FFmpeg 2.8, and the libavutil/hwcontext.h file you used required FFmpeg 3.1 minimum, so that's why it didn't compile for me at the time.

So, what I did was I fixed up the 3.1 requirement, and fixed up the additional warnings. I tested it and the encoder appears to work great.

I will be merging your PR shortly with my additional fixes -- you'll see "closed from [merged commit]".

Thank you for making this, great work.

@jp9000 jp9000 closed this in a64ae11 Sep 25, 2018

Andersama added a commit to Andersama/obs-studio that referenced this pull request Jan 3, 2019

UI: Fix mixer context menu toggling layout on kde
This is a weird one. On KDE just clicking in the options or right
clicking the empty space areas of the mixer dock would trigger a layout
change.

This fixes mantis obsproject#1256

Andersama added a commit to Andersama/obs-studio that referenced this pull request Jan 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment