From a9bd4535d2eac283824d8598aa17f1f44f83a74a Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 15 Apr 2016 11:31:24 +0200 Subject: client API: improve mpv_set_property() handling of MPV_FORMAT_NODE If a mpv_node wrapped a string, the behavior was different from calling mpv_set_property() with MPV_FORMAT_STRING directly. Change this. The original intention was to be strict about types if MPV_FORMAT_NODE is used. But I think the result was less than ideal, and the same change towards less strict behavior was made to mpv_set_option() ages ago. --- DOCS/client-api-changes.rst | 9 ++++++++ libmpv/client.h | 14 ++++++++++-- options/m_option.c | 15 +++++++++++++ options/m_option.h | 4 ++++ options/m_property.c | 17 +++++---------- player/client.c | 52 ++++++++++++++++----------------------------- 6 files changed, 63 insertions(+), 48 deletions(-) diff --git a/DOCS/client-api-changes.rst b/DOCS/client-api-changes.rst index f2cc5993eb..44490ea79d 100644 --- a/DOCS/client-api-changes.rst +++ b/DOCS/client-api-changes.rst @@ -32,6 +32,15 @@ API changes :: + --- mpv 0.17.1 --- + 1.21 - mpv_set_property() changes behavior with MPV_FORMAT_NODE. Before this + change it rejected mpv_nodes with format==MPV_FORMAT_STRING if the + property was not a string or did not have special mechanisms in place + the function failed. Now it always invokes the option string parser, + and mpv_node with a basic data type works exactly as if the function + is invoked with that type directly. This new behavior is equivalent + to mpv_set_option(). + This also affects the mp.set_property_native() Lua function. --- mpv 0.12.0 --- 1.20 - deprecate "GL_MP_D3D_interfaces"/"glMPGetD3DInterface", and introduce "GL_MP_MPGetNativeDisplay"/"glMPGetNativeDisplay" (this is a diff --git a/libmpv/client.h b/libmpv/client.h index 4ba96bd861..1980cea881 100644 --- a/libmpv/client.h +++ b/libmpv/client.h @@ -215,7 +215,7 @@ extern "C" { * relational operators (<, >, <=, >=). */ #define MPV_MAKE_VERSION(major, minor) (((major) << 16) | (minor) | 0UL) -#define MPV_CLIENT_API_VERSION MPV_MAKE_VERSION(1, 20) +#define MPV_CLIENT_API_VERSION MPV_MAKE_VERSION(1, 21) /** * Return the MPV_CLIENT_API_VERSION the mpv source has been compiled with. @@ -780,6 +780,10 @@ void mpv_free_node_contents(mpv_node *node); * mpv_set_property() to change settings during playback, because the property * mechanism guarantees that changes take effect immediately. * + * Using a format other than MPV_FORMAT_NODE is equivalent to constructing a + * mpv_node with the given format and data, and passing the mpv_node to this + * function. + * * @param name Option name. This is the same as on the mpv command line, but * without the leading "--". * @param format see enum mpv_format. @@ -877,7 +881,13 @@ int mpv_command_node_async(mpv_handle *ctx, uint64_t reply_userdata, * usually will fail with MPV_ERROR_PROPERTY_FORMAT. In some cases, the data * is automatically converted and access succeeds. For example, MPV_FORMAT_INT64 * is always converted to MPV_FORMAT_DOUBLE, and access using MPV_FORMAT_STRING - * usually invokes a string parser. + * usually invokes a string parser. The same happens when calling this function + * with MPV_FORMAT_NODE: the underlying format may be converted to another + * type if possible. + * + * Using a format other than MPV_FORMAT_NODE is equivalent to constructing a + * mpv_node with the given format and data, and passing the mpv_node to this + * function. (Before API version 1.21, this was different.) * * @param name The property name. See input.rst for a list of properties. * @param format see enum mpv_format. diff --git a/options/m_option.c b/options/m_option.c index 1d929269b0..1d8b1d0bf6 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -106,6 +106,21 @@ const m_option_t *m_option_list_find(const m_option_t *list, const char *name) return m_option_list_findb(list, bstr0(name)); } +int m_option_set_node_or_string(struct mp_log *log, const m_option_t *opt, + const char *name, void *dst, struct mpv_node *src) +{ + if (src->format == MPV_FORMAT_STRING) { + // The af and vf option unfortunately require this, because the + // option name includes the "action". + bstr optname = bstr0(name), a, b; + if (bstr_split_tok(optname, "/", &a, &b)) + optname = b; + return m_option_parse(log, opt, optname, bstr0(src->u.string), dst); + } else { + return m_option_set_node(opt, dst, src); + } +} + // Default function that just does a memcpy static void copy_opt(const m_option_t *opt, void *dst, const void *src) diff --git a/options/m_option.h b/options/m_option.h index 320a9e5b99..7e6550691a 100644 --- a/options/m_option.h +++ b/options/m_option.h @@ -504,6 +504,10 @@ static inline int m_option_set_node(const m_option_t *opt, void *dst, return M_OPT_UNKNOWN; } +// Call m_option_parse for strings, m_option_set_node otherwise. +int m_option_set_node_or_string(struct mp_log *log, const m_option_t *opt, + const char *name, void *dst, struct mpv_node *src); + // see m_option_type.get static inline int m_option_get_node(const m_option_t *opt, void *ta_parent, struct mpv_node *dst, void *src) diff --git a/options/m_property.c b/options/m_property.c index 951b788d4b..de2361b868 100644 --- a/options/m_property.c +++ b/options/m_property.c @@ -104,16 +104,8 @@ int m_property_do(struct mp_log *log, const struct m_property *prop_list, return str != NULL; } case M_PROPERTY_SET_STRING: { - if (!log) - return M_PROPERTY_ERROR; - bstr optname = bstr0(name), a, b; - if (bstr_split_tok(optname, "/", &a, &b)) - optname = b; - if (m_option_parse(log, &opt, optname, bstr0(arg), &val) < 0) - return M_PROPERTY_ERROR; - r = do_action(prop_list, name, M_PROPERTY_SET, &val, ctx); - m_option_free(&opt, &val); - return r; + struct mpv_node node = { .format = MPV_FORMAT_STRING, .u.string = arg }; + return do_action(prop_list, name, M_PROPERTY_SET_NODE, &node, ctx); } case M_PROPERTY_SWITCH: { if (!log) @@ -163,11 +155,12 @@ int m_property_do(struct mp_log *log, const struct m_property *prop_list, return r; } case M_PROPERTY_SET_NODE: { + if (!log) + return M_PROPERTY_ERROR; if ((r = do_action(prop_list, name, M_PROPERTY_SET_NODE, arg, ctx)) != M_PROPERTY_NOT_IMPLEMENTED) return r; - struct mpv_node *node = arg; - int err = m_option_set_node(&opt, &val, node); + int err = m_option_set_node_or_string(log, &opt, name, &val, arg); if (err == M_OPT_UNKNOWN) { r = M_PROPERTY_NOT_IMPLEMENTED; } else if (err < 0) { diff --git a/player/client.c b/player/client.c index d3b0567a97..6b5eaf7883 100644 --- a/player/client.c +++ b/player/client.c @@ -1071,44 +1071,28 @@ static void setproperty_fn(void *arg) struct setproperty_request *req = arg; const struct m_option *type = get_mp_type(req->format); - int err; - switch (req->format) { - case MPV_FORMAT_STRING: { - // Go through explicit string conversion. M_PROPERTY_SET_NODE doesn't - // do this, because it tries to be somewhat type-strict. But the client - // needs a way to set everything by string. - char *s = *(char **)req->data; - MP_VERBOSE(req->mpctx, "Set property string: %s='%s'\n", req->name, s); - err = mp_property_do(req->name, M_PROPERTY_SET_STRING, s, req->mpctx); - break; - } - case MPV_FORMAT_NODE: - case MPV_FORMAT_FLAG: - case MPV_FORMAT_INT64: - case MPV_FORMAT_DOUBLE: { - struct mpv_node node; - if (req->format == MPV_FORMAT_NODE) { - node = *(struct mpv_node *)req->data; - } else { - // These are basically emulated via mpv_node. - node.format = req->format; - memcpy(&node.u, req->data, type->type->size); - } - if (mp_msg_test(req->mpctx->log, MSGL_V)) { - struct m_option ot = {.type = &m_option_type_node}; - char *t = m_option_print(&ot, &node); - MP_VERBOSE(req->mpctx, "Set property: %s=%s\n", req->name, t ? t : "?"); - talloc_free(t); - } - err = mp_property_do(req->name, M_PROPERTY_SET_NODE, &node, req->mpctx); - break; - } - default: - abort(); + struct mpv_node *node; + struct mpv_node tmp; + if (req->format == MPV_FORMAT_NODE) { + node = req->data; + } else { + tmp.format = req->format; + memcpy(&tmp.u, req->data, type->type->size); + node = &tmp; } + int err = mp_property_do(req->name, M_PROPERTY_SET_NODE, node, req->mpctx); + req->status = translate_property_error(err); + if (mp_msg_test(req->mpctx->log, MSGL_V)) { + struct m_option ot = {.type = &m_option_type_node}; + char *t = m_option_print(&ot, node); + MP_VERBOSE(req->mpctx, "Set property: %s=%s -> %d\n", + req->name, t ? t : "?", err); + talloc_free(t); + } + if (req->reply_ctx) { status_reply(req->reply_ctx, MPV_EVENT_SET_PROPERTY_REPLY, req->userdata, req->status); -- cgit v1.2.3