Skip to content
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

NVENC Refactor #10536

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

NVENC Refactor #10536

wants to merge 4 commits into from

Conversation

derrod
Copy link
Member

@derrod derrod commented Apr 14, 2024

Description

Note

This pull request is targeted for the next major version as it breaks some backwards compatibility (mainly Kepler GPUs).

  • Adds refactored native NVENC implementation in separate obs-nvenc module
  • Removes old native NVENC implementation and disables FFmpeg NVENC by default.

Features

  1. Improved Capability Checks
    • obs-nvenc-test[.exe] now works on both Linux and Windows
    • Properties are now limited to what the hardware supports, making the silent retry/fallback unnecessary
    • Manual GPU blacklist is no longer required
  2. New SDK (12.2) features
    • Split encoding (driver version 555 or higher) on dual-NVENC GPUs to run slower presets at higher resolutions, or achieve even higher frame rates at low resolutions
    • Ultra High Quality preset for HEVC (Turing+)
  3. Additional features from old SDKs
    • B-Frames as reference to increase quality at practically no performance impact
    • Target Quality VBR mode (CQVBR) as an alternative to CQP mode
    • Maximum b-frames are now accurate (e.g. on Ada: 4 for H.264, 5 for HEVC, 7 for AV1)
  4. Custom options field
    • Allows changing a number of often requested options such as AQ strength or lookahead depth without requiring more properties elements*
    • Should help people migrate away from third-party plugins
  5. Backwards compatibility
    • Old encoder IDs will stay functional until we decide to migrate users automatically

* Current selection of options is mostly for experimentation, it will probably be cut down to only a few that are sensible but don't warrant their own UI elements.

Motivation and Context

  • Remove compatibility hack which has become a maintenance burden
  • Remove NVENC code from FFmpeg module which it no longer depends on
  • Support latest NVIDIA SDK and drivers
  • Improved readability and maintainability

How Has This Been Tested?

  • RTX 4090 on Windows 10
  • RTX 4090 on openSUSE Tumbleweed

Note

This plugin is only compatible with the current cmake revision (3.0) on Linux and Windows, and will not work with legacy cmake.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Tweak (non-breaking change to improve existing functionality)
  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@derrod derrod added Seeking Testers Build artifacts on CI Windows Affects Windows Linux Affects Linux labels Apr 14, 2024
@derrod derrod force-pushed the nvenc-rework branch 3 times, most recently from e546c7a to af3ab45 Compare April 16, 2024 08:35
@derrod
Copy link
Member Author

derrod commented Apr 16, 2024

Updated with some cleanup and included a53e4b1 to see if Flatpak will work.

@derrod derrod force-pushed the nvenc-rework branch 3 times, most recently from c23552d to 981d3a9 Compare April 16, 2024 19:50
@Bleuzen
Copy link

Bleuzen commented Apr 18, 2024

Somehow this never uses the texture encoder in my testing so far

info: [obs-nvenc] nv12/p010 not active, falling back to non-texture encoder

This always prints when I start recording. Note that I do use NV12, also tried different settings, nothing helped

Also the Lossless mode is broken
First it says

error: [obs-nvenc] init_encoder_h264: nv.nvEncInitializeEncoder(enc->session, &enc->params) failed: 8 (NV_ENC_ERR_INVALID_PARAM): Adaptive quantization is not supported with lossless encoding

which is easy to fix but would be nice if OBS could automatically disable (& hide if possible) the adaptive quantization option if lossless is selected

but even after disabling that it does error out with

error: [obs-nvenc] init_encoder_h264: nv.nvEncInitializeEncoder(enc->session, &enc->params) failed: 8 (NV_ENC_ERR_INVALID_PARAM): With lossless preset, RC Mode / Profile not supported with qpPrimeYZeroTransformBypassFlag.
error: Already in non_texture encoder, can't fall back further!

@RytoEX
Copy link
Member

RytoEX commented Apr 18, 2024

info: [obs-nvenc] nv12/p010 not active, falling back to non-texture encoder

Please see #10567.

@derrod
Copy link
Member Author

derrod commented Apr 19, 2024

I should be able to fix the lossless thing relatively easily. And as Ryan mentioned texture encoders will need #10567 merged to work again, but that should be done soon™ so I can rebase this PR.

@Bleuzen
Copy link

Bleuzen commented Apr 19, 2024

Please see #10567.

Can confirm it works with this

@derrod
Copy link
Member Author

derrod commented Apr 20, 2024

Linux texture encoding and lossless mode for H.264 should work again now.

