From patchwork Tue Aug 31 12:22:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas George X-Patchwork-Id: 29902 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6602:2a4a:0:0:0:0 with SMTP id k10csp4873342iov; Tue, 31 Aug 2021 05:22:28 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw30mfrawiEDXPTpznTk8uxJQ5DBGAuEojsayxtHkMA2CIyvqeubxEVTOAKzc0zIP4Cu17i X-Received: by 2002:aa7:d3d6:: with SMTP id o22mr12396610edr.155.1630412548820; Tue, 31 Aug 2021 05:22:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1630412548; cv=none; d=google.com; s=arc-20160816; b=tp9qQEBavhea3ZvAjSmftWEkBxEGJ4yE7cLZOMZ2GJD0I8bZhw/StFDEjbBR6B+FkO EgQXiCc1KnFo5x47GEpp8Tn+nqK6ck15k51Z0Z7VJP8O33KXbZiCMfADUEF8XB0jRhxK AJbWFIq6b4NFf1I+8oPxKCOj0ce43Zc+a7/No+5iEry4LV4u5fjHkzEzipEAXV+bxYXl uEP41xqoZhIJJOOqYvaCIEudBYufby4hSXc2Hy97QBnvcS4mMI6R3Gl+1SdtVeFRntns mBf0wnczWZr2aM94+bOy2+zbrM4vzZccMEj+mcjbHAKVr4ofjJhWJXp0zSQ3YEu5rAQU JDBw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:reply-to:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:mime-version:message-id:date:to:from :delivered-to; bh=FbwgD403bSPD24jBocjfQaQwyb0eKL+2iQYNtTwiI/k=; b=F5FIMJ8x38mFhU2/nVjZ1/wpT0XsrN8gtIz8lQr6O/v63tM1iXxfCIku6RqG2Jp9fN NzRqB4u9Pcn/C1diU5oHmUnHQZXmU2t0YnflxoqmLosdSBUpySBYHXvFSJKST0H+8F8t T/xOoWQQGroWEMyerd0SFnAtmDjh1q25cpHSGwu1xouANcNzgsABrpyC+TNpSp34xSx7 SdL0ezR/QMKqmBT3H2Z5GsRgsagYloD2Z0F/+XfQDBTX/9teioqJtsgzHjZu6gSEAnsk lALgLO68O9IOIi6brqu5Cxwpu88aF9FzzhcUBRKAqkdBqdi4S9haYZOK35i2XLyZjzAU BS9g== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id t21si17642815ejy.151.2021.08.31.05.22.24; Tue, 31 Aug 2021 05:22:28 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6049B68A134; Tue, 31 Aug 2021 15:22:19 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D147968016E for ; Tue, 31 Aug 2021 15:22:12 +0300 (EEST) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 17VCMCT8019249 for ; Tue, 31 Aug 2021 14:22:12 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id F0996EB5BC; Tue, 31 Aug 2021 14:22:11 +0200 (CEST) From: Nicolas George To: ffmpeg-devel@ffmpeg.org Date: Tue, 31 Aug 2021 14:22:04 +0200 Message-Id: <20210831122209.586348-1-george@nsup.org> X-Mailer: git-send-email 2.33.0 MIME-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Tue, 31 Aug 2021 14:22:12 +0200 (CEST) Subject: [FFmpeg-devel] [PATCH 1/6] lavf/concat: refactor parsing X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: fHXF5cqCx3qB Signed-off-by: Nicolas George --- libavformat/concatdec.c | 245 +++++++++++++++++++++++++--------------- 1 file changed, 157 insertions(+), 88 deletions(-) It does not make the code shorter, but it makes it clearer and reduces the risk of mistakes, like the ones I made myself recently. Also, it makes adding new directives much simpler. And I am about to add quite a few. diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index 30db456b0e..223c7e36c4 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -19,6 +19,7 @@ */ #include "libavutil/avstring.h" +#include "libavutil/avassert.h" #include "libavutil/bprint.h" #include "libavutil/intreadwrite.h" #include "libavutil/opt.h" @@ -407,15 +408,54 @@ static int concat_read_close(AVFormatContext *avf) return 0; } -static int concat_read_header(AVFormatContext *avf) +#define MAX_ARGS 2 +#define NEEDS_UNSAFE (1 << 1) +#define NEEDS_FILE (1 << 1) +#define NEEDS_STREAM (1 << 2) + +typedef struct ParseSyntax { + const char *keyword; + char args[MAX_ARGS]; + uint8_t flags; +} ParseSyntax; + +typedef enum ParseDirective { + DIR_FFCONCAT, + DIR_FILE, + DIR_DURATION, + DIR_INPOINT, + DIR_OUTPOINT, + DIR_FPMETAS, + DIR_OPTION, + DIR_STREAM, + DIR_EXSID, +} ParseDirective; + +static const ParseSyntax syntax[] = { + [DIR_FFCONCAT ] = { "ffconcat", "kk", 0 }, + [DIR_FILE ] = { "file", "s", 0 }, + [DIR_DURATION ] = { "duration", "d", NEEDS_FILE }, + [DIR_INPOINT ] = { "inpoint", "d", NEEDS_FILE }, + [DIR_OUTPOINT ] = { "outpoint", "d", NEEDS_FILE }, + [DIR_FPMETAS ] = { "file_packet_metadata", "s", NEEDS_FILE }, + [DIR_OPTION ] = { "option", "ks", NEEDS_FILE | NEEDS_UNSAFE }, + [DIR_STREAM ] = { "stream", "", 0 }, + [DIR_EXSID ] = { "exact_stream_id", "i", NEEDS_STREAM }, +}; + +static int concat_parse_script(AVFormatContext *avf) { ConcatContext *cat = avf->priv_data; + unsigned nb_files_alloc = 0; AVBPrint bp; uint8_t *cursor, *keyword; - int line = 0, i; - unsigned nb_files_alloc = 0; ConcatFile *file = NULL; - int64_t ret, time = 0; + unsigned line = 0, arg; + const ParseSyntax *dir; + char *arg_kw[MAX_ARGS]; + char *arg_str[MAX_ARGS] = { 0 }; + int64_t arg_int[MAX_ARGS]; + int ret; av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); @@ -425,100 +465,131 @@ static int concat_read_header(AVFormatContext *avf) keyword = get_keyword(&cursor); if (!*keyword || *keyword == '#') continue; + for (dir = syntax; dir < syntax + FF_ARRAY_ELEMS(syntax); dir++) + if (!strcmp(dir->keyword, keyword)) + break; + if (dir >= syntax + FF_ARRAY_ELEMS(syntax)) { + av_log(avf, AV_LOG_ERROR, "Line %d: unknown keyword '%s'\n", + line, keyword); + FAIL(AVERROR_INVALIDDATA); + } - if (!strcmp(keyword, "file")) { - char *filename = av_get_token((const char **)&cursor, SPACE_CHARS); - if (!filename) { - av_log(avf, AV_LOG_ERROR, "Line %d: filename required\n", line); - FAIL(AVERROR_INVALIDDATA); + /* Flags check */ + if ((dir->flags & NEEDS_UNSAFE) && cat->safe) { + av_log(avf, AV_LOG_ERROR, "Line %d: %s not allowed if safe\n", line, keyword); + FAIL(AVERROR_INVALIDDATA); + } + if ((dir->flags & NEEDS_FILE) && !cat->nb_files) { + av_log(avf, AV_LOG_ERROR, "Line %d: %s without file\n", line, keyword); + FAIL(AVERROR_INVALIDDATA); + } + if ((dir->flags & NEEDS_STREAM) && !avf->nb_streams) { + av_log(avf, AV_LOG_ERROR, "Line %d: %s without stream\n", line, keyword); + FAIL(AVERROR_INVALIDDATA); + } + + /* Arguments parsing */ + for (arg = 0; arg < FF_ARRAY_ELEMS(dir->args) && dir->args[arg]; arg++) { + switch (dir->args[arg]) { + case 'd': /* duration */ + arg_kw[arg] = get_keyword(&cursor); + ret = av_parse_time(&arg_int[arg], arg_kw[arg], 1); + if (ret < 0) { + av_log(avf, AV_LOG_ERROR, "Line %d: invalid duration '%s'\n", + line, arg_kw[arg]); + goto fail; + } + break; + case 'i': /* integer */ + arg_int[arg] = strtol(get_keyword(&cursor), NULL, 0); + break; + case 'k': /* keyword */ + arg_kw[arg] = get_keyword(&cursor); + break; + case 's': /* string */ + av_assert0(!arg_str[arg]); + arg_str[arg] = av_get_token((const char **)&cursor, SPACE_CHARS); + if (!arg_str[arg]) + FAIL(AVERROR(ENOMEM)); + if (!*arg_str[arg]) { + av_log(avf, AV_LOG_ERROR, "Line %d: string required\n", line); + FAIL(AVERROR_INVALIDDATA); + } + break; + default: + FAIL(AVERROR_BUG); } - if ((ret = add_file(avf, filename, &file, &nb_files_alloc)) < 0) - goto fail; - } else if (!strcmp(keyword, "duration") || !strcmp(keyword, "inpoint") || !strcmp(keyword, "outpoint")) { - char *dur_str = get_keyword(&cursor); - int64_t dur; - if (!file) { - av_log(avf, AV_LOG_ERROR, "Line %d: %s without file\n", - line, keyword); + } + + /* Directive action */ + switch ((ParseDirective)(dir - syntax)) { + case DIR_FFCONCAT: + if (strcmp(arg_kw[0], "version") || strcmp(arg_kw[1], "1.0")) { + av_log(avf, AV_LOG_ERROR, "Line %d: invalid version\n", line); FAIL(AVERROR_INVALIDDATA); } - if ((ret = av_parse_time(&dur, dur_str, 1)) < 0) { - av_log(avf, AV_LOG_ERROR, "Line %d: invalid %s '%s'\n", - line, keyword, dur_str); + break; + case DIR_FILE: + ret = add_file(avf, arg_str[0], &file, &nb_files_alloc); + arg_str[0] = NULL; + if (ret < 0) goto fail; - } - if (!strcmp(keyword, "duration")) - file->user_duration = dur; - else if (!strcmp(keyword, "inpoint")) - file->inpoint = dur; - else if (!strcmp(keyword, "outpoint")) - file->outpoint = dur; - } else if (!strcmp(keyword, "file_packet_metadata")) { - char *metadata; - if (!file) { - av_log(avf, AV_LOG_ERROR, "Line %d: %s without file\n", - line, keyword); - FAIL(AVERROR_INVALIDDATA); - } - metadata = av_get_token((const char **)&cursor, SPACE_CHARS); - if (!metadata) { - av_log(avf, AV_LOG_ERROR, "Line %d: packet metadata required\n", line); - FAIL(AVERROR_INVALIDDATA); - } - if ((ret = av_dict_parse_string(&file->metadata, metadata, "=", "", 0)) < 0) { + break; + case DIR_DURATION: + file->user_duration = arg_int[0]; + break; + case DIR_INPOINT: + file->inpoint = arg_int[0]; + break; + case DIR_OUTPOINT: + file->outpoint = arg_int[0]; + break; + case DIR_FPMETAS: + if ((ret = av_dict_parse_string(&file->metadata, arg_str[0], "=", "", 0)) < 0) { av_log(avf, AV_LOG_ERROR, "Line %d: failed to parse metadata string\n", line); - av_freep(&metadata); FAIL(AVERROR_INVALIDDATA); } - av_freep(&metadata); - } else if (!strcmp(keyword, "option")) { - char *key, *val; - if (cat->safe) { - av_log(avf, AV_LOG_ERROR, "Options not permitted in safe mode.\n"); - FAIL(AVERROR(EPERM)); - } - if (!file) { - av_log(avf, AV_LOG_ERROR, "Line %d: %s without file\n", - line, keyword); - FAIL(AVERROR_INVALIDDATA); - } - if (!(key = av_get_token((const char **)&cursor, SPACE_CHARS)) || - !(val = av_get_token((const char **)&cursor, SPACE_CHARS))) { - av_freep(&key); - FAIL(AVERROR(ENOMEM)); - } - ret = av_dict_set(&file->options, key, val, - AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL); + av_freep(&arg_str[0]); + break; + case DIR_OPTION: + ret = av_dict_set(&file->options, arg_kw[0], arg_str[1], AV_DICT_DONT_STRDUP_VAL); if (ret < 0) FAIL(ret); - } else if (!strcmp(keyword, "stream")) { + arg_str[1] = NULL; + break; + case DIR_STREAM: if (!avformat_new_stream(avf, NULL)) FAIL(AVERROR(ENOMEM)); - } else if (!strcmp(keyword, "exact_stream_id")) { - if (!avf->nb_streams) { - av_log(avf, AV_LOG_ERROR, "Line %d: exact_stream_id without stream\n", - line); - FAIL(AVERROR_INVALIDDATA); - } - avf->streams[avf->nb_streams - 1]->id = - strtol(get_keyword(&cursor), NULL, 0); - } else if (!strcmp(keyword, "ffconcat")) { - char *ver_kw = get_keyword(&cursor); - char *ver_val = get_keyword(&cursor); - if (strcmp(ver_kw, "version") || strcmp(ver_val, "1.0")) { - av_log(avf, AV_LOG_ERROR, "Line %d: invalid version\n", line); - FAIL(AVERROR_INVALIDDATA); - } - } else { - av_log(avf, AV_LOG_ERROR, "Line %d: unknown keyword '%s'\n", - line, keyword); - FAIL(AVERROR_INVALIDDATA); + break; + case DIR_EXSID: + avf->streams[avf->nb_streams - 1]->id = arg_int[0]; + break; + default: + FAIL(AVERROR_BUG); } } - if (ret != AVERROR_EOF && ret < 0) - goto fail; - if (!cat->nb_files) - FAIL(AVERROR_INVALIDDATA); + +fail: + for (arg = 0; arg < MAX_ARGS; arg++) + av_freep(&arg_str[arg]); + av_bprint_finalize(&bp, NULL); + return ret == AVERROR_EOF ? 0 : ret; +} + +static int concat_read_header(AVFormatContext *avf) +{ + ConcatContext *cat = avf->priv_data; + int64_t time = 0; + unsigned i; + int ret; + + ret = concat_parse_script(avf); + if (ret < 0) + return ret; + if (!cat->nb_files) { + av_log(avf, AV_LOG_ERROR, "No files to concat\n"); + return AVERROR_INVALIDDATA; + } for (i = 0; i < cat->nb_files; i++) { if (cat->files[i].start_time == AV_NOPTS_VALUE) @@ -541,11 +612,9 @@ static int concat_read_header(AVFormatContext *avf) cat->stream_match_mode = avf->nb_streams ? MATCH_EXACT_ID : MATCH_ONE_TO_ONE; if ((ret = open_file(avf, 0)) < 0) - goto fail; + return ret; -fail: - av_bprint_finalize(&bp, NULL); - return ret; + return 0; } static int open_next_file(AVFormatContext *avf)