options: take care of propertly updating options on runtime changes

All option write accesses are now put through the property interface,
which means runtime option value verification and runtime updates are
applied. This is done even for command line arguments and config files.

This has many subtle and not-so-subtle consequences. The potential for
unintended and intended subtle or not-subtle behavior changes is very
large.

Architecturally, this is us literally jumping through hoops. It really
should work the other way around, with options being able to have
callbacks for value verification and applying runtime updates. But this
would require rewriting the entirety of command.c. This change is more
practical, and if anything will at least allow incremental changes.

Some options are too incompatible for this to work - these are excluded
with an explicit blacklist.

This change fixes many issues caused by the mismatch between properties
and options. For example, this fixes #3281.
This commit is contained in:
wm4 2016-09-17 20:48:22 +02:00
parent 2d34171bec
commit a3e8ff624c
7 changed files with 125 additions and 8 deletions

View File

@ -20,6 +20,15 @@ Interface changes
::
--- mpv 0.21.0 ---
- setting certain options at runtime will now take care of updating them
property (see for example issue #3281). On the other hand, it will also
do runtime verification and reject option changes that do not work
(example: setting the "vf" option to a filter during playback, which fails
to initialize - the option value will remain at its old value). In general,
"set name value" should be mostly equivalent to "set options/name value"
in cases where the "name" property is not deprecated and "options/name"
exists - deviations from this are either bugs, or documented as caveats
in the "Inconsistencies between options and properties" manpage section.
- deprecate _all_ --vo and --ao suboptions. Generally, all suboptions are
replaced by global options, which do exactly the same. For example,
"--vo=opengl:scale=nearest" turns into "--scale=nearest". In some cases,

View File

@ -2066,11 +2066,15 @@ caveats with some properties (due to historical reasons):
allows setting any track ID, and which tracks to enable is chosen at
loading time.)
Option changes at runtime are affected by this as well.
``deinterlace``
While video is active, this behaves differently from the option. It will
never return the ``auto`` value (but the state as observed by the video
chain). You cannot set ``auto`` either.
Option changes at runtime are affected by this as well.
``video-aspect``
While video is active, always returns the effective aspect ratio.
@ -2089,11 +2093,15 @@ caveats with some properties (due to historical reasons):
same way. Also, there are no ``vf-add`` etc. properties, but you can use
the ``vf``/``af`` group of commands to achieve the same.
Option changes at runtime are affected by this as well.
``chapter``
While playback is *not* active, the property behaves like the option, and
you can set a chapter range. While playback is active, you can set only
the current chapter (to which the player will seek immediately).
Option changes at runtime are affected by this as well.
``volume``
When set as option, the maximum (set by ``--volume-max``) is not checked,
while when set as property, the maximum is enforced.

View File