@Lain-B Lain-B self-assigned this Apr 20, 2024
@derrod derrod force-pushed the nvenc-rework branch 2 times, most recently from d6a4b06 to de34c28 Compare April 21, 2024 01:47
@wanhongqing123
Copy link

This code is beautiful

* So don't blame me for that. */

#define APPLY_BIT_OPT(opt_name, bits) \
if (strcmp(opt->name, #opt_name) == 0) { \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this be an inline function (per the comment above)? The macro is used quite often below which means that none of those steps can be debugged (except in raw assembly)?

Copy link
Member Author

@derrod derrod Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't get a pointer to a specific value in a bitfield. Trying to do so is actually a compiler error: https://godbolt.org/z/or31z8saW

I'm also not really concerned about the debugability of setting a value on a struct member. This is a trivial macro to save some boilerplate that exists in the same file and doesn't do any recursion. It can be easily expaned in-place by your IDE if you really need to step through it.

@derrod derrod force-pushed the nvenc-rework branch 3 times, most recently from 1c96fbf to 06be1b9 Compare April 30, 2024 00:14
@sandr1x
Copy link

sandr1x commented May 5, 2024

I checked the operation of B-frames as Reference via Elecard StramEye. For H.264 and HEVC everything works correctly. Disabled/middle/each modes

disableBadapt=1 disabled adaptive b-frames for P6,7 presets for H.264. Thank you! I think people may have difficulty where to take the arguments. Maybe add information in the manuals?

@Bleuzen
Copy link

Bleuzen commented May 8, 2024

1.)
In CQVBR mode it is possible to set the maximum bitrate to 0 in the UI

Thought it would probably just disable the max bitrate and use only the target quality to control the bitrate without a cap

Instead setting the max bitrate to 0 seems to cap it at some default value (depending on codec, resolution & fps), for example for 1440p 60 FPS HEVC it limits to 20 Mb/s:

... while it does go above those 20 Mb/s if the max bitrate is non-zero and higher.
A bit confusion from a users perspective, so probably either:

  • disable the max bitrate when it is set to 0 (would prefer this)
  • or set a lower limit for this so that the max bitrate could not be set to 0

2.)
It would be cool to also log the max bitrate when starting a stream/recording

info: [obs-nvenc: 'advanced_video_recording'] settings:
        codec:        AV1
        rate_control: CQVBR
        bitrate:      8000
--->    max_bitrate:      <---
        cq/cqp:       0

3.)
In the UI I think it makes more sense to swap these:

Because

  • target quality is the main feature separating this mode
  • it always confuses my brain when switching rate control often since the max bitrate is at another place in VBR mode

@derrod
Copy link
Member Author

derrod commented May 8, 2024

Being able to set it to 0 is valid, and it should set it to 0 in the NVENC configuration as well. NVIDIA limits the bitrate in target quality mode even with max bitrate set to 0, this limit is based on the level. Seems for HEVC this ends up being much lower than H.264. It's one of the reasons we kept CQP around.

@Bleuzen
Copy link

Bleuzen commented May 8, 2024

Being able to set it to 0 is valid, and it should set it to 0 in the NVENC configuration as well.

I do agree from a technical perspective. However from a users perspective this is bad. You wouldn't expect a bitrate cap if max bitrate is set to 0 since there is nothing clearly indicating a fixed limit in this case. This could ruin recordings (bad quality) for new users who do not know this background info so I try to think of how this could be improved...

What do you think of extending the label

which is similar to the keyframe interval label

this would at least make it clear that there is a limit so I consider this more user friendly

NVIDIA limits the bitrate in target quality mode even with max bitrate set to 0, this limit is based on the level.

yeah just found that Handbrake and ffmpeg do the same with no specific level set

sorry if this is a dumb question (the level thing is new to me) but is there a downside if OBS would just use a higher level? (or allow the users to configure it)

Seems for HEVC this ends up being much lower

AV1 is also affected

It's one of the reasons we kept CQP around.

I mean, CQ mode is not too bad, as long as you set a high enough level or max bitrate. And please always keep CQP around too since it can be the better choice depending on the content

@sandr1x
Copy link

sandr1x commented May 18, 2024

This is a test build of obs, but it is unstable. In New World, I noticed that obs was unable to provide a stable framerate. If you start recording, the encoder is overloaded and stuttering occurs. With version 30 everything works fine.
2024-05-18_061755

@derrod derrod force-pushed the nvenc-rework branch 2 times, most recently from 3262366 to 7252f2b Compare May 28, 2024 09:37
@derrod derrod marked this pull request as ready for review June 9, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linux Affects Linux Seeking Testers Build artifacts on CI Windows Affects Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants