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).
This commit is contained in:
wm4 2020-03-14 02:07:35 +01:00
parent 7c4a550c0e
commit c784820454
6 changed files with 80 additions and 11 deletions

View File

@ -118,11 +118,11 @@ static void copy_opt(const m_option_t *opt, void *dst, const void *src)
memcpy(dst, src, opt->type->size); 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) struct bstr name, struct bstr param, void *dst)
{ {
if (bstr_equals0(param, "yes") || !param.len) { 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; 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"); 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) if (fabs(add) < 0.5)
return; 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; 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) if (src->format != MPV_FORMAT_FLAG)
return M_OPT_UNKNOWN; 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; 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) struct mpv_node *dst, void *src)
{ {
dst->format = MPV_FORMAT_FLAG; dst->format = MPV_FORMAT_FLAG;
@ -179,6 +179,67 @@ static int flag_get(const m_option_t *opt, void *ta_parent,
return 1; 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) static bool flag_equal(const m_option_t *opt, void *a, void *b)
{ {
return VAL(a) == VAL(b); return VAL(a) == VAL(b);

View File

@ -38,6 +38,7 @@ struct mpv_global;
///////////////////////////// Options types declarations //////////////////// ///////////////////////////// Options types declarations ////////////////////
// Simple types // 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_flag;
extern const m_option_type_t m_option_type_dummy_flag; extern const m_option_type_t m_option_type_dummy_flag;
extern const m_option_type_t m_option_type_int; 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 // through this union. It serves for self-documentation and to get minimal
// size/alignment requirements for option values in general. // size/alignment requirements for option values in general.
union m_option_value { union m_option_value {
bool bool_;
int flag; // not the C type "bool"! int flag; // not the C type "bool"!
int int_; int int_;
int64_t int64; int64_t int64;
@ -352,6 +354,7 @@ struct m_option_type {
// Option description // Option description
struct m_option { struct m_option {
// Option name. // Option name.
// Option declarations can use this as positional field.
const char *name; const char *name;
// Option type. // Option type.
@ -587,6 +590,10 @@ extern const char m_option_path_separator;
#define OPT_HELPER_REMOVEPAREN(...) __VA_ARGS__ #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 /* The OPT_SOMETHING->OPT_SOMETHING_ kind of redirection exists to
* make the code fully standard-conforming: the C standard requires that * make the code fully standard-conforming: the C standard requires that
* __VA_ARGS__ has at least one argument (though GCC for example would accept * __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. * argument to ensure __VA_ARGS__ is not empty when calling the next macro.
*/ */
// Note: new code should use OPTF_BOOL instead
#define OPT_FLAG(...) \ #define OPT_FLAG(...) \
OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_flag) OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_flag)

View File

@ -126,7 +126,7 @@ static const m_option_t mp_vo_opt_list[] = {
OPT_STRING("x11-name", winname, 0), OPT_STRING("x11-name", winname, 0),
OPT_FLOATRANGE("monitoraspect", force_monitor_aspect, 0, 0.0, 9.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_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_ALIAS("fs", "fullscreen"),
OPT_FLAG("native-keyrepeat", native_keyrepeat, 0), OPT_FLAG("native-keyrepeat", native_keyrepeat, 0),
OPT_FLOATRANGE("panscan", panscan, 0, 0.0, 1.0), OPT_FLOATRANGE("panscan", panscan, 0, 0.0, 1.0),

View File

@ -13,7 +13,7 @@ typedef struct mp_vo_opts {
int snap_window; int snap_window;
int ontop; int ontop;
int ontop_level; int ontop_level;
int fullscreen; bool fullscreen;
int border; int border;
int fit_border; int fit_border;
int all_workspaces; int all_workspaces;

View File

@ -81,7 +81,7 @@ class MPVHelper: LogHelper {
} }
func setConfigProperty(fullscreen: Bool) { func setConfigProperty(fullscreen: Bool) {
optsPtr.pointee.fullscreen = Int32(fullscreen) optsPtr.pointee.fullscreen = fullscreen
m_config_cache_write_opt(optsCachePtr, UnsafeMutableRawPointer(&optsPtr.pointee.fullscreen)) m_config_cache_write_opt(optsCachePtr, UnsafeMutableRawPointer(&optsPtr.pointee.fullscreen))
} }

View File

@ -4143,7 +4143,7 @@ static bool check_property_autorepeat(char *property, struct MPContext *mpctx)
return true; return true;
// This is a heuristic at best. // 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 false;
return true; return true;