vo_gpu, options: don't return NaN through API

Internally, vo_gpu uses NaN for some options to indicate a default value
that is different depending on the context (e.g. different scalers).
There are 2 problems with this:

1. you couldn't reset the options to their defaults
2. NaN is a damn mess and shouldn't be part of the API

The option parser already rejected NaN explicitly, which is why 1.
didn't work. Regarding 2., JSON might be a good example, and actually
caused a bug report.

Fix this by mapping NaN to the special value "default". I think I'd
prefer other mechanisms (maybe just having every scaler expose separate
options?), but for now this will do. See you in a future commit, which
painfully deprecates this and replaces it with something else.

I refrained from using "no" (my favorite magic value for "unset" etc.)
because then I'd have e.g. make --no-scale-param1 work, which in
addition to a lot of effort looks dumb and nobody will use it.

Here's also an apology for the shitty added test script.

Fixes: #6691
This commit is contained in:
wm4 2019-10-25 00:25:05 +02:00
parent b0827b4dc4
commit 77f309c94f
6 changed files with 91 additions and 19 deletions

View File

@ -191,6 +191,9 @@ Interface changes
internal counter that remembered the current position. internal counter that remembered the current position.
- remove deprecated ao/vo auto profiles. Consider using scripts like - remove deprecated ao/vo auto profiles. Consider using scripts like
auto-profiles.lua instead. auto-profiles.lua instead.
- --[c]scale-[w]param[1|2] and --tone-mapping-param now accept "default",
and if set to that value, reading them as property will also return
"default", instead of float nan as in previous versions
--- mpv 0.28.0 --- --- mpv 0.28.0 ---
- rename --hwdec=mediacodec option to mediacodec-copy, to reflect - rename --hwdec=mediacodec option to mediacodec-copy, to reflect
conventions followed by other hardware video decoding APIs conventions followed by other hardware video decoding APIs

View File

@ -4404,8 +4404,10 @@ The following video options are currently all specific to ``--vo=gpu`` and
smooth. smooth.
``--scale-param1=<value>``, ``--scale-param2=<value>``, ``--cscale-param1=<value>``, ``--cscale-param2=<value>``, ``--dscale-param1=<value>``, ``--dscale-param2=<value>``, ``--tscale-param1=<value>``, ``--tscale-param2=<value>`` ``--scale-param1=<value>``, ``--scale-param2=<value>``, ``--cscale-param1=<value>``, ``--cscale-param2=<value>``, ``--dscale-param1=<value>``, ``--dscale-param2=<value>``, ``--tscale-param1=<value>``, ``--tscale-param2=<value>``
Set filter parameters. Ignored if the filter is not tunable. Currently, Set filter parameters. By default, these are set to the special string
this affects the following filter parameters: ``default``, which maps to a scaler-specific default value. Ignored if the
filter is not tunable. Currently, this affects the following filter
parameters:
bcspline bcspline
Spline parameters (``B`` and ``C``). Defaults to 0.5 for both. Spline parameters (``B`` and ``C``). Defaults to 0.5 for both.
@ -4477,8 +4479,10 @@ The following video options are currently all specific to ``--vo=gpu`` and
``--scale-wparam=<window>``, ``--cscale-wparam=<window>``, ``--cscale-wparam=<window>``, ``--tscale-wparam=<window>`` ``--scale-wparam=<window>``, ``--cscale-wparam=<window>``, ``--cscale-wparam=<window>``, ``--tscale-wparam=<window>``
(Advanced users only) Configure the parameter for the window function given (Advanced users only) Configure the parameter for the window function given
by ``--scale-window`` etc. Ignored if the window is not tunable. Currently, by ``--scale-window`` etc. By default, these are set to the special string
this affects the following window parameters: ``default``, which maps to a window-specific default value. Ignored if the
window is not tunable. Currently, this affects the following window
parameters:
kaiser kaiser
Window parameter (alpha). Defaults to 6.33. Window parameter (alpha). Defaults to 6.33.
@ -5524,8 +5528,10 @@ The following video options are currently all specific to ``--vo=gpu`` and
the display. the display.
``--tone-mapping-param=<value>`` ``--tone-mapping-param=<value>``
Set tone mapping parameters. Ignored if the tone mapping algorithm is not Set tone mapping parameters. By default, this is set to the special string
tunable. This affects the following tone mapping algorithms: ``default``, which maps to an algorithm-specific default value. Ignored if
the tone mapping algorithm is not tunable. This affects the following tone
mapping algorithms:
clip clip
Specifies an extra linear coefficient to multiply into the signal Specifies an extra linear coefficient to multiply into the signal

37
TOOLS/lua/nan-test.lua Normal file
View File

@ -0,0 +1,37 @@
-- Test a float property which internally uses NaN.
-- Run with --no-config (or just scale-param1 not set).
local utils = require 'mp.utils'
prop_name = "scale-param1"
-- internal NaN, return string "default" instead of NaN
v = mp.get_property_native(prop_name, "fail")
print("Exp:", "string", "\"default\"")
print("Got:", type(v), utils.to_string(v))
v = mp.get_property(prop_name)
print("Exp:", "default")
print("Got:", v)
-- not representable -> return provided fallback value
v = mp.get_property_number(prop_name, -100)
print("Exp:", -100)
print("Got:", v)
mp.set_property_native(prop_name, 123)
v = mp.get_property_number(prop_name, -100)
print("Exp:", "number", 123)
print("Got:", type(v), utils.to_string(v))
-- try to set an actual NaN
st, msg = mp.set_property_number(prop_name, 0.0/0)
print("Exp:", nil, "<message>")
print("Got:", st, msg)
-- set default
mp.set_property(prop_name, "default")
v = mp.get_property(prop_name)
print("Exp:", "default")
print("Got:", v)

View File

@ -918,6 +918,11 @@ static int parse_double(struct mp_log *log, const m_option_t *opt,
if (bstr_eatstart0(&rest, ":") || bstr_eatstart0(&rest, "/")) if (bstr_eatstart0(&rest, ":") || bstr_eatstart0(&rest, "/"))
tmp_float /= bstrtod(rest, &rest); tmp_float /= bstrtod(rest, &rest);
if ((opt->flags & M_OPT_DEFAULT_NAN) && bstr_equals0(param, "default")) {
tmp_float = NAN;
goto done;
}
if (rest.len) { if (rest.len) {
mp_err(log, "The %.*s option must be a floating point number or a " mp_err(log, "The %.*s option must be a floating point number or a "
"ratio (numerator[:/]denominator): %.*s\n", "ratio (numerator[:/]denominator): %.*s\n",
@ -931,6 +936,7 @@ static int parse_double(struct mp_log *log, const m_option_t *opt,
return M_OPT_OUT_OF_RANGE; return M_OPT_OUT_OF_RANGE;
} }
done:
if (dst) if (dst)
VAL(dst) = tmp_float; VAL(dst) = tmp_float;
return 1; return 1;
@ -938,12 +944,18 @@ static int parse_double(struct mp_log *log, const m_option_t *opt,
static char *print_double(const m_option_t *opt, const void *val) static char *print_double(const m_option_t *opt, const void *val)
{ {
return talloc_asprintf(NULL, "%f", VAL(val)); double f = VAL(val);
if (isnan(f) && (opt->flags & M_OPT_DEFAULT_NAN))
return talloc_strdup(NULL, "default");
return talloc_asprintf(NULL, "%f", f);
} }
static char *print_double_f3(const m_option_t *opt, const void *val) static char *print_double_f3(const m_option_t *opt, const void *val)
{ {
return talloc_asprintf(NULL, "%.3f", VAL(val)); double f = VAL(val);
if (isnan(f))
return print_double(opt, val);
return talloc_asprintf(NULL, "%.3f", f);
} }
static void add_double(const m_option_t *opt, void *val, double add, bool wrap) static void add_double(const m_option_t *opt, void *val, double add, bool wrap)
@ -989,8 +1001,14 @@ static int double_set(const m_option_t *opt, void *dst, struct mpv_node *src)
static int double_get(const m_option_t *opt, void *ta_parent, static int double_get(const m_option_t *opt, void *ta_parent,
struct mpv_node *dst, void *src) struct mpv_node *dst, void *src)
{ {
dst->format = MPV_FORMAT_DOUBLE; double f = *(double *)src;
dst->u.double_ = *(double *)src; if (isnan(f) && (opt->flags & M_OPT_DEFAULT_NAN)) {
dst->format = MPV_FORMAT_STRING;
dst->u.string = talloc_strdup(ta_parent, "default");
} else {
dst->format = MPV_FORMAT_DOUBLE;
dst->u.double_ = f;
}
return 1; return 1;
} }
@ -1023,12 +1041,14 @@ static int parse_float(struct mp_log *log, const m_option_t *opt,
static char *print_float(const m_option_t *opt, const void *val) static char *print_float(const m_option_t *opt, const void *val)
{ {
return talloc_asprintf(NULL, "%f", VAL(val)); double tmp = VAL(val);
return print_double(opt, &tmp);
} }
static char *print_float_f3(const m_option_t *opt, const void *val) static char *print_float_f3(const m_option_t *opt, const void *val)
{ {
return talloc_asprintf(NULL, "%.3f", VAL(val)); double tmp = VAL(val);
return print_double_f3(opt, &tmp);
} }
static void add_float(const m_option_t *opt, void *val, double add, bool wrap) static void add_float(const m_option_t *opt, void *val, double add, bool wrap)
@ -1057,9 +1077,8 @@ static int float_set(const m_option_t *opt, void *dst, struct mpv_node *src)
static int float_get(const m_option_t *opt, void *ta_parent, static int float_get(const m_option_t *opt, void *ta_parent,
struct mpv_node *dst, void *src) struct mpv_node *dst, void *src)
{ {
dst->format = MPV_FORMAT_DOUBLE; double tmp = VAL(src);
dst->u.double_ = VAL(src); return double_get(opt, ta_parent, dst, &tmp);
return 1;
} }
const m_option_type_t m_option_type_float = { const m_option_type_t m_option_type_float = {

View File

@ -424,6 +424,9 @@ char *format_file_size(int64_t size);
#define UPDATE_OPTS_MASK \ #define UPDATE_OPTS_MASK \
(((UPDATE_OPT_LAST << 1) - 1) & ~(unsigned)(UPDATE_OPT_FIRST - 1)) (((UPDATE_OPT_LAST << 1) - 1) & ~(unsigned)(UPDATE_OPT_FIRST - 1))
// type_float/type_double: string "default" is parsed as NaN (and reverse)
#define M_OPT_DEFAULT_NAN (1 << 29)
// Like M_OPT_TYPE_OPTIONAL_PARAM. // Like M_OPT_TYPE_OPTIONAL_PARAM.
#define M_OPT_OPTIONAL_PARAM (1 << 30) #define M_OPT_OPTIONAL_PARAM (1 << 30)

View File

@ -342,14 +342,18 @@ static int validate_error_diffusion_opt(struct mp_log *log, const m_option_t *op
#define OPT_BASE_STRUCT struct gl_video_opts #define OPT_BASE_STRUCT struct gl_video_opts
// Use for options which use NAN for defaults.
#define OPT_FLOATDEF(name, var, flags) \
OPT_FLOAT(name, var, (flags) | M_OPT_DEFAULT_NAN)
#define SCALER_OPTS(n, i) \ #define SCALER_OPTS(n, i) \
OPT_STRING_VALIDATE(n, scaler[i].kernel.name, 0, validate_scaler_opt), \ OPT_STRING_VALIDATE(n, scaler[i].kernel.name, 0, validate_scaler_opt), \
OPT_FLOAT(n"-param1", scaler[i].kernel.params[0], 0), \ OPT_FLOATDEF(n"-param1", scaler[i].kernel.params[0], 0), \
OPT_FLOAT(n"-param2", scaler[i].kernel.params[1], 0), \ OPT_FLOATDEF(n"-param2", scaler[i].kernel.params[1], 0), \
OPT_FLOAT(n"-blur", scaler[i].kernel.blur, 0), \ OPT_FLOAT(n"-blur", scaler[i].kernel.blur, 0), \
OPT_FLOATRANGE(n"-cutoff", scaler[i].cutoff, 0, 0.0, 1.0), \ OPT_FLOATRANGE(n"-cutoff", scaler[i].cutoff, 0, 0.0, 1.0), \
OPT_FLOATRANGE(n"-taper", scaler[i].kernel.taper, 0, 0.0, 1.0), \ OPT_FLOATRANGE(n"-taper", scaler[i].kernel.taper, 0, 0.0, 1.0), \
OPT_FLOAT(n"-wparam", scaler[i].window.params[0], 0), \ OPT_FLOATDEF(n"-wparam", scaler[i].window.params[0], 0), \
OPT_FLOAT(n"-wblur", scaler[i].window.blur, 0), \ OPT_FLOAT(n"-wblur", scaler[i].window.blur, 0), \
OPT_FLOATRANGE(n"-wtaper", scaler[i].window.taper, 0, 0.0, 1.0), \ OPT_FLOATRANGE(n"-wtaper", scaler[i].window.taper, 0, 0.0, 1.0), \
OPT_FLOATRANGE(n"-clamp", scaler[i].clamp, 0, 0.0, 1.0), \ OPT_FLOATRANGE(n"-clamp", scaler[i].clamp, 0, 0.0, 1.0), \
@ -383,7 +387,7 @@ const struct m_sub_options gl_video_conf = {
tone_map.scene_threshold_low, 0, 0, 20.0), tone_map.scene_threshold_low, 0, 0, 20.0),
OPT_FLOATRANGE("hdr-scene-threshold-high", OPT_FLOATRANGE("hdr-scene-threshold-high",
tone_map.scene_threshold_high, 0, 0, 20.0), tone_map.scene_threshold_high, 0, 0, 20.0),
OPT_FLOAT("tone-mapping-param", tone_map.curve_param, 0), OPT_FLOATDEF("tone-mapping-param", tone_map.curve_param, 0),
OPT_FLOATRANGE("tone-mapping-max-boost", tone_map.max_boost, 0, 1.0, 10.0), OPT_FLOATRANGE("tone-mapping-max-boost", tone_map.max_boost, 0, 1.0, 10.0),
OPT_FLOAT("tone-mapping-desaturate", tone_map.desat, 0), OPT_FLOAT("tone-mapping-desaturate", tone_map.desat, 0),
OPT_FLOATRANGE("tone-mapping-desaturate-exponent", OPT_FLOATRANGE("tone-mapping-desaturate-exponent",