0
0
mirror of https://github.com/obsproject/obs-studio.git synced 2024-09-20 13:08:50 +02:00

obs-x264: Refactor tokenizing of options

We do a bad job of handling errors in user-supplied x264 options. I want
to improve our error handling. To make my job easier, move the code for
parsing the x264 options string into its own function. Also, add some
tests for the functionality.

Aside from a minor tweak to a log message for the opencl option, this
commit should not change behavior.
This commit is contained in:
Matthew Glazar 2020-05-16 20:35:35 -07:00
parent 07ae6b4ca9
commit 40b4e32c41
5 changed files with 217 additions and 64 deletions

View File

@ -4,6 +4,18 @@ find_package(Libx264 REQUIRED)
include_directories(${LIBX264_INCLUDE_DIRS})
add_definitions(${LIBX264_DEFINITIONS})
set(obs-x264-util_HEADERS
obs-x264-options.h)
set(obs-x264-util_SOURCES
obs-x264-options.c)
add_library(obs-x264-util STATIC
${obs-x264-util_HEADERS}
${obs-x264-util_SOURCES})
target_link_libraries(obs-x264-util PRIVATE libobs)
set_target_properties(obs-x264-util PROPERTIES FOLDER "plugins")
set(obs-x264_SOURCES
obs-x264.c
obs-x264-plugin-main.c)
@ -16,10 +28,17 @@ if(WIN32)
endif()
add_library(obs-x264 MODULE
${obs-x264_HEADERS}
${obs-x264_SOURCES})
target_link_libraries(obs-x264
libobs
obs-x264-util
${LIBX264_LIBRARIES})
set_target_properties(obs-x264 PROPERTIES FOLDER "plugins")
install_obs_plugin_with_data(obs-x264 data)
add_executable(obs-x264-test obs-x264-test.c)
set_target_properties(obs-x264-test PROPERTIES FOLDER "plugins")
target_link_libraries(obs-x264-test PRIVATE libobs obs-x264-util)
add_test(NAME obs-x264-test COMMAND obs-x264-test)

View File

@ -0,0 +1,60 @@
#include <stdbool.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
#include <util/bmem.h>
#include <util/dstr.h>
#include "obs-x264-options.h"
static bool getparam(const char *param, char **name, const char **value)
{
const char *assign;
if (!param || !*param || (*param == '='))
return false;
assign = strchr(param, '=');
if (!assign || !*assign || !*(assign + 1))
return false;
*name = bstrdup_n(param, assign - param);
*value = assign + 1;
return true;
}
struct obs_x264_options obs_x264_parse_options(const char *options_string)
{
char **input_words = strlist_split(options_string, ' ', false);
if (!input_words) {
return (struct obs_x264_options){
.count = 0,
.options = NULL,
.input_words = NULL,
};
}
size_t input_option_count = 0;
for (char **input_word = input_words; *input_word; ++input_word)
input_option_count += 1;
struct obs_x264_option *out_options =
bmalloc(input_option_count * sizeof(*out_options));
struct obs_x264_option *out_option = out_options;
for (char **input_word = input_words; *input_word; ++input_word) {
if (getparam(*input_word, &out_option->name,
&out_option->value)) {
++out_option;
}
}
return (struct obs_x264_options){
.count = out_option - out_options,
.options = out_options,
.input_words = input_words,
};
}
void obs_x264_free_options(struct obs_x264_options options)
{
for (size_t i = 0; i < options.count; ++i) {
bfree(options.options[i].name);
}
strlist_free(options.input_words);
}

View File

@ -0,0 +1,17 @@
#pragma once
#include <stddef.h>
struct obs_x264_option {
char *name;
char *value;
};
struct obs_x264_options {
size_t count;
struct obs_x264_option *options;
char **input_words;
};
struct obs_x264_options obs_x264_parse_options(const char *options_string);
void obs_x264_free_options(struct obs_x264_options options);

View File

