options: change --sub-file behavior

--sub-file is actually a string list, so you can add multipel external
subtitle files. But to be able to set a list, the option value was split
on ",". This made it impossible to add filenames.

One possible solution would be adding escaping. That's probably a good
idea (and some other options already do this), but it's also complicated
both to implement and for the user.

The simpler solution is making --sub-file appending, and make it take
only a single entry.

I'm not quite sure about this yet. It breaks the invariant that if a
value is printed and parsed, you get the same value back. So for now,
just go with the simple solution.

Fixes #840.

3 files changed, 41 insertions, 1 deletions

diff --git a/options/m_option.c b/options/m_option.c
index f52a81a0b9..77e610f08c 100644
--- a/options/m_option.c
+++ b/options/m_option.c
@@ -1344,6 +1344,42 @@ const m_option_type_t m_option_type_string_list = {
     .set   = str_list_set,
 };
 
+static void str_list_append(void *dst, bstr s)
+{
+    if (!dst)
+        return;
+    char **list= VAL(dst);
+    int len = 0;
+    while (list && list[len])
+        len++;
+    MP_TARRAY_APPEND(NULL, list, len, bstrto0(NULL, s));
+    MP_TARRAY_APPEND(NULL, list, len, NULL);
+    VAL(dst) = list;
+}
+
+static int parse_str_append_list(struct mp_log *log, const m_option_t *opt,
+                                 struct bstr name, struct bstr param, void *dst)
+{
+    if (param.len == 0)
+        return M_OPT_MISSING_PARAM;
+
+    str_list_append(dst, param);
+
+    return 1;
+}
+
+const m_option_type_t m_option_type_string_append_list = {
+    .name  = "String list",
+    .size  = sizeof(char **),
+    .flags = M_OPT_TYPE_DYNAMIC,
+    .parse = parse_str_append_list,
+    .print = print_str_list,
+    .copy  = copy_str_list,
+    .free  = free_str_list,
+    .get   = str_list_get,
+    .set   = str_list_set,
+};
+
 static int read_subparam(struct mp_log *log, bstr optname,
                          bstr *str, bstr *out_subparam);
 
diff --git a/options/m_option.h b/options/m_option.h
index 6a38a7f4e6..b7579bd5e9 100644
--- a/options/m_option.h
+++ b/options/m_option.h
@@ -47,6 +47,7 @@ extern const m_option_type_t m_option_type_float;
 extern const m_option_type_t m_option_type_double;
 extern const m_option_type_t m_option_type_string;
 extern const m_option_type_t m_option_type_string_list;
+extern const m_option_type_t m_option_type_string_append_list;
 extern const m_option_type_t m_option_type_keyvalue_list;
 extern const m_option_type_t m_option_type_time;
 extern const m_option_type_t m_option_type_rel_time;
@@ -575,6 +576,9 @@ extern const char m_option_path_separator;
 #define OPT_STRINGLIST(...) \
     OPT_GENERAL(char**, __VA_ARGS__, .type = &m_option_type_string_list)
 
+#define OPT_STRING_APPEND_LIST(...) \
+    OPT_GENERAL(char**, __VA_ARGS__, .type = &m_option_type_string_append_list)
+
 #define OPT_KEYVALUELIST(...) \
     OPT_GENERAL(char**, __VA_ARGS__, .type = &m_option_type_keyvalue_list)
 
diff --git a/options/options.c b/options/options.c
index 3cebd0c5fd..8f071345d5 100644
--- a/options/options.c
+++ b/options/options.c
@@ -396,7 +396,7 @@ const m_option_t mp_opts[] = {
 
 // ------------------------- subtitles options --------------------
 
-    OPT_STRINGLIST("sub-file", sub_name, 0),
+    OPT_STRING_APPEND_LIST("sub-file", sub_name, 0),
     OPT_PATHLIST("sub-paths", sub_paths, 0),
     OPT_STRING("sub-codepage", sub_cp, 0),
     OPT_FLOAT("sub-delay", sub_delay, 0),