AMF: rate control improvements

[psyke83]

Description

Changes to AMF encoder:

• Add user-adjustable enforce_hrd option and set by default, to help constrain the target bitrate for VBR/CBR rate control methods. Rationale for making user-selectable: I observed encoder throttling with enforce_hrd on RX 570 several years ago at 1080p or higher, but cannot re-test to ensure the issue is not still present. May also cause quality degradation for scene transitions that previously would trigger bitrate overshoots.
• Set CBR as default RC. User testing seems to suggest that CBR + HRD gives the best results for bitrate constraints.

Issues Fixed or Closed

Closes #1040

Type of Change

• [x] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Dependency update (updates to dependencies)
• [x] Documentation update (changes to documentation)
• [ ] Repository update (changes to repository files, e.g. .github/...)

Checklist

• [x] My code follows the style guidelines of this project
• [x] I have performed a self-review of my own code
• [x] I have commented my code, particularly in hard-to-understand areas
• [x] I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch must be updated before it can be merged. You must also Allow edits from maintainers.

• [x] I want maintainers to keep my branch updated

[ReenigneArcher]

Homebrew is failing because your default branch doesn't match what we have set, nightly

https://github.com/LizardByte/Sunshine/actions/runs/8274317116/job/22639486783?pr=2251#step:7:209

You can fix this by changing your default branch in your fork, and we can re-run the workflow.

[psyke83]

Homebrew is failing because your default branch doesn't match what we have set, nightly

https://github.com/LizardByte/Sunshine/actions/runs/8274317116/job/22639486783?pr=2251#step:7:209

You can fix this by changing your default branch in your fork, and we can re-run the workflow.

Thanks - that sorted the errors. I had to refresh the PR again, as I overlooked the preset RC set in the video struct. Everything should be OK now.

[psyke83]

I've been testing CBR + HRD on my RX 6600 for a few days, and haven't noticed any issues with reduced quality or peak bitrate spikes, but I'm wondering if I should use this PR to update some more features related to AMF?

Looking at the reference version of ffmpeg we're currently building against:

1. There's a new usage preset - lowlatency_high_quality - but this is only present for H264 and HEVC.
2. There are new rate control methods that work across all three codecs. Looking at the description on the AMF wiki, the HQ-VBR methods will not be useful in Sunshine, as they use a numerical quality target instead of a target bitrate. HQ-CBR might be worth exposing, with the caveat that the wiki recommends that this rate control method should be used with HRD and filler data, so exposing filler data as an option might be a good idea.
3. Continuing from the last point, the wiki also suggests that HRD and filler data should be enabled together for the standard CBR rate control. Perhaps this is another justification to expose the filler data option?
4. Perhaps it might be a good idea to add some description text to at least some of the web UI entries to bring it to parity with NVENC?

[psyke83]

Update on my previous notes:

• HQCBR seems to flat-out break Sunshine. Perhaps it's not worth troubleshooting the issue; the wiki says it works best with a large CPB, so it may not be useful for low latency streaming.
• The AMF usage presets were not being parsed correctly on my system (Usage / HevcUsage was always 0 - transcoding - regardless of what I selected), which turns out to be a bug in the usage_from_view() conversion. Added a fix for the issue. I wonder if this is related to the ultralowlatency issue that @cgutman recently worked around; i.e. it was really failing from the transcoding profile, or perhaps an undefined value received from usage_from_view was parsed differently?
• I've gone ahead and added the new usage profiles, as I don't see any reason to withold these choices from users (and one in particular is a new low latency profile).
• I still haven't added descriptions to the AMF web UI entries, but I can do so if anyone agrees that it's worthwhile.

[psyke83]

Just a note on the new usage presets: I removed the high_quality preset, as it breaks Sunshine for the same reason as hqcbr; both enable the PreAnalysis (not preencode) feature.

From looking at the HEVC API, the usage presets set the following:

• AMF_VIDEO_ENCODER_HEVC_LOWLATENCY_MODE
• AMF_VIDEO_ENCODER_HEVC_PRE_ANALYSIS_ENABLE
• AMF_VIDEO_ENCODER_HEVC_PEAK_BITRATE
• AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD
• AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_SKIP_FRAME_ENABLE
• AMF_VIDEO_ENCODER_HEVC_VBV_BUFFER_SIZE
• AMF_VIDEO_ENCODER_HEVC_ENFORCE_HRD
• AMF_VIDEO_ENCODER_HEVC_PREENCODE_ENABLE
• AMF_VIDEO_ENCODER_HEVC_ENABLE_VBAQ
• AMF_VIDEO_ENCODER_HEVC_HIGH_MOTION_QUALITY_BOOST_ENABLE
• AMF_VIDEO_ENCODER_HEVC_GOP_SIZE
• AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET
• AMF_VIDEO_ENCODER_HEVC_QUERY_TIMEOUT

Many of these properties are overridden by our configuration, and others by ffmpeg -- even if the default is not changed. Example: the quality option sets AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET - and will always override the usage preset's value, even if we weren't currently passing the quality option via Sunshine. The high_quality usage preset is breaking specifically due to AMF_VIDEO_ENCODER_HEVC_PRE_ANALYSIS_ENABLE being enabled, and that's one of the few properties that isn't explicitly set via ffmpeg.

The lowlatency_high_quality usage preset changes properties that otherwise can't be set via Sunshine (and aren't overwritten by ffmpeg):

• AMF_VIDEO_ENCODER_HEVC_LOWLATENCY_MODE (enabled; ultralowlatency is the only other profile that sets this)
• ~~AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_SKIP_FRAME_ENABLE (disabled; all other profiles except transcoding enable this)~~
• AMF_VIDEO_ENCODER_HEVC_HIGH_MOTION_QUALITY_BOOST_ENABLE (enabled; all other profiles set this to disabled)

We could probably fix the high_quality usage preset by hardcoding preanalysis to false, but I'm not sure if it's worthwhile. The hqcbr RC won't work at all without PreAnalysis enabled, so that's a non-starter.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 6.11%. Comparing base (2af0ce3) to head (330f1c5).

Additional details and impacted files

@@            Coverage Diff             @@
##           nightly   #2251      +/-   ##
==========================================
+ Coverage     6.00%   6.11%   +0.11%     
==========================================
  Files           85      85              
  Lines        18296   18303       +7     
  Branches      8310    8319       +9     
==========================================
+ Hits          1098    1119      +21     
- Misses       15468   16076     +608     
+ Partials      1730    1108     -622     

Flag	Coverage Δ
Linux	4.11% <0.00%> (-0.01%)	:arrow_down:
Windows	1.51% <0.00%> (-0.01%)	:arrow_down:
macOS-12	8.24% <9.09%> (?)
macOS-13	7.44% <9.09%> (-0.01%)	:arrow_down:
macOS-14	7.72% <9.09%> (+<0.01%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/config.h	0.00% <ø> (ø)
src/video.cpp	24.11% <ø> (+1.14%)	:arrow_up:
src/config.cpp	5.35% <9.09%> (+0.11%)	:arrow_up:

... and 27 files with indirect coverage changes

[psyke83]

Only change would be to remove any changes to localization files other than en.json. I need to make this more clear in the docs, but it should be ONLY en.json.

I assume the rationale is so that missing strings from other languages use the en fallback, correct?

I've made the requested change and tested a local build, and I can see that this will cause issues with the current state of the localization files - specifically amd_rc_cbr and amd_rc_vbr_latency. Setting the UI to any other language than "English" will show the outdated strings that are currently present in all other files.

The solution would be to delete the outdated strings from all other files besides en.json. Shall I do that?

[ReenigneArcher]

Essentially, the en.json file is the source. The rest are updated by crowdin automatically -> https://github.com/LizardByte/Sunshine/pull/2290

If files other than en.json are modified, there will be a few issues.

• Crowdin won't detect these
• I would have to rebase the automated PR, and that would be difficult given the different languages and the way it does the commits.

But yes, they may be outdated for a period of time, until I merge the automated updates from crowdin.

[psyke83]

Essentially, the en.json file is the source. The rest are updated by crowdin automatically -> #2290

If files other than en.json are modified, there will be a few issues.

• Crowdin won't detect these
• I would have rebase the automated PR, and that would be difficult given the different languages and the way it does the commits.

But yes, they may be outdated for a period of time, until I merge the automated updates from crowdin.

I see, thanks for explaining. I've pushed the updated PR that only touches en.json, as requested.