xaymar/obs-ffmpeg-encoder
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 14 Wiki Activity

Gpu field in software encoders #15

Merged
kryztoval merged 2 commits from v0.4-gpu_field into v0.4 2019-11-11 19:46:25 +01:00
Conversation 5 Commits 2 Files Changed 2
+15
kryztoval commented 2019-11-08 21:57:49 +01:00 (Migrated from github.com)
Copy Link
Copy Source

Description

locale: Added the text for the label and description for the GPU field.
source/encoder.cpp: Added the label, entry point, settings and rules to enable GPU selection on the field.
image
source/encoder.cpp: Added error look up to show a more understandable message to the user

Motivation and Context

Finding out what parameters to use in the custom field has always been a complicated task https://github.com/Xaymar/obs-ffmpeg-encoder/issues/12 even in the Custom ffmpeg plugin that comes standard with OBS. Since this plug in is meant to solve the complications out of configuring the NVENC encoders in OBS, as such it felt at home to add the possiblity to select the GPU from a simple to use interface.

How Has This Been Tested?

I have used those changes in a local OBS setup

Types of changes

New feature

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.
### Description locale: Added the text for the label and description for the GPU field. source/encoder.cpp: Added the label, entry point, settings and rules to enable GPU selection on the field. ![image](https://user-images.githubusercontent.com/5733028/68506463-515d2a00-022f-11ea-86a9-eb66ffd98083.png) source/encoder.cpp: Added error look up to show a more understandable message to the user ### Motivation and Context Finding out what parameters to use in the custom field has always been a complicated task https://github.com/Xaymar/obs-ffmpeg-encoder/issues/12 even in the Custom ffmpeg plugin that comes standard with OBS. Since this plug in is meant to solve the complications out of configuring the NVENC encoders in OBS, as such it felt at home to add the possiblity to select the GPU from a simple to use interface. ### How Has This Been Tested? I have used those changes in a local OBS setup ### Types of changes New feature ### Checklist: - [x] 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. - [x] All commit messages are properly formatted and commits squashed where appropriate.
Xaymar (Migrated from github.com) requested changes 2019-11-10 08:13:38 +01:00
Xaymar (Migrated from github.com) left a comment
Copy Link
Copy Source

Minor changes, but otherwise fine.

Minor changes, but otherwise fine.
data/locale/en-US.ini
@@ -20,6 +20,9 @@ FFmpeg.StandardCompliance.Strict="Strict"
FFmpeg.StandardCompliance.Normal="Normal"
Xaymar (Migrated from github.com) commented 2019-11-10 08:10:05 +01:00
Copy Link
Copy Source

GPU should be uppercase as "gpu" is not a word.

GPU should be uppercase as "gpu" is not a word.
Xaymar (Migrated from github.com) commented 2019-11-10 08:10:14 +01:00
Copy Link
Copy Source

Same as other comment.

Same as other comment.
source/encoder.cpp
@@ -64,6 +64,7 @@ extern "C" {
#define ST_FFMPEG_THREADS "FFmpeg.Threads"
Xaymar (Migrated from github.com) commented 2019-11-10 08:10:24 +01:00
Copy Link
Copy Source

Same as other comment.

Same as other comment.
source/encoder.cpp
@@ -555,6 +557,10 @@ void obsffmpeg::encoder_factory::get_properties(obs_properties_t* props, bool hw
obs_property_set_long_description(p, TRANSLATE(DESC(ST_FFMPEG_CUSTOMSETTINGS)));
Xaymar (Migrated from github.com) commented 2019-11-10 08:12:18 +01:00
Copy Link
Copy Source

Should be limited to uint8_t::max instead. I doubt someone will get 32676 GPUs in a single system - even with the best PCI-E splitters, you'd at most only get 192 to 256 GPUs.

Should be limited to uint8_t::max instead. I doubt someone will get 32676 GPUs in a single system - even with the best PCI-E splitters, you'd at most only get 192 to 256 GPUs.
source/encoder.cpp
@@ -930,6 +939,9 @@ bool obsffmpeg::encoder::update(obs_data_t* settings)
_context->keyint_min = _context->gop_size;
Xaymar (Migrated from github.com) commented 2019-11-10 08:13:29 +01:00
Copy Link
Copy Source

Set options on _context with AV_OPT_SEARCH_CHILDREN. It works better if the encoder and FFmpeg have the same property.

Set options on _context with AV_OPT_SEARCH_CHILDREN. It works better if the encoder and FFmpeg have the same property.
kryztoval (Migrated from github.com) reviewed 2019-11-10 08:31:15 +01:00
source/encoder.cpp
@@ -555,6 +557,10 @@ void obsffmpeg::encoder_factory::get_properties(obs_properties_t* props, bool hw
obs_property_set_long_description(p, TRANSLATE(DESC(ST_FFMPEG_CUSTOMSETTINGS)));
kryztoval (Migrated from github.com) commented 2019-11-10 08:31:15 +01:00
Copy Link
Copy Source

In theory the maximum number of GPUs you can get are 13 straight GPUs but only 9 of those can be nVidia. I like 256 value :)

but this value was lifted from the obs-ffmpeg standard plugin, though sure I will change it and update this.

In theory the maximum number of GPUs you can get are 13 straight GPUs but only 9 of those can be nVidia. I like 256 value :) but this value was lifted from the obs-ffmpeg standard plugin, though sure I will change it and update this.
kryztoval (Migrated from github.com) reviewed 2019-11-10 08:32:37 +01:00
source/encoder.cpp
@@ -930,6 +939,9 @@ bool obsffmpeg::encoder::update(obs_data_t* settings)
_context->keyint_min = _context->gop_size;
kryztoval (Migrated from github.com) commented 2019-11-10 08:32:37 +01:00
Copy Link
Copy Source

This was also based upon the obs-ffmpeg standard plugin, but sure, I'll set the Search clildren option.

Should this point to _context instead of _context->priv_data

This was also based upon the obs-ffmpeg standard plugin, but sure, I'll set the Search clildren option. Should this point to `_context` instead of `_context->priv_data`
Xaymar (Migrated from github.com) reviewed 2019-11-10 11:36:05 +01:00
source/encoder.cpp
@@ -930,6 +939,9 @@ bool obsffmpeg::encoder::update(obs_data_t* settings)
_context->keyint_min = _context->gop_size;
Xaymar (Migrated from github.com) commented 2019-11-10 11:36:05 +01:00
Copy Link
Copy Source

Yeah it should point to _context. AV_OPT_SEARCH_CHILDREN makes it descend into _context->priv_data by itself, so when options move from encoder to ffmpeg or ffmpeg to encoder things still work as expected.

Yeah it should point to _context. AV_OPT_SEARCH_CHILDREN makes it descend into _context->priv_data by itself, so when options move from encoder to ffmpeg or ffmpeg to encoder things still work as expected.
Xaymar (Migrated from github.com) approved these changes 2019-11-11 19:44:25 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
kryztoval
Xaymar
Labels
Clear labels
good first issue
Good for newcomers
 status:duplicate
This issue or pull request already exists
 status:help wanted
Extra attention is needed
 status:invalid
This doesn't seem right
 status:wontfix
This will not be worked on
 type:bug
Functionality is broken to some degree.
 type:enhancement
New feature or request
 type:question
Generic questions about behavior and functionality.
 type:regression
Behavior that used to work and now doesn't.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
xaymar
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: xaymar/obs-ffmpeg-encoder#15
Delete Branch "v0.4-gpu_field"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?