client API: treat MPV_FORMAT_STRING differently in mpv_set_property

Always map MPV_FORMAT_STRING to setting property value directly through
M_PROPERTY_SET_STRING, instead of trying to go through
M_PROPERTY_SET_NODE.

This treats a direct MPV_FORMAT_STRING query differently from a
MPV_FORMAT_STRING wrapped in a mpv_node. This was already the case in
mpv_get_property(). The reason for all this is that mpv_node is supposed
to be the exact type, while a direct MPV_FORMAT_STRING goes through all
possible conversions.

Not sure if these semantics are good.

1 files changed, 20 insertions, 19 deletions

diff --git a/player/client.c b/player/client.c
index e2257ab8cf..537f8f3cb2 100644
--- a/player/client.c
+++ b/player/client.c
@@ -677,34 +677,35 @@ static void setproperty_fn(void *arg)
     struct setproperty_request *req = arg;
     const struct m_option *type = get_mp_type(req->format);
 
-    struct mpv_node node;
-    node.format = req->format;
-
-    req->status = 0;
+    int err;
     switch (req->format) {
-    case MPV_FORMAT_NODE:
-        node = *(struct mpv_node *)req->data;
+    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;
+        err = mp_property_do(req->name, M_PROPERTY_SET_STRING, s, req->mpctx);
         break;
-    case MPV_FORMAT_STRING:
+    }
+    case MPV_FORMAT_NODE:
     case MPV_FORMAT_FLAG:
     case MPV_FORMAT_INT64:
-    case MPV_FORMAT_DOUBLE:
-        // These are basically emulated via mpv_node.
-        memcpy(&node.u, req->data, type->type->size);
+    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);
+        }
+        err = mp_property_do(req->name, M_PROPERTY_SET_NODE, &node, req->mpctx);
         break;
+    }
     default:
         abort();
     }
 
-    int err = mp_property_do(req->name, M_PROPERTY_SET_NODE, &node, req->mpctx);
-    if (err == M_PROPERTY_NOT_IMPLEMENTED && req->format == 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.
-        err = mp_property_do(req->name, M_PROPERTY_SET_STRING, node.u.string,
-                             req->mpctx);
-    }
-
     req->status = translate_property_error(err);
 
     if (req->reply_ctx) {