@ -0,0 +1,65 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "obs-x264-options.h"
#define CHECK(condition) \
do { \
if (!(condition)) { \
fprintf(stderr, "%s:%d: error: check failed: %s\n", \
__FILE__, __LINE__, #condition); \
exit(1); \
} \
} while (0)
static void test_obs_x264_parse_options()
{
struct obs_x264_options options;
options = obs_x264_parse_options(NULL);
CHECK(options.count == 0);
obs_x264_free_options(options);
options = obs_x264_parse_options("");
CHECK(options.count == 0);
obs_x264_free_options(options);
options = obs_x264_parse_options("ref=3");
CHECK(options.count == 1);
CHECK(strcmp(options.options[0].name, "ref") == 0);
CHECK(strcmp(options.options[0].value, "3") == 0);
obs_x264_free_options(options);
options = obs_x264_parse_options("ref=3 bframes=8");
CHECK(options.count == 2);
CHECK(strcmp(options.options[0].name, "ref") == 0);
CHECK(strcmp(options.options[0].value, "3") == 0);
CHECK(strcmp(options.options[1].name, "bframes") == 0);
CHECK(strcmp(options.options[1].value, "8") == 0);
obs_x264_free_options(options);
// Invalid options are dropped.
options = obs_x264_parse_options(
"ref=3 option_with_no_equal_sign bframes=8");
CHECK(options.count == 2);
CHECK(strcmp(options.options[0].name, "ref") == 0);
CHECK(strcmp(options.options[0].value, "3") == 0);
CHECK(strcmp(options.options[1].name, "bframes") == 0);
CHECK(strcmp(options.options[1].value, "8") == 0);
obs_x264_free_options(options);
// Extra whitespace is ignored between and around options.
options = obs_x264_parse_options(" ref=3 bframes=8 ");
CHECK(options.count == 2);
CHECK(strcmp(options.options[0].name, "ref") == 0);
CHECK(strcmp(options.options[0].value, "3") == 0);
CHECK(strcmp(options.options[1].name, "bframes") == 0);
CHECK(strcmp(options.options[1].value, "8") == 0);
obs_x264_free_options(options);
}
int main()
{
test_obs_x264_parse_options();
return 0;
}

View File

@ -20,6 +20,7 @@
#include <util/darray.h>
#include <util/platform.h>
#include <obs-module.h>
#include "obs-x264-options.h"
#ifndef _STDINT_H_INCLUDED
#define _STDINT_H_INCLUDED
@ -255,24 +256,23 @@ static const char *validate(struct obs_x264 *obsx264, const char *val,
return NULL;
}
static void override_base_param(struct obs_x264 *obsx264, const char *param,
char **preset, char **profile, char **tune)
static void override_base_param(struct obs_x264 *obsx264,
struct obs_x264_option option, char **preset,
char **profile, char **tune)
{
char *name;
const char *val;
if (getparam(param, &name, &val)) {
const char *name = option.name;
const char *val = option.value;
if (astrcmpi(name, "preset") == 0) {
const char *valid_name = validate(
obsx264, val, "preset", x264_preset_names);
const char *valid_name =
validate(obsx264, val, "preset", x264_preset_names);
if (valid_name) {
bfree(*preset);
*preset = bstrdup(val);
}
} else if (astrcmpi(name, "profile") == 0) {
const char *valid_name = validate(
obsx264, val, "profile", x264_profile_names);
const char *valid_name =
validate(obsx264, val, "profile", x264_profile_names);
if (valid_name) {
bfree(*profile);
*profile = bstrdup(val);
@ -286,41 +286,33 @@ static void override_base_param(struct obs_x264 *obsx264, const char *param,
*tune = bstrdup(val);
}
}
bfree(name);
}
}
static inline void override_base_params(struct obs_x264 *obsx264, char **params,
static inline void override_base_params(struct obs_x264 *obsx264,
const struct obs_x264_options *options,
char **preset, char **profile,
char **tune)
{
while (*params)
override_base_param(obsx264, *(params++), preset, profile,
tune);
for (size_t i = 0; i < options->count; ++i)
override_base_param(obsx264, options->options[i], preset,
profile, tune);
}
#define OPENCL_ALIAS "opencl_is_experimental_and_potentially_unstable"
static inline void set_param(struct obs_x264 *obsx264, const char *param)
static inline void set_param(struct obs_x264 *obsx264,
struct obs_x264_option option)
{
char *name;
const char *val;
if (getparam(param, &name, &val)) {
if (strcmp(name, "preset") != 0 &&
strcmp(name, "profile") != 0 && strcmp(name, "tune") != 0 &&
strcmp(name, "fps") != 0 &&
strcmp(name, "force-cfr") != 0 &&
strcmp(name, "width") != 0 && strcmp(name, "height") != 0 &&
strcmp(name, "opencl") != 0) {
if (strcmp(name, OPENCL_ALIAS) == 0)
strcpy(name, "opencl");
const char *name = option.name;
const char *val = option.value;
if (strcmp(name, "preset") != 0 && strcmp(name, "profile") != 0 &&
strcmp(name, "tune") != 0 && strcmp(name, "fps") != 0 &&
strcmp(name, "force-cfr") != 0 && strcmp(name, "width") != 0 &&
strcmp(name, "height") != 0 && strcmp(name, "opencl") != 0) {
if (strcmp(option.name, OPENCL_ALIAS) == 0)
name = "opencl";
if (x264_param_parse(&obsx264->params, name, val) != 0)
warn("x264 param: %s failed", param);
}
bfree(name);
warn("x264 param: %s=%s failed", name, val);
}
}
@ -398,7 +390,7 @@ enum rate_control {
};
static void update_params(struct obs_x264 *obsx264, obs_data_t *settings,
char **params, bool update)
const struct obs_x264_options *options, bool update)
{
video_t *video = obs_encoder_video(obsx264->encoder);
const struct video_output_info *voi = video_output_get_info(video);
@ -516,8 +508,8 @@ static void update_params(struct obs_x264 *obsx264, obs_data_t *settings,
else
obsx264->params.i_csp = X264_CSP_NV12;
while (*params)
set_param(obsx264, *(params++));
for (size_t i = 0; i < options->count; ++i)
set_param(obsx264, options->options[i]);
if (!update) {
info("settings:\n"
@ -543,18 +535,17 @@ static bool update_settings(struct obs_x264 *obsx264, obs_data_t *settings,
char *preset = bstrdup(obs_data_get_string(settings, "preset"));
char *profile = bstrdup(obs_data_get_string(settings, "profile"));
char *tune = bstrdup(obs_data_get_string(settings, "tune"));
const char *opts = obs_data_get_string(settings, "x264opts");
const char *options_string = obs_data_get_string(settings, "x264opts");
struct obs_x264_options options =
obs_x264_parse_options(options_string);
char **paramlist;
bool success = true;
paramlist = strlist_split(opts, ' ', false);
if (!update)
blog(LOG_INFO, "---------------------------------");
if (!obsx264->context) {
override_base_params(obsx264, paramlist, &preset, &profile,
override_base_params(obsx264, &options, &preset, &profile,
&tune);
if (preset && *preset)
@ -568,9 +559,10 @@ static bool update_settings(struct obs_x264 *obsx264, obs_data_t *settings,
}
if (success) {
update_params(obsx264, settings, paramlist, update);
if (opts && *opts && !update)
info("custom settings: %s", opts);
update_params(obsx264, settings, &options, update);
if (options.count > 0 && options_string && !update) {
info("custom settings: %s", options_string);
}
if (!obsx264->context)
apply_x264_profile(obsx264, profile);
@ -578,7 +570,7 @@ static bool update_settings(struct obs_x264 *obsx264, obs_data_t *settings,
obsx264->params.b_repeat_headers = false;
strlist_free(paramlist);
obs_x264_free_options(options);
bfree(preset);
bfree(profile);
bfree(tune);