-
-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
I don't know enough about HEVC tiers and "maximum bitrate" in VBR, so I left some "placeholder text" commented out. Also, not sure about line 158.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes required. Let's not make the tooltip overly complex, as it may have to be translated into other languages.
Should be good now, I hope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost perfect, just a few minor issues. On a side note, could you group the descriptions for the rate control modes with the names for the rate control modes? They're currently out of order.
data/locale/en-US.ini
Outdated
@@ -46,6 +46,8 @@ KeyFrames="Key Frames" | |||
KeyFrames.IntervalType="Interval Type" | |||
KeyFrames.IntervalType.Frames="Frames" | |||
KeyFrames.IntervalType.Seconds="Seconds" | |||
KeyFrames.IntervalType.Description="Keyframe interval type" | |||
KeyFrames.Interval.Description="Distance between key frames, in frames or seconds. Default is 2 seconds." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be best to remove the default value here, as it may change and can be better shown by OBS's UI code instead of being put into the tooltip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OBS' UI doesn't offer an immediately apparent way to unset any changed settings to default, so I thought putting a note somewhere "if you broke it, here's how to unbreak" would help, but if you intend on putting it in later, this won't be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but there's a need for a UI rework in OBS anyway. Including default values shouldn't be an issue once the decisions have been made there.
data/locale/en-US.ini
Outdated
@@ -93,7 +99,13 @@ NVENC.Preset.Lossless="Lossless" | |||
NVENC.Preset.LosslessHighPerformance="Lossless High Performance" | |||
NVENC.RateControl="Rate Control Options" | |||
NVENC.RateControl.Mode="Mode" | |||
NVENC.RateControl.Mode.CQP="Constant Quantization Parameter" | |||
NVENC.RateControl.Mode.Description="Rate control mode selection" | |||
NVENC.RateControl.Mode.CQP.Description="A flat compression ratio with no regard for bit rates.\nThis yields the highest quality-per-bitrate." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\nThis yields the highest quality-per-bitrate.
since this depends on the users settings, don't include it.
data/locale/en-US.ini
Outdated
NVENC.RateControl.Mode.CQP.Description="A flat compression ratio with no regard for bit rates.\nThis yields the highest quality-per-bitrate." | ||
NVENC.RateControl.Mode.VBR.Description="Sacrifices quality to stay below the upper bitrate limit,\nor saves bitrate where possible." | ||
NVENC.RateControl.Mode.VBR_HQ.Description="Variable Bitrate with two-pass encoding enabled by default." | ||
NVENC.RateControl.Mode.CBR.Description="Sacrifices quality in high-motion scenes and performs bit stuffing\nin low-motion scenes to maintain a constant bitrate." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should actually describe what CBR does, not how it works.
For example:
Compresses footage so that it matches the target bitrate over the duration of one second. This comes at a cost in quality during high motion scenes or scenes with flickering brightness like often seen in RPGs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a good way to explain this myself, so I'll go with your description.
data/locale/en-US.ini
Outdated
NVENC.RateControl.Mode.VBR_HQ.Description="Variable Bitrate with two-pass encoding enabled by default." | ||
NVENC.RateControl.Mode.CBR.Description="Sacrifices quality in high-motion scenes and performs bit stuffing\nin low-motion scenes to maintain a constant bitrate." | ||
NVENC.RateControl.Mode.CBR_HQ.Description="Constant Bitrate with two-pass encoding enabled by default." | ||
NVENC.RateControl.Mode.CBR_LD_HQ.Description="Constant Bitrate optimized for lowest encoding latency, mainly by disabling B-frames and using slice multithreading." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, mainly by disabling B-frames and using slice multithreading
does not apply to NVENC unless you know how all NVENC chips work internally.
data/locale/en-US.ini
Outdated
@@ -109,6 +121,7 @@ NVENC.RateControl.TwoPass="Enable Two Pass" | |||
NVENC.RateControl.TwoPass.Description="Enable a secondary pass for encoding, which can help with quality and bitrate stability.\nImproves quality slightly at the cost of some GPU time.\nNvidia Turing hardware might actually see a quality degrade from this." | |||
NVENC.RateControl.Bitrate="Bitrate Limits" | |||
NVENC.RateControl.Bitrate.Target="Target Bitrate" | |||
NVENC.RateControl.Bitrate.Target.Description="Target bitrate, in kilobits per second." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, in kilobits per second
is part of the general UI, there should be a kbit/s suffix in the field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leaves me with a tooltip that says "Target bitrate." I'll remove that, too.
Updated again. |
I don't know enough about HEVC tiers and "maximum bitrate" in VBR, so I left some "placeholder text" commented out. Also, not sure about line 158.
Description
Added some explanations where I could, in hopes of them being useful. Also added commented-out placeholder texts for missing labels.
Motivation and Context
Issue #11
How Has This Been Tested?
Works on my machine
Types of changes
Documentation (a change to documentation pages)
Checklist: