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

obs-nvenc: Fix lookahead depth value logging #11816

Merged

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Feb 2, 2025

Description

Log the final lookaheadDepth value after all calculations, instead of an intermediate value. (For Nvenc.)

Motivation and Context

Without this change, a value gets logged based on an internal variable rc_lookahead that's not necessarily equal to the actual, final value that gets used for the encode. (e.g. It can get bounded to a lower value afterward without updating this internal variable.)

Would like to show users the actual value of lookaheadDepth that's being used for their encode session, or at least show what's actually set on the encode session. (It's a bit unclear which values the driver actually respects / meaningfully responds to.)

Thanks to @Penwy for pointing out this discrepancy.

How Has This Been Tested?

  • I set lookaheadDepth=9999 in the "Custom Encoder Options" field of an "NVIDIA NVENC H.264" encoder in the Advanced Output tab. (This is a knowingly "too-high" value.)
  • I then started a recording and checked the logs to see if the correct value was logged, knowing the obs-nvenc plugin's code internally bounds this value to within some reasonable limits.

Before:

lookahead:    true (9999 frames)
[ . . . ]
user opts:    lookaheadDepth=9999

After:

lookahead:    true (56 frames)
[ . . . ]
user opts:    lookaheadDepth=9999

Note the "true (56 frames)" instead of "true (9999 frames)". This (56) is the actual value the plugin has limited/sanitized the lookaheadDepth value to. So, this change causes the correct (bounded) final value to be logged. Doing it this way should now survive any refactoring of the above "intermediate state"/input-sanitizing code as well.

Tested on Windows 10 on a 3070, Studio Driver 561.09, FWIW.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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.

This was logging an intermediate value, rather than the final lookahead
depth value after all calculations. Log the final value instead.
@WizardCM WizardCM added the Bug Fix Non-breaking change which fixes an issue label Feb 2, 2025
@Lain-B
Copy link
Collaborator

Lain-B commented Feb 2, 2025

Only commenting on the formatting real quick: the build would fail if it wasn't formatted correctly, so you got the formatting right.

Copy link
Collaborator

@Lain-B Lain-B left a comment

Choose a reason for hiding this comment

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

Seems fine to me. You agree @derrod?

@Lain-B Lain-B merged commit a8a349c into obsproject:master Feb 23, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants