locale: Text labels for some options #13

Merged
UserNaem merged 4 commits from patch-1 into master 2019-11-10 08:09:41 +01:00
UserNaem commented 2019-11-05 00:49:21 +01:00 (Migrated from github.com)

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:

  • 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 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. <!--- Please fill out the following template, which will help other contributors review your Pull Request. --> <!--- Make sure you’ve read the contribution guidelines --> ### 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 <!--- What types of changes does your PR introduce? Uncomment all that apply --> <!--- - Bug fix (non-breaking change which fixes an issue) --> <!--- - New feature (non-breaking change which adds functionality) --> <!--- - Performance enhancement (non-breaking change which improves efficiency) --> <!--- - Code cleanup (non-breaking change which makes code smaller or more readable) --> <!--- - Breaking change (fix or feature that would cause existing functionality to change) --> Documentation (a change to documentation pages) ### Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] My code has been run through [clang-format](https://github.com/obsproject/obs-studio/blob/master/.clang-format). - [x] I have read the [**contributing** document](https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.rst). - [x] My code is not on the master branch. - [x] The code has been tested. - [ ] All commit messages are properly formatted and commits squashed where appropriate.
Xaymar (Migrated from github.com) requested changes 2019-11-05 14:22:13 +01:00
Xaymar (Migrated from github.com) left a comment

Some changes required. Let's not make the tooltip overly complex, as it may have to be translated into other languages.

Some changes required. Let's not make the tooltip overly complex, as it may have to be translated into other languages.
@@ -46,6 +46,8 @@ KeyFrames="Key Frames"
KeyFrames.IntervalType="Interval Type"
Xaymar (Migrated from github.com) commented 2019-11-05 14:15:03 +01:00

Description text is incorrect, higher values are not correlated with more efficient compression. Higher values may cause less efficient compression due to no proper reference frame being left. Information about video editors should also not be added here as it is specific to certain video editors and not a general observation that applies to all of them.

Description text is incorrect, higher values are not correlated with more efficient compression. Higher values may cause less efficient compression due to no proper reference frame being left. Information about video editors should also not be added here as it is specific to certain video editors and not a general observation that applies to all of them.
@@ -55,7 +57,9 @@ Codec.H264.Profile.baseline="Baseline"
Codec.H264.Profile.main="Main"
Xaymar (Migrated from github.com) commented 2019-11-05 14:15:50 +01:00

Recommendations for profiles should not be included. Information about "modern devices" is also not necessary here.

Recommendations for profiles should not be included. Information about "modern devices" is also not necessary here.
Xaymar (Migrated from github.com) commented 2019-11-05 14:17:10 +01:00

The last statement is unconfirmed, and thus should be removed. There are encoders that willingly pick higher levels in order to give themselves more room or enable features that can better compress things.

The last statement is unconfirmed, and thus should be removed. There are encoders that willingly pick higher levels in order to give themselves more room or enable features that can better compress things.
Xaymar (Migrated from github.com) commented 2019-11-05 14:17:19 +01:00

Comments should not be added.

Comments should not be added.
Xaymar (Migrated from github.com) commented 2019-11-05 14:17:27 +01:00

Comments should not be added.

Comments should not be added.
Xaymar (Migrated from github.com) commented 2019-11-05 14:17:42 +01:00

See Codec.H264.Level.Description.

See Codec.H264.Level.Description.
@@ -79,6 +84,7 @@ Codec.ProRes.Profile.AP4X="4444 Extra Quality/XQ (AP4X)"
# NVENC
NVENC.Preset="Preset"
Xaymar (Migrated from github.com) commented 2019-11-05 14:18:35 +01:00

"for NVENC" can be excluded here as this will only show up on NVENC.

"for NVENC" can be excluded here as this will only show up on NVENC.
@@ -93,12 +99,19 @@ NVENC.Preset.Lossless="Lossless"
NVENC.Preset.LosslessHighPerformance="Lossless High Performance"
NVENC.RateControl="Rate Control Options"
NVENC.RateControl.Mode="Mode"
Xaymar (Migrated from github.com) commented 2019-11-05 14:20:08 +01:00

Please split the description here up into NVENC.RateControl.Mode.XXX.Description, as having one huge description breaks the layout on smaller monitors or high DPI monitors.

Please split the description here up into NVENC.RateControl.Mode.XXX.Description, as having one huge description breaks the layout on smaller monitors or high DPI monitors.
@@ -102,3 +114,4 @@
NVENC.RateControl.Mode.CBR_LD_HQ.Description="Constant Bitrate optimized for lowest encoding latency."
NVENC.RateControl.LookAhead="Look Ahead"
NVENC.RateControl.LookAhead.Description="Look ahead this many frames while encoding to better distribute bitrate.\nImproves quality slightly at the cost of some GPU time.\nSet to 0 to disable."
NVENC.RateControl.AdaptiveI="Enable adaptive I-Frame insertion"
Xaymar (Migrated from github.com) commented 2019-11-05 14:20:19 +01:00

No comments.

No comments.
Xaymar (Migrated from github.com) commented 2019-11-05 14:21:22 +01:00

