From c78482045444c488bb7948305d583a55d17cd236 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 14 Mar 2020 02:07:35 +0100 Subject: [PATCH] options: introduce bool option type, use it for --fullscreen The option code is very old and was added to MPlayer in the early 2000s, when C99 was still new. MPlayer did not use the "bool" type anywhere,l and the logical option equivalent to bool, the "flag" option type, used int, with the convention that only the values 0 and 1 are allowed. mpv may have hammered many, many additional tentacles to the option code, but some of the basics never changed, and m_option_type_flag still uses int. This seems a bit weird, since mpv uses bool for booleans. So finally introduce an m_option_type_bool. To avoid duplicating too much code, change the flag code to bool, and "reimplement" m_option_type_flag on top of m_option_type_bool. As a "demonstration", change the --fullscreen option to this new type. Ideally, all options would be changed too bool, and m_option_type_flag would be removed. But that is a lot of monotonous thankless work, so I'm not doing it, and making it a painful years long transition. At the same time, I'm introducing a new concept for option declarations. Instead of OPT_BOOL(), which define the full m_option struct contents, there's OPTF_BOOL(), which only takes the option field name itself. The name is provided via a normal struct field initializer. Other fields (such as flags) can be provided via designated initializers. The advantage of this is that we don't need tons of nested vararg macros. We also don't need to deal with 0-sized varargs being a pain (and in fact they are not a thing in standard C99 and probably C11). There is no need to provide a mandatory flags argument either, which is the reason why so many OPT_ macros are used with a "0" argument. (The flag argument seems to confuse other developers; they either don't immediately recognize what it is, and sometimes it's supposed to be the option's default value.) Not having to mess with the flag argument in such option macros is also a reason for the removal of M_OPT_RANGE etc., for the better or worse. The only place that special-cased the _flag option type was in command.c; change it to use something effectively very similar that automatically includes the new _bool option type. Everything else should be transparent to the change. The fullscreen option change should be transparent too, as C99 bool is basically an integer type that is clamped to 0/1 (except in Swift, Swift sucks). --- options/m_option.c | 75 ++++++++++++++++++++++++++++++++---- options/m_option.h | 8 ++++ options/options.c | 2 +- options/options.h | 2 +- osdep/macos/mpv_helper.swift | 2 +- player/command.c | 2 +- 6 files changed, 80 insertions(+), 11 deletions(-) diff --git a/options/m_option.c b/options/m_option.c index c31fececdf..e0cc804914 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -118,11 +118,11 @@ static void copy_opt(const m_option_t *opt, void *dst, const void *src) memcpy(dst, src, opt->type->size); } -// Flag +// Bool -#define VAL(x) (*(int *)(x)) +#define VAL(x) (*(bool *)(x)) -static int parse_flag(struct mp_log *log, const m_option_t *opt, +static int parse_bool(struct mp_log *log, const m_option_t *opt, struct bstr name, struct bstr param, void *dst) { if (bstr_equals0(param, "yes") || !param.len) { @@ -149,12 +149,12 @@ static int parse_flag(struct mp_log *log, const m_option_t *opt, return is_help ? M_OPT_EXIT : M_OPT_INVALID; } -static char *print_flag(const m_option_t *opt, const void *val) +static char *print_bool(const m_option_t *opt, const void *val) { return talloc_strdup(NULL, VAL(val) ? "yes" : "no"); } -static void add_flag(const m_option_t *opt, void *val, double add, bool wrap) +static void add_bool(const m_option_t *opt, void *val, double add, bool wrap) { if (fabs(add) < 0.5) return; @@ -163,7 +163,7 @@ static void add_flag(const m_option_t *opt, void *val, double add, bool wrap) VAL(val) = state ? 1 : 0; } -static int flag_set(const m_option_t *opt, void *dst, struct mpv_node *src) +static int bool_set(const m_option_t *opt, void *dst, struct mpv_node *src) { if (src->format != MPV_FORMAT_FLAG) return M_OPT_UNKNOWN; @@ -171,7 +171,7 @@ static int flag_set(const m_option_t *opt, void *dst, struct mpv_node *src) return 1; } -static int flag_get(const m_option_t *opt, void *ta_parent, +static int bool_get(const m_option_t *opt, void *ta_parent, struct mpv_node *dst, void *src) { dst->format = MPV_FORMAT_FLAG; @@ -179,6 +179,67 @@ static int flag_get(const m_option_t *opt, void *ta_parent, return 1; } +static bool bool_equal(const m_option_t *opt, void *a, void *b) +{ + return VAL(a) == VAL(b); +} + +const m_option_type_t m_option_type_bool = { + .name = "Flag", // same as m_option_type_flag; transparent to user + .size = sizeof(bool), + .flags = M_OPT_TYPE_OPTIONAL_PARAM | M_OPT_TYPE_CHOICE, + .parse = parse_bool, + .print = print_bool, + .copy = copy_opt, + .add = add_bool, + .set = bool_set, + .get = bool_get, + .equal = bool_equal, +}; + +#undef VAL + +// Flag + +#define VAL(x) (*(int *)(x)) + +static int parse_flag(struct mp_log *log, const m_option_t *opt, + struct bstr name, struct bstr param, void *dst) +{ + bool bdst = false; + int r = parse_bool(log, opt, name, param, &bdst); + if (dst) + VAL(dst) = bdst; + return r; +} + +static char *print_flag(const m_option_t *opt, const void *val) +{ + return print_bool(opt, &(bool){VAL(val)}); +} + +static void add_flag(const m_option_t *opt, void *val, double add, bool wrap) +{ + bool bval = VAL(val); + add_bool(opt, &bval, add, wrap); + VAL(val) = bval; +} + +static int flag_set(const m_option_t *opt, void *dst, struct mpv_node *src) +{ + bool bdst = false; + int r = bool_set(opt, &bdst, src); + if (r >= 0) + VAL(dst) = bdst; + return r; +} + +static int flag_get(const m_option_t *opt, void *ta_parent, + struct mpv_node *dst, void *src) +{ + return bool_get(opt, ta_parent, dst, &(bool){VAL(src)}); +} + static bool flag_equal(const m_option_t *opt, void *a, void *b) { return VAL(a) == VAL(b); diff --git a/options/m_option.h b/options/m_option.h index 8d7063c212..97383237a5 100644 --- a/options/m_option.h +++ b/options/m_option.h @@ -38,6 +38,7 @@ struct mpv_global; ///////////////////////////// Options types declarations //////////////////// // Simple types +extern const m_option_type_t m_option_type_bool; extern const m_option_type_t m_option_type_flag; extern const m_option_type_t m_option_type_dummy_flag; extern const m_option_type_t m_option_type_int; @@ -226,6 +227,7 @@ struct m_sub_options { // through this union. It serves for self-documentation and to get minimal // size/alignment requirements for option values in general. union m_option_value { + bool bool_; int flag; // not the C type "bool"! int int_; int64_t int64; @@ -352,6 +354,7 @@ struct m_option_type { // Option description struct m_option { // Option name. + // Option declarations can use this as positional field. const char *name; // Option type. @@ -587,6 +590,10 @@ extern const char m_option_path_separator; #define OPT_HELPER_REMOVEPAREN(...) __VA_ARGS__ +#define OPTF_BOOL(field) \ + .type = &m_option_type_bool, \ + .offset = MP_CHECKED_OFFSETOF(OPT_BASE_STRUCT, field, bool), + /* The OPT_SOMETHING->OPT_SOMETHING_ kind of redirection exists to * make the code fully standard-conforming: the C standard requires that * __VA_ARGS__ has at least one argument (though GCC for example would accept @@ -594,6 +601,7 @@ extern const char m_option_path_separator; * argument to ensure __VA_ARGS__ is not empty when calling the next macro. */ +// Note: new code should use OPTF_BOOL instead #define OPT_FLAG(...) \ OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_flag) diff --git a/options/options.c b/options/options.c index 97e82372de..b237c0b268 100644 --- a/options/options.c +++ b/options/options.c @@ -126,7 +126,7 @@ static const m_option_t mp_vo_opt_list[] = { OPT_STRING("x11-name", winname, 0), OPT_FLOATRANGE("monitoraspect", force_monitor_aspect, 0, 0.0, 9.0), OPT_FLOATRANGE("monitorpixelaspect", monitor_pixel_aspect, 0, 1.0/32.0, 32.0), - OPT_FLAG("fullscreen", fullscreen, 0), + {"fullscreen", OPTF_BOOL(fullscreen)}, OPT_ALIAS("fs", "fullscreen"), OPT_FLAG("native-keyrepeat", native_keyrepeat, 0), OPT_FLOATRANGE("panscan", panscan, 0, 0.0, 1.0), diff --git a/options/options.h b/options/options.h index 3a0663b6f8..101d039cb1 100644 --- a/options/options.h +++ b/options/options.h @@ -13,7 +13,7 @@ typedef struct mp_vo_opts { int snap_window; int ontop; int ontop_level; - int fullscreen; + bool fullscreen; int border; int fit_border; int all_workspaces; diff --git a/osdep/macos/mpv_helper.swift b/osdep/macos/mpv_helper.swift index 00c34924e3..d01694d736 100644 --- a/osdep/macos/mpv_helper.swift +++ b/osdep/macos/mpv_helper.swift @@ -81,7 +81,7 @@ class MPVHelper: LogHelper { } func setConfigProperty(fullscreen: Bool) { - optsPtr.pointee.fullscreen = Int32(fullscreen) + optsPtr.pointee.fullscreen = fullscreen m_config_cache_write_opt(optsCachePtr, UnsafeMutableRawPointer(&optsPtr.pointee.fullscreen)) } diff --git a/player/command.c b/player/command.c index 393966bb77..2c86a0ecb7 100644 --- a/player/command.c +++ b/player/command.c @@ -4143,7 +4143,7 @@ static bool check_property_autorepeat(char *property, struct MPContext *mpctx) return true; // This is a heuristic at best. - if (prop.type == &m_option_type_flag || prop.type == &m_option_type_choice) + if (prop.type->flags & M_OPT_TYPE_CHOICE) return false; return true;