From 96ef62161ac8edb3c111bde7a79cb07dd6db813c Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 15 Feb 2020 15:07:52 +0100 Subject: [PATCH] demux_edl: improve parsing slightly Add a mp_log context to the parse_edl() function, and report some errors. Previously, this just told you that something was wrong. Add some error reporting to make it slightly less evil. Put all parameters in a list before processing them. This makes adding parameters for special headers easier, and we can report parameters that were not understood. (For "compatibility", which probably doesn't matter at all, still don't count them as errors; as before.) --- DOCS/edl-mpv.rst | 3 +- demux/demux_edl.c | 148 +++++++++++++++++++++++++++++++--------------- 2 files changed, 103 insertions(+), 48 deletions(-) diff --git a/DOCS/edl-mpv.rst b/DOCS/edl-mpv.rst index 46a25293aa..881c3138d4 100644 --- a/DOCS/edl-mpv.rst +++ b/DOCS/edl-mpv.rst @@ -189,7 +189,8 @@ Currently, time values are floating point values in seconds. As an extension, you can set the ``timestamps=chapters`` option. If this option is set, timestamps have to be integers, and refer to chapter numbers, starting -with 0. +with 0. The default value for this parameter is ``seconds``, which means the +time is as described in the previous paragraph. Example:: diff --git a/demux/demux_edl.c b/demux/demux_edl.c index 9d276732fe..291072d6e6 100644 --- a/demux/demux_edl.c +++ b/demux/demux_edl.c @@ -63,16 +63,63 @@ struct priv { bstr data; }; -// Parse a time (absolute file time or duration). Currently equivalent to a -// number. Return false on failure. -static bool parse_time(bstr str, double *out_time) +// Static allocation out of laziness. +#define NUM_MAX_PARAMS 20 + +struct parse_ctx { + struct mp_log *log; + bool error; + bstr param_vals[NUM_MAX_PARAMS]; + bstr param_names[NUM_MAX_PARAMS]; + int num_params; +}; + +// This returns a value with bstr.start==NULL if nothing found. If the parameter +// was specified, bstr.str!=NULL, even if the string is empty (bstr.len==0). +// The parameter is removed from the list if found. +static bstr get_param(struct parse_ctx *ctx, const char *name) { - bstr rest; - double time = bstrtod(str, &rest); - if (!str.len || rest.len || !isfinite(time)) - return false; - *out_time = time; - return true; + bstr bname = bstr0(name); + for (int n = 0; n < ctx->num_params; n++) { + if (bstr_equals(ctx->param_names[n], bname)) { + bstr res = ctx->param_vals[n]; + int count = ctx->num_params; + MP_TARRAY_REMOVE_AT(ctx->param_names, count, n); + count = ctx->num_params; + MP_TARRAY_REMOVE_AT(ctx->param_vals, count, n); + ctx->num_params -= 1; + if (!res.start) + res = bstr0(""); // keep guarantees + return res; + } + } + return (bstr){0}; +} + +// Same as get_param(), but return C string. Return NULL if missing. +static char *get_param0(struct parse_ctx *ctx, void *ta_ctx, const char *name) +{ + return bstrdup0(ta_ctx, get_param(ctx, name)); +} + +// Optional time parameter. Currently a number. +// Returns true: parameter was present and valid, *t is set +// Returns false: parameter was not present (or broken => ctx.error set) +static bool get_param_time(struct parse_ctx *ctx, const char *name, double *t) +{ + bstr val = get_param(ctx, name); + if (val.start) { + bstr rest; + double time = bstrtod(val, &rest); + if (!val.len || rest.len || !isfinite(time)) { + MP_ERR(ctx, "Invalid time string: '%.*s'\n", BSTR_P(val)); + ctx->error = true; + return false; + } + *t = time; + return true; + } + return false; } static struct tl_parts *add_part(struct tl_root *root) @@ -88,7 +135,7 @@ static struct tl_parts *add_part(struct tl_root *root) * entry ::= ( ',' )* * param ::= [ '='] ( | '%' '%' ) */ -static struct tl_root *parse_edl(bstr str) +static struct tl_root *parse_edl(bstr str, struct mp_log *log) { struct tl_root *root = talloc_zero(NULL, struct tl_root); struct tl_parts *tl = add_part(root); @@ -100,9 +147,7 @@ static struct tl_root *parse_edl(bstr str) if (bstr_eatstart0(&str, "\n") || bstr_eatstart0(&str, ";")) continue; bool is_header = bstr_eatstart0(&str, "!"); - bstr f_type = {0}; - bstr f_init = {0}; - struct tl_part p = { .length = -1 }; + struct parse_ctx ctx = { .log = log }; int nparam = 0; while (1) { bstr name, val; @@ -129,42 +174,23 @@ static struct tl_root *parse_edl(bstr str) val = bstr_splice(str, 0, next); str = bstr_cut(str, next); } - // Interpret parameters. Explicitly ignore unknown ones. - if (is_header) { - if (bstr_equals0(name, "type")) { - f_type = val; - } else if (bstr_equals0(name, "init")) { - f_init = val; - } + if (ctx.num_params >= NUM_MAX_PARAMS) { + mp_err(log, "Too many parameters, ignoring '%.*s'.\n", + BSTR_P(name)); } else { - if (bstr_equals0(name, "file")) { - p.filename = bstrto0(tl, val); - } else if (bstr_equals0(name, "start")) { - if (!parse_time(val, &p.offset)) - goto error; - p.offset_set = true; - } else if (bstr_equals0(name, "length")) { - if (!parse_time(val, &p.length)) - goto error; - } else if (bstr_equals0(name, "timestamps")) { - if (bstr_equals0(val, "chapters")) - p.chapter_ts = true; - } else if (bstr_equals0(name, "title")) { - p.title = bstrto0(tl, val); - } else if (bstr_equals0(name, "layout")) { - if (bstr_equals0(val, "this")) - p.is_layout = true; - } + ctx.param_names[ctx.num_params] = name; + ctx.param_vals[ctx.num_params] = val; + ctx.num_params += 1; } nparam++; if (!bstr_eatstart0(&str, ",")) break; } if (is_header) { + bstr f_type = get_param(&ctx, "type"); if (bstr_equals0(f_type, "mp4_dash")) { tl->dash = true; - if (f_init.len) - tl->init_fragment_url = bstrto0(tl, f_init); + tl->init_fragment_url = get_param0(&ctx, tl, "init"); } else if (bstr_equals0(f_type, "no_clip")) { tl->no_clip = true; } else if (bstr_equals0(f_type, "new_stream")) { @@ -172,22 +198,50 @@ static struct tl_root *parse_edl(bstr str) } else if (bstr_equals0(f_type, "no_chapters")) { tl->disable_chapters = true; } else { + mp_err(log, "Unknown header: '%.*s'\n", BSTR_P(f_type)); goto error; } } else { - if (!p.filename) + struct tl_part p = { .length = -1 }; + p.filename = get_param0(&ctx, tl, "file"); + p.offset_set = get_param_time(&ctx, "start", &p.offset); + get_param_time(&ctx, "length", &p.length); + bstr ts = get_param(&ctx, "timestamps"); + if (bstr_equals0(ts, "chapters")) { + p.chapter_ts = true; + } else if (ts.start && !bstr_equals0(ts, "seconds")) { + mp_warn(log, "Unknown timestamp type: '%.*s'\n", BSTR_P(ts)); + } + p.title = get_param0(&ctx, tl, "title"); + bstr layout = get_param(&ctx, "layout"); + if (layout.start) { + if (bstr_equals0(layout, "this")) { + p.is_layout = true; + } else { + mp_warn(log, "Unknown layout param: '%.*s'\n", BSTR_P(layout)); + } + } + if (!p.filename) { + mp_err(log, "Missing filename in segment.'\n"); goto error; + } MP_TARRAY_APPEND(tl, tl->parts, tl->num_parts, p); } - } - if (!root->num_pars) - goto error; - for (int n = 0; n < root->num_pars; n++) { - if (root->pars[n]->num_parts < 1) + if (ctx.error) goto error; + for (int n = 0; n < ctx.num_params; n++) + mp_warn(log, "Unknown parameter: '%.*s'\n", BSTR_P(ctx.param_names[n])); + } + assert(root->num_pars); + for (int n = 0; n < root->num_pars; n++) { + if (root->pars[n]->num_parts < 1) { + mp_err(log, "EDL specifies no segments.'\n"); + goto error; + } } return root; error: + mp_err(log, "EDL parsing failed.\n"); talloc_free(root); return NULL; } @@ -412,7 +466,7 @@ static void build_mpv_edl_timeline(struct timeline *tl) { struct priv *p = tl->demuxer->priv; - struct tl_root *root = parse_edl(p->data); + struct tl_root *root = parse_edl(p->data, tl->log); if (!root) { MP_ERR(tl, "Error in EDL.\n"); return;