Turing NVENC restriction should be moved to the end of the tooltip.

Turing NVENC restriction should be moved to the end of the tooltip.
Xaymar (Migrated from github.com) commented 2019-11-05 14:21:32 +01:00

Yes, that is possible.

Yes, that is possible.
Xaymar (Migrated from github.com) commented 2019-11-05 14:21:43 +01:00

See previous no comments.

See previous no comments.
Xaymar (Migrated from github.com) reviewed 2019-11-05 14:23:12 +01:00
@@ -46,6 +46,8 @@ KeyFrames="Key Frames"
KeyFrames.IntervalType="Interval Type"
Xaymar (Migrated from github.com) commented 2019-11-05 14:23:11 +01:00

For example, Resolve works perfectly fine with keyframes every 15 seconds, but Adobe Premiere Pro struggles with it.

For example, Resolve works perfectly fine with keyframes every 15 seconds, but Adobe Premiere Pro struggles with it.
UserNaem (Migrated from github.com) reviewed 2019-11-06 02:38:23 +01:00
UserNaem (Migrated from github.com) commented 2019-11-06 02:38:23 +01:00

How to do this, exactly? I tried "" and it still shows the same tooltip, I tried " " and it shows a tooltip with a whitespace character.

How to do this, exactly? I tried `""` and it still shows the same tooltip, I tried `" "` and it shows a tooltip with a whitespace character.
UserNaem commented 2019-11-06 06:05:42 +01:00 (Migrated from github.com)

Should be good now, I hope.

Should be good now, I hope.
Xaymar (Migrated from github.com) reviewed 2019-11-07 09:32:33 +01:00
Xaymar (Migrated from github.com) commented 2019-11-07 09:32:32 +01:00

I just have to remove the code in the ui handler for it.

I just have to remove the code in the ui handler for it.
Xaymar (Migrated from github.com) requested changes 2019-11-07 09:52:07 +01:00
Xaymar (Migrated from github.com) left a comment

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.

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.
@@ -46,6 +46,8 @@ KeyFrames="Key Frames"
KeyFrames.IntervalType="Interval Type"
Xaymar (Migrated from github.com) commented 2019-11-07 09:37:30 +01:00

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.

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.
@@ -93,12 +99,19 @@ NVENC.Preset.Lossless="Lossless"
NVENC.Preset.LosslessHighPerformance="Lossless High Performance"
Xaymar (Migrated from github.com) commented 2019-11-07 09:41:48 +01:00

\nThis yields the highest quality-per-bitrate. since this depends on the users settings, don't include it.

`\nThis yields the highest quality-per-bitrate.` since this depends on the users settings, don't include it.
Xaymar (Migrated from github.com) commented 2019-11-07 09:44:01 +01:00

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.

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.
Xaymar (Migrated from github.com) commented 2019-11-07 09:51:01 +01:00

, mainly by disabling B-frames and using slice multithreading does not apply to NVENC unless you know how all NVENC chips work internally.

`, mainly by disabling B-frames and using slice multithreading` does not apply to NVENC unless you know how all NVENC chips work internally.
Xaymar (Migrated from github.com) commented 2019-11-07 09:51:26 +01:00

, in kilobits per second is part of the general UI, there should be a kbit/s suffix in the field.

`, in kilobits per second` is part of the general UI, there should be a kbit/s suffix in the field.
UserNaem (Migrated from github.com) reviewed 2019-11-10 00:45:46 +01:00
@@ -93,12 +99,19 @@ NVENC.Preset.Lossless="Lossless"
NVENC.Preset.LosslessHighPerformance="Lossless High Performance"
UserNaem (Migrated from github.com) commented 2019-11-10 00:45:46 +01:00

I can't think of a good way to explain this myself, so I'll go with your description.

I can't think of a good way to explain this myself, so I'll go with your description.
UserNaem (Migrated from github.com) reviewed 2019-11-10 00:46:30 +01:00
UserNaem (Migrated from github.com) commented 2019-11-10 00:46:30 +01:00

This leaves me with a tooltip that says "Target bitrate." I'll remove that, too.

This leaves me with a tooltip that says "Target bitrate." I'll remove that, too.
UserNaem (Migrated from github.com) reviewed 2019-11-10 00:50:36 +01:00
@@ -46,6 +46,8 @@ KeyFrames="Key Frames"
KeyFrames.IntervalType="Interval Type"
UserNaem (Migrated from github.com) commented 2019-11-10 00:50:36 +01:00

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.

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.
UserNaem commented 2019-11-10 01:35:45 +01:00 (Migrated from github.com)

Updated again.

Updated again.
Xaymar (Migrated from github.com) approved these changes 2019-11-10 08:09:05 +01:00
Xaymar (Migrated from github.com) reviewed 2019-11-10 08:30:27 +01:00
@@ -46,6 +46,8 @@ KeyFrames="Key Frames"
KeyFrames.IntervalType="Interval Type"
Xaymar (Migrated from github.com) commented 2019-11-10 08:30:27 +01:00

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.

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.
Sign in to join this conversation.