Message ID | 20210728121517.533173-2-george@nsup.org |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/5] libavformat/concatdec: remove support for unsafe=-1. | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
Nicolas George: > Signed-off-by: Nicolas George <george@nsup.org> > --- > doc/demuxers.texi | 4 ++++ > libavformat/concatdec.c | 37 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 40 insertions(+), 1 deletion(-) > > > I intend to refactor the parser some time later. > > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > index 5f18e4551b..eb3351833a 100644 > --- a/doc/demuxers.texi > +++ b/doc/demuxers.texi > @@ -151,6 +151,10 @@ Metadata of the packets of the file. The specified metadata will be set for > each file packet. You can specify this directive multiple times to add multiple > metadata entries. > > +@item @code{option @var{key} @var{value}} > +Option to access, open and probe the file. > +Can be present multiple times. > + > @item @code{stream} > Introduce a stream in the virtual file. > All subsequent stream-related directives apply to the last introduced > diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c > index 3db1693725..0203b1e6dc 100644 > --- a/libavformat/concatdec.c > +++ b/libavformat/concatdec.c > @@ -51,6 +51,7 @@ typedef struct { > int64_t inpoint; > int64_t outpoint; > AVDictionary *metadata; > + AVDictionary *options; > int nb_streams; > } ConcatFile; > > @@ -329,6 +330,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno) > { > ConcatContext *cat = avf->priv_data; > ConcatFile *file = &cat->files[fileno]; > + AVDictionary *options = NULL; > int ret; > > if (cat->avf) > @@ -344,12 +346,21 @@ static int open_file(AVFormatContext *avf, unsigned fileno) > if ((ret = ff_copy_whiteblacklists(cat->avf, avf)) < 0) > return ret; > > - if ((ret = avformat_open_input(&cat->avf, file->url, NULL, NULL)) < 0 || > + ret = av_dict_copy(&options, file->options, 0); > + if (ret < 0) > + return ret; > + > + if ((ret = avformat_open_input(&cat->avf, file->url, NULL, &options)) < 0 || > (ret = avformat_find_stream_info(cat->avf, NULL)) < 0) { > av_log(avf, AV_LOG_ERROR, "Impossible to open '%s'\n", file->url); > avformat_close_input(&cat->avf); > return ret; options would leak here. > } > + if (options) { > + av_log(avf, AV_LOG_WARNING, "Unused options for '%s'.\n", file->url); > + /* TODO log unused options once we have a proper string API */ > + av_dict_free(&options); > + } > cat->cur_file = file; > file->start_time = !fileno ? 0 : > cat->files[fileno - 1].start_time + > @@ -386,6 +397,7 @@ static int concat_read_close(AVFormatContext *avf) > } > av_freep(&cat->files[i].streams); > av_dict_free(&cat->files[i].metadata); > + av_dict_free(&cat->files[i].options); > } > if (cat->avf) > avformat_close_input(&cat->avf); > @@ -457,6 +469,29 @@ static int concat_read_header(AVFormatContext *avf) > 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_log(avf, AV_LOG_ERROR, "Line %d: key and val required\n", line); > + FAIL(AVERROR_INVALIDDATA); 1. key might leak here. 2. The error message and the return code are wrong: av_get_token() only fails upon allocation error; it does not fail when *cursor == \0. (The code already suffers from this misconception.) > + } > + ret = av_dict_set(&file->options, key, val, > + AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL); > + if (ret < 0) { > + av_freep(&key); > + av_freep(&val); Double free. > + FAIL(ret); > + } > } else if (!strcmp(keyword, "stream")) { > if (!avformat_new_stream(avf, NULL)) > FAIL(AVERROR(ENOMEM)); >
Nicolas George: > Signed-off-by: Nicolas George <george@nsup.org> > --- > doc/demuxers.texi | 4 ++++ > libavformat/concatdec.c | 37 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 40 insertions(+), 1 deletion(-) > > > I intend to refactor the parser some time later. > > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > index 5f18e4551b..eb3351833a 100644 > --- a/doc/demuxers.texi > +++ b/doc/demuxers.texi > @@ -151,6 +151,10 @@ Metadata of the packets of the file. The specified metadata will be set for > each file packet. You can specify this directive multiple times to add multiple > metadata entries. > > +@item @code{option @var{key} @var{value}} > +Option to access, open and probe the file. > +Can be present multiple times. > + > @item @code{stream} > Introduce a stream in the virtual file. > All subsequent stream-related directives apply to the last introduced > diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c > index 3db1693725..0203b1e6dc 100644 > --- a/libavformat/concatdec.c > +++ b/libavformat/concatdec.c > @@ -51,6 +51,7 @@ typedef struct { > int64_t inpoint; > int64_t outpoint; > AVDictionary *metadata; > + AVDictionary *options; > int nb_streams; > } ConcatFile; > > @@ -329,6 +330,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno) > { > ConcatContext *cat = avf->priv_data; > ConcatFile *file = &cat->files[fileno]; > + AVDictionary *options = NULL; > int ret; > > if (cat->avf) > @@ -344,12 +346,21 @@ static int open_file(AVFormatContext *avf, unsigned fileno) > if ((ret = ff_copy_whiteblacklists(cat->avf, avf)) < 0) > return ret; > > - if ((ret = avformat_open_input(&cat->avf, file->url, NULL, NULL)) < 0 || > + ret = av_dict_copy(&options, file->options, 0); > + if (ret < 0) > + return ret; In case of failure, options needs to be freed. - Andreas
diff --git a/doc/demuxers.texi b/doc/demuxers.texi index 5f18e4551b..eb3351833a 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -151,6 +151,10 @@ Metadata of the packets of the file. The specified metadata will be set for each file packet. You can specify this directive multiple times to add multiple metadata entries. +@item @code{option @var{key} @var{value}} +Option to access, open and probe the file. +Can be present multiple times. + @item @code{stream} Introduce a stream in the virtual file. All subsequent stream-related directives apply to the last introduced diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index 3db1693725..0203b1e6dc 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -51,6 +51,7 @@ typedef struct { int64_t inpoint; int64_t outpoint; AVDictionary *metadata; + AVDictionary *options; int nb_streams; } ConcatFile; @@ -329,6 +330,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno) { ConcatContext *cat = avf->priv_data; ConcatFile *file = &cat->files[fileno]; + AVDictionary *options = NULL; int ret; if (cat->avf) @@ -344,12 +346,21 @@ static int open_file(AVFormatContext *avf, unsigned fileno) if ((ret = ff_copy_whiteblacklists(cat->avf, avf)) < 0) return ret; - if ((ret = avformat_open_input(&cat->avf, file->url, NULL, NULL)) < 0 || + ret = av_dict_copy(&options, file->options, 0); + if (ret < 0) + return ret; + + if ((ret = avformat_open_input(&cat->avf, file->url, NULL, &options)) < 0 || (ret = avformat_find_stream_info(cat->avf, NULL)) < 0) { av_log(avf, AV_LOG_ERROR, "Impossible to open '%s'\n", file->url); avformat_close_input(&cat->avf); return ret; } + if (options) { + av_log(avf, AV_LOG_WARNING, "Unused options for '%s'.\n", file->url); + /* TODO log unused options once we have a proper string API */ + av_dict_free(&options); + } cat->cur_file = file; file->start_time = !fileno ? 0 : cat->files[fileno - 1].start_time + @@ -386,6 +397,7 @@ static int concat_read_close(AVFormatContext *avf) } av_freep(&cat->files[i].streams); av_dict_free(&cat->files[i].metadata); + av_dict_free(&cat->files[i].options); } if (cat->avf) avformat_close_input(&cat->avf); @@ -457,6 +469,29 @@ static int concat_read_header(AVFormatContext *avf) 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_log(avf, AV_LOG_ERROR, "Line %d: key and val required\n", line); + FAIL(AVERROR_INVALIDDATA); + } + ret = av_dict_set(&file->options, key, val, + AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL); + if (ret < 0) { + av_freep(&key); + av_freep(&val); + FAIL(ret); + } } else if (!strcmp(keyword, "stream")) { if (!avformat_new_stream(avf, NULL)) FAIL(AVERROR(ENOMEM));
Signed-off-by: Nicolas George <george@nsup.org> --- doc/demuxers.texi | 4 ++++ libavformat/concatdec.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) I intend to refactor the parser some time later.