@ -676,9 +676,11 @@ static int handle_set_opt_flags(struct m_config *config,
return set ? 2 : 1;
}
// The type data points to is as in: co->opt
int m_config_set_option_raw(struct m_config *config, struct m_config_option *co,
void *data, int flags)
// Unlike m_config_set_option_raw() this does not go through the property layer
// via config.option_set_callback.
int m_config_set_option_raw_direct(struct m_config *config,
struct m_config_option *co,
void *data, int flags)
{
if (!co)
return M_OPT_UNKNOWN;
@ -702,6 +704,24 @@ int m_config_set_option_raw(struct m_config *config, struct m_config_option *co,
return 0;
}
// Similar to m_config_set_option_ext(), but set as data in its native format.
// This takes care of some details like sending change notifications.
// The type data points to is as in: co->opt
int m_config_set_option_raw(struct m_config *config, struct m_config_option *co,
void *data, int flags)
{
if (config->option_set_callback) {
int r = handle_set_opt_flags(config, co, flags);
if (r <= 1)
return r;
return config->option_set_callback(config->option_set_callback_cb,
co, data, flags);
} else {
return m_config_set_option_raw_direct(config, co, data, flags);
}
}
static int parse_subopts(struct m_config *config, char *name, char *prefix,
struct bstr param, int flags);

View File

@ -77,6 +77,10 @@ typedef struct m_config {
int (*includefunc)(void *ctx, char *filename, int flags);
void *includefunc_ctx;
int (*option_set_callback)(void *ctx, struct m_config_option *co,
void *data, int flags);
void *option_set_callback_cb;
// For the command line parser
int recursion_depth;
@ -170,11 +174,13 @@ static inline int m_config_set_option0(struct m_config *config,
return m_config_set_option(config, bstr0(name), bstr0(param));
}
// Similar to m_config_set_option_ext(), but set as data in its native format.
// The type data points to is as in co->opt
int m_config_set_option_raw(struct m_config *config, struct m_config_option *co,
void *data, int flags);
int m_config_set_option_raw_direct(struct m_config *config,
struct m_config_option *co,
void *data, int flags);
// Similar to m_config_set_option_ext(), but set as data using mpv_node.
struct mpv_node;
int m_config_set_option_node(struct m_config *config, bstr name,

View File

@ -126,6 +126,9 @@ static int edit_filters(struct MPContext *mpctx, struct mp_log *log,
static int set_filters(struct MPContext *mpctx, enum stream_type mediatype,
struct m_obj_settings *new_chain);
static int mp_property_do_silent(const char *name, int action, void *val,
struct MPContext *ctx);
static void hook_remove(struct MPContext *mpctx, int index)
{
struct command_ctx *cmd = mpctx->command_ctx;
@ -254,6 +257,64 @@ static char *format_delay(double time)
return talloc_asprintf(NULL, "%d ms", (int)lrint(time * 1000));
}
// Option-property bridge. This is used so that setting options via various
// mechanisms (including command line parsing, config files, per-file options)
// updates state associated with them. For that, they have to go through the
// property layer. (Ideally, this would be the other way around, and there
// would be per-option change handlers instead.)
// Note that the property-option bridge sidesteps this, as we'd get infinite
// recursion.
int mp_on_set_option(void *ctx, struct m_config_option *co, void *data, int flags)
{
struct MPContext *mpctx = ctx;
// These options are too inconsistent as they could be pulled through the
// property layer. Ideally we'd remove these inconsistencies in the future,
// though the actual problem is compatibility to user-expected behavior.
// What matters is whether _write_ access is different - property read
// access is not used here.
// We're also fine with cases where the property restricts the writable
// value range if playback is active, but not otherwise.
// OK, restrict during playback: vid, aid, sid, deinterlace, video-aspect,
// vf*, af*, chapter
// OK, is handled separately: playlist
// OK, does not conflict on low level: audio-file, sub-file, external-file
static const char *const no_property[] = {
"playlist-pos", // checks playlist bounds, "no" choice missing
"volume", // restricts to --volume-max
"demuxer", "idle", "length", "audio-samplerate", "audio-channels",
"audio-format", "fps", "cache", // different semantics
NULL
};
for (int n = 0; no_property[n]; n++) {
if (strcmp(co->name, no_property[n]) == 0)
goto direct_option;
}
// Normalize "vf*" to "vf"
const char *name = co->name;
bstr bname = bstr0(name);
char tmp[50];
if (bstr_eatend0(&bname, "*")) {
snprintf(tmp, sizeof(name), "%.*s", BSTR_P(bname));
name = tmp;
}
int r = mp_property_do_silent(name, M_PROPERTY_SET, data, mpctx);
if (r != M_PROPERTY_OK)
return M_OPT_INVALID;
// The flag can't be passed through the property layer correctly.
if (flags & M_SETOPT_FROM_CMDLINE)
co->is_set_from_cmdline = true;
return 0;
direct_option:
return m_config_set_option_raw_direct(mpctx->mconfig, co, data, flags);
}
// Property-option bridge. (Maps the property to the option with the same name.)
static int mp_property_generic_option_do(void *ctx, struct m_property *prop,
int action, void *arg, bool force)
@ -277,7 +338,7 @@ static int mp_property_generic_option_do(void *ctx, struct m_property *prop,
m_option_copy(opt->opt, arg, valptr);
return M_PROPERTY_OK;
case M_PROPERTY_SET:
if (m_config_set_option_raw(mpctx->mconfig, opt, arg, flags) < 0)
if (m_config_set_option_raw_direct(mpctx->mconfig, opt, arg, flags) < 0)
return M_PROPERTY_ERROR;
return M_PROPERTY_OK;
}
@ -4069,13 +4130,20 @@ static bool is_property_set(int action, void *val)
}
}
int mp_property_do(const char *name, int action, void *val,
struct MPContext *ctx)
static int mp_property_do_silent(const char *name, int action, void *val,
struct MPContext *ctx)
{
struct command_ctx *cmd = ctx->command_ctx;
int r = m_property_do(ctx->log, cmd->properties, name, action, val, ctx);
if (r == M_PROPERTY_OK && is_property_set(action, val))
mp_notify_property(ctx, (char *)name);
return r;
}
int mp_property_do(const char *name, int action, void *val,
struct MPContext *ctx)
{
int r = mp_property_do_silent(name, action, val, ctx);
if (mp_msg_test(ctx->log, MSGL_V) && is_property_set(action, val)) {
struct m_option ot = {0};
void *data = val;

View File

@ -24,6 +24,7 @@ struct MPContext;
struct mp_cmd;
struct mp_log;
struct mpv_node;
struct m_config_option;
void command_init(struct MPContext *mpctx);
void command_uninit(struct MPContext *mpctx);
@ -35,6 +36,8 @@ void property_print_help(struct MPContext *mpctx);
int mp_property_do(const char* name, int action, void* val,
struct MPContext *mpctx);
int mp_on_set_option(void *ctx, struct m_config_option *co, void *data, int flags);
void mp_notify(struct MPContext *mpctx, int event, void *arg);
void mp_notify_property(struct MPContext *mpctx, const char *property);

View File

@ -356,6 +356,9 @@ struct MPContext *mp_create(void)
mp_input_set_cancel(mpctx->input, mpctx->playback_abort);
mpctx->mconfig->option_set_callback = mp_on_set_option;
mpctx->mconfig->option_set_callback_cb = mpctx;
return mpctx;
}