Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Gpu field in software encoders #15

Merged
merged 2 commits into from Nov 11, 2019
Merged

Conversation

kryztoval
Copy link
Contributor

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.

source/encoder.cpp: Added GPU field selection to Software encoders
locale: Added label and description for GPU field
Copy link
Owner

@Xaymar Xaymar left a comment

Choose a reason for hiding this comment

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

Minor changes, but otherwise fine.

@@ -20,6 +20,9 @@ FFmpeg.StandardCompliance.Strict="Strict"
FFmpeg.StandardCompliance.Normal="Normal"
FFmpeg.StandardCompliance.Unofficial="Unofficial"
FFmpeg.StandardCompliance.Experimental="Experimental"
FFmpeg.gpu="GPU"
Copy link
Owner

Choose a reason for hiding this comment

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

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

@@ -20,6 +20,9 @@ FFmpeg.StandardCompliance.Strict="Strict"
FFmpeg.StandardCompliance.Normal="Normal"
FFmpeg.StandardCompliance.Unofficial="Unofficial"
FFmpeg.StandardCompliance.Experimental="Experimental"
FFmpeg.gpu="GPU"
FFmpeg.gpu.Description="For multiple GPU, selects which gpu to use as the main encoder"
Copy link
Owner

Choose a reason for hiding this comment

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

Same as other comment.

@@ -64,6 +64,7 @@ extern "C" {
#define ST_FFMPEG_THREADS "FFmpeg.Threads"
#define ST_FFMPEG_COLORFORMAT "FFmpeg.ColorFormat"
#define ST_FFMPEG_STANDARDCOMPLIANCE "FFmpeg.StandardCompliance"
#define ST_FFMPEG_GPU "FFmpeg.gpu"
Copy link
Owner

Choose a reason for hiding this comment

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

Same as other comment.

@@ -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)));
}
if (!hw_encode) {
{
auto p = obs_properties_add_int(grp, ST_FFMPEG_GPU, TRANSLATE(ST_FFMPEG_GPU), 0, std::numeric_limits<int16_t>::max(), 1);
Copy link
Owner

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@kryztoval kryztoval Nov 10, 2019

Choose a reason for hiding this comment

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

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.

@@ -930,6 +939,9 @@ bool obsffmpeg::encoder::update(obs_data_t* settings)
_context->keyint_min = _context->gop_size;
}

if (!_hwinst)
av_opt_set_int(_context->priv_data, "gpu", (int)obs_data_get_int(settings, ST_FFMPEG_GPU), 0);
Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@kryztoval kryztoval Nov 10, 2019

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

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.

locale: Matched variable case and corrected narration a little bit
@Xaymar Xaymar merged commit 8b515be into Xaymar:v0.4 Nov 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants