| Message ID | 20210121230013.22751-1-jeebjp@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avformat/concatdec: add support for setting input options | 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 |
On Fri, Jan 22, 2021 at 1:00 AM Jan Ekström <jeebjp@gmail.com> wrote: > > This way protocol or format related options can be set for all > of the files opened during concatenation. Ping for reviews. Jan
Jan Ekström (12021-01-22): > This way protocol or format related options can be set for all > of the files opened during concatenation. > --- > libavformat/concatdec.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) I know it is a little more work, but options need to be set segment per segment, in the script file. Thanks for working on this. It has been needed for a long time. Regards,
On Mon, Jan 25, 2021 at 11:52 AM Nicolas George <george@nsup.org> wrote: > > Jan Ekström (12021-01-22): > > This way protocol or format related options can be set for all > > of the files opened during concatenation. > > --- > > libavformat/concatdec.c | 26 +++++++++++++++++++++++++- > > 1 file changed, 25 insertions(+), 1 deletion(-) > > I know it is a little more work, but options need to be set segment per > segment, in the script file. I knew that would be useful, but opted out of it since: 1. For now I didn't yet need it. 2. I wanted to keep out of the actual playlist parsing code. 3. Possible security ramifications (since we already have "safe" / "unsafe" playlist entries). It is a relatively simple extension yes, if you ignore the points 2/3. You have an AVDictionary per file and after initializing the AVDictionary in the opening function and applying global input options - copy the contents of the per-file options there, too. Jan
Jan Ekström (12021-01-25): > I knew that would be useful, but opted out of it since: > 1. For now I didn't yet need it. That is not a good reason. > 2. I wanted to keep out of the actual playlist parsing code. I know, file parsing code is not nice. But this is what needs doing. > 3. Possible security ramifications (since we already have "safe" / > "unsafe" playlist entries). You are right, we need to treat options as unsave. > > It is a relatively simple extension yes, if you ignore the points 2/3. > You have an AVDictionary per file and after initializing the > AVDictionary in the opening function and applying global input options > - copy the contents of the per-file options there, too. Please, no half-baked solution. Regards,
On Mon, Jan 25, 2021 at 2:50 PM Nicolas George <george@nsup.org> wrote: > > Jan Ekström (12021-01-25): > > I knew that would be useful, but opted out of it since: > > 1. For now I didn't yet need it. > > That is not a good reason. > > > 2. I wanted to keep out of the actual playlist parsing code. > > I know, file parsing code is not nice. But this is what needs doing. > > > 3. Possible security ramifications (since we already have "safe" / > > "unsafe" playlist entries). > > You are right, we need to treat options as unsave. > > > > > It is a relatively simple extension yes, if you ignore the points 2/3. > > You have an AVDictionary per file and after initializing the > > AVDictionary in the opening function and applying global input options > > - copy the contents of the per-file options there, too. > > Please, no half-baked solution. > I am pretty sure you mis-understood what I meant here. I meant that if you exclude those bits that I was not interested in (namely, parsing the playlist file and handling the "safe" flag), the extension would be simple. Not meaning half-baked solutions. Pushed a version last night which seems to now include the parsing changes to a branch: https://github.com/jeeb/ffmpeg/commits/add_input_options_to_concatdec If this looks good, I'll just update the documentation and post it on the mailing list. Jan
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c index 6d5b9914f9..3c81d9fea6 100644 --- a/libavformat/concatdec.c +++ b/libavformat/concatdec.c @@ -66,6 +66,7 @@ typedef struct { ConcatMatchMode stream_match_mode; unsigned auto_convert; int segment_time_metadata; + AVDictionary *input_options; } ConcatContext; static int concat_probe(const AVProbeData *probe) @@ -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,31 @@ 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 || + if (cat->input_options && + (ret = av_dict_copy(&options, cat->input_options, 0) < 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); + av_dict_free(&options); return ret; } + + if (av_dict_count(options)) { + AVDictionaryEntry *en = NULL; + + while ((en = av_dict_get(options, "", en, AV_DICT_IGNORE_SUFFIX))) { + av_log(avf, AV_LOG_WARNING, + "Option '%s' set to '%s' was ignored when opening %s " + "with the %s reader!\n", + en->key, en->value, file->url, cat->avf->iformat->name); + } + } + + av_dict_free(&options); + cat->cur_file = file; file->start_time = !fileno ? 0 : cat->files[fileno - 1].start_time + @@ -764,6 +785,9 @@ static const AVOption options[] = { OFFSET(auto_convert), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, DEC }, { "segment_time_metadata", "output file segment start time and duration as packet metadata", OFFSET(segment_time_metadata), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DEC }, + { "input_options", + "set options for all opened inputs using a :-separated list of key=value pairs", + OFFSET(input_options), AV_OPT_TYPE_DICT, { .str = NULL }, 0, 0, DEC }, { NULL } };