options: change --msg-level option

Make it accept "," as separator, instead of only ":". Do this by using
the key-value-list parser. Before this, the option was stored as a
string, with the option parser verifying that the option value as
correct. Now it's stored pre-parsed, although the log levels still
require separate verification and parsing-on-use to some degree (which
is why the msg-level option type doesn't go away).

Because the internal type changes, the client API "native" type also
changes. This could be prevented with some more effort, but I don't
think it's worth it - if MPV_FORMAT_STRING is used, it still works the
same, just with a different separator on read accesses.

DOCS/client-api-changes.rst

@@ -25,6 +25,8 @@ API changes
 
 ::
 
+ git    - the --msg-level option changes its native type from a flat string to
+          a key-value list (setting/reading the option as string still works)
  1.14   - add mpv_wait_async_requests()
  1.13   - add MPV_EVENT_QUEUE_OVERFLOW
  1.12   - add class Handle to qthelper.hpp

DOCS/man/options.rst

@@ -2745,7 +2745,7 @@ Terminal
 ``--no-msg-color``
     Disable colorful console output on terminals.
 
-``--msg-level=<module1=level1:module2=level2:...>``
+``--msg-level=<module1=level1,module2=level2,...>``
     Control verbosity directly for each module. The ``all`` module changes the
     verbosity of all the modules not explicitly specified on the command line.
 

common/msg.c

@@ -48,7 +48,7 @@
 struct mp_log_root {
     struct mpv_global *global;
     // --- protected by mp_msg_lock
-    char *msglevels;
+    char **msg_levels;
     bool use_terminal;  // make accesses to stderr/stdout
     bool module;
     bool show_time;
@@ -96,28 +96,26 @@ static pthread_mutex_t mp_msg_lock = PTHREAD_MUTEX_INITIALIZER;
 static const struct mp_log null_log = {0};
 struct mp_log *const mp_null_log = (struct mp_log *)&null_log;
 
-static bool match_mod(const char *name, bstr mod)
+static bool match_mod(const char *name, const char *mod)
 {
-    if (bstr_equals0(mod, "all"))
+    if (!strcmp(mod, "all"))
         return true;
     // Path prefix matches
     bstr b = bstr0(name);
-    return bstr_eatstart(&b, mod) && (bstr_eatstart0(&b, "/") || !b.len);
+    return bstr_eatstart0(&b, mod) && (bstr_eatstart0(&b, "/") || !b.len);
 }
 
 static void update_loglevel(struct mp_log *log)
 {
+    struct mp_log_root *root = log->root;
     pthread_mutex_lock(&mp_msg_lock);
     log->level = -1;
     log->terminal_level = -1;
     if (log->root->use_terminal) {
         log->level = MSGL_STATUS + log->root->verbose; // default log level
-        bstr s = bstr0(log->root->msglevels);
-        bstr mod;
-        int level;
-        while (mp_msg_split_msglevel(&s, &mod, &level) > 0) {
-            if (match_mod(log->verbose_prefix, mod))
-                log->level = level;
+        for (int n = 0; root->msg_levels && root->msg_levels[n * 2 + 0]; n++) {
+            if (match_mod(log->verbose_prefix, root->msg_levels[n * 2 + 0]))
+                log->level = mp_msg_find_level(root->msg_levels[n * 2 + 1]);
         }
         log->terminal_level = log->root->use_terminal ? log->level : -1;
     }
@@ -475,8 +473,9 @@ void mp_msg_update_msglevels(struct mpv_global *global)
         root->termosd = isatty(STDERR_FILENO);
     }
 
-    talloc_free(root->msglevels);
-    root->msglevels = talloc_strdup(root, global->opts->msglevels);
+    m_option_type_msglevels.free(&root->msg_levels);
+    m_option_type_msglevels.copy(NULL, &root->msg_levels,
+                                 &global->opts->msg_levels);
 
     if (!root->log_file && opts->log_file && opts->log_file[0])
         root->log_file = fopen(opts->log_file, "wb");
@@ -506,6 +505,7 @@ void mp_msg_uninit(struct mpv_global *global)
         fclose(root->stats_file);
     if (root->log_file)
         fclose(root->log_file);
+    m_option_type_msglevels.free(&root->msg_levels);
     talloc_free(root);
     global->log = NULL;
 }
@@ -639,26 +639,11 @@ const int mp_mpv_log_levels[MSGL_MAX + 1] = {
     [MSGL_STATS]        = 0, // never used
 };
 
-int mp_msg_split_msglevel(struct bstr *s, struct bstr *out_mod, int *out_level)
+int mp_msg_find_level(const char *s)
 {
-    if (s->len == 0)
-        return 0;
-    bstr elem, rest;
-    bstr_split_tok(*s, ":", &elem, &rest);
-    bstr mod, level;
-    if (!bstr_split_tok(elem, "=", &mod, &level) || mod.len == 0)
-        return -1;
-    int ilevel = -1;
     for (int n = 0; n < MP_ARRAY_SIZE(mp_log_levels); n++) {
-        if (mp_log_levels[n] && bstr_equals0(level, mp_log_levels[n])) {
-            ilevel = n;
-            break;
-        }
+        if (mp_log_levels[n] && mp_log_levels[n] && !strcmp(s, mp_log_levels[n]))
+            return n;
     }
-    if (ilevel < 0 && !bstr_equals0(level, "no"))
-        return -1;
-    *s = rest;
-    *out_mod = mod;
-    *out_level = ilevel;
-    return 1;
+    return -1;
 }

common/msg_control.h

@@ -28,9 +28,7 @@ void mp_msg_log_buffer_destroy(struct mp_log_buffer *buffer);
 struct mp_log_buffer_entry *mp_msg_log_buffer_read(struct mp_log_buffer *buffer);
 
 int mp_msg_open_stats_file(struct mpv_global *global, const char *path);
-
-struct bstr;
-int mp_msg_split_msglevel(struct bstr *s, struct bstr *out_mod, int *out_level);
+int mp_msg_find_level(const char *s);
 
 extern const char *const mp_log_levels[MSGL_MAX + 1];
 extern const int mp_mpv_log_levels[MSGL_MAX + 1];

options/m_option.c

@@ -1495,6 +1495,73 @@ const m_option_type_t m_option_type_keyvalue_list = {
     .set   = keyvalue_list_set,
 };
 
+
+#undef VAL
+#define VAL(x) (*(char **)(x))
+
+static int check_msg_levels(struct mp_log *log, char **list)
+{
+    for (int n = 0; list && list[n * 2 + 0]; n++) {
+        char *level = list[n * 2 + 1];
+        if (mp_msg_find_level(level) < 0 && strcmp(level, "no") != 0) {
+            mp_err(log, "Invalid message level '%s'\n", level);
+            return M_OPT_INVALID;
+        }
+    }
+    return 1;
+}
+
+static int parse_msglevels(struct mp_log *log, const m_option_t *opt,
+                           struct bstr name, struct bstr param, void *dst)
+{
+    if (bstr_equals0(param, "help")) {
+        mp_info(log, "Syntax: --msglevel=module1=level:module2=level:...\n"
+                     "'module' is output prefix as shown with -v, or a prefix\n"
+                     "of it. level is one of:\n\n"
+                     "  fatal error warn info status v debug trace\n\n"
+                     "The level specifies the minimum log level a message\n"
+                     "must have to be printed.\n"
+                     "The special module name 'all' affects all modules.\n");
+        return M_OPT_EXIT;
+    }
+
+    char **dst_copy = NULL;
+    int r = m_option_type_keyvalue_list.parse(log, opt, name, param, &dst_copy);
+    if (r >= 0)
+        r = check_msg_levels(log, dst_copy);
+
+    if (r >= 0)
+        m_option_type_keyvalue_list.copy(opt, dst, &dst_copy);
+    m_option_type_keyvalue_list.free(&dst_copy);
+    return r;
+}
+
+static int set_msglevels(const m_option_t *opt, void *dst,
+                             struct mpv_node *src)
+{
+    char **dst_copy = NULL;
+    int r = m_option_type_keyvalue_list.set(opt, &dst_copy, src);
+    if (r >= 0)
+        r = check_msg_levels(mp_null_log, dst_copy);
+
+    if (r >= 0)
+        m_option_type_keyvalue_list.copy(opt, dst, &dst_copy);
+    m_option_type_keyvalue_list.free(&dst_copy);
+    return r;
+}
+
+const m_option_type_t m_option_type_msglevels = {
+    .name = "Output verbosity levels",
+    .size  = sizeof(char **),
+    .flags = M_OPT_TYPE_DYNAMIC,
+    .parse = parse_msglevels,
+    .print = print_keyvalue_list,
+    .copy  = copy_str_list,
+    .free  = free_str_list,
+    .get   = keyvalue_list_get,
+    .set   = set_msglevels,
+};
+
 /////////////////// Print
 
 static int parse_print(struct mp_log *log, const m_option_t *opt,
@@ -1630,62 +1697,6 @@ const m_option_type_t m_option_type_subconfig = {
     .parse = parse_subconf,
 };
 
-#undef VAL
-#define VAL(x) (*(char **)(x))
-
-static int parse_msglevels(struct mp_log *log, const m_option_t *opt,
-                           struct bstr name, struct bstr param, void *dst)
-{
-    if (param.start == NULL)
-        return M_OPT_MISSING_PARAM;
-
-    if (bstr_equals0(param, "help")) {
-        mp_info(log, "Syntax: --msglevel=module1=level:module2=level:...\n"
-                     "'module' is output prefix as shown with -v, or a prefix\n"
-                     "of it. level is one of:\n\n"
-                     "  fatal error warn info status v debug trace\n\n"
-                     "The level specifies the minimum log level a message\n"
-                     "must have to be printed.\n"
-                     "The special module name 'all' affects all modules.\n");
-        return M_OPT_EXIT;
-    }
-
-    bstr s = param;
-    while (1) {
-        int res = mp_msg_split_msglevel(&s, &(bstr){0}, &(int){0});
-        if (res == 0)
-            break;
-        if (res < 0) {
-            mp_err(log, "Invalid syntax: %.*s\n", BSTR_P(s));
-            return M_OPT_INVALID;
-        }
-    }
-
-    if (dst && param.len) {
-        char *prev = VAL(dst);
-        char *new;
-        if (prev && prev[0]) {
-            new = talloc_asprintf(NULL, "%s:%.*s", prev, BSTR_P(param));
-        } else {
-            new = bstrdup0(NULL, param);
-        }
-        talloc_free(prev);
-        VAL(dst) = new;
-    }
-
-    return 1;
-}
-
-const m_option_type_t m_option_type_msglevels = {
-    .name = "Output verbosity levels",
-    .size  = sizeof(char *),
-    .flags = M_OPT_TYPE_DYNAMIC,
-    .parse = parse_msglevels,
-    .print = print_str,
-    .copy  = copy_str,
-    .free  = free_str,
-};
-
 #undef VAL
 
 // Split the string on the given split character.

options/options.c

@@ -105,7 +105,7 @@ const m_option_t mp_opts[] = {
     OPT_FLAG("quiet", quiet, CONF_GLOBAL),
     OPT_FLAG_STORE("really-quiet", verbose, CONF_GLOBAL | CONF_PRE_PARSE, -10),
     OPT_FLAG("terminal", use_terminal, CONF_GLOBAL | CONF_PRE_PARSE),
-    OPT_GENERAL(char*, "msg-level", msglevels, CONF_GLOBAL|CONF_PRE_PARSE,
+    OPT_GENERAL(char**, "msg-level", msg_levels, CONF_GLOBAL|CONF_PRE_PARSE,
                 .type = &m_option_type_msglevels),
     OPT_STRING("dump-stats", dump_stats, CONF_GLOBAL | CONF_PRE_PARSE),
     OPT_FLAG("msg-color", msg_color, CONF_GLOBAL | CONF_PRE_PARSE),

options/options.h

@@ -54,9 +54,9 @@ struct mp_cache_opts {
 
 typedef struct MPOpts {
     int use_terminal;
-    char *msglevels;
     char *dump_stats;
     int verbose;
+    char **msg_levels;
     int msg_color;
     int msg_module;
     int msg_time;