diff mbox series

[FFmpeg-devel,v2] ffprobe: Allow unknown format private AVOptions

Message ID 20200628205809.721321-1-derek.buitenhuis@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2] ffprobe: Allow unknown format private AVOptions
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Derek Buitenhuis June 28, 2020, 8:58 p.m. UTC
This useful, because by ffprobe's very nature, you use it to probe
a file and find out what it is. Requiring every format private option
to be known to the demuxer forces one to run ffprobe twice, if one
wants to use ffprobe in a generic way.

For example, say one wants to probe all user-uploaded files, while
also ignoring edit lists for any MP4s that are uploaded. Currently,
you'd have to run ffprobe twice: once to identify the format, and
once again to actually probe the metadata you want. After this
patch, you could set -ignore_editlist 1 on every call and only
probe once.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
Changed to just be the main behavior instead of behind an option,
as Michael suggested.

Didn't really know what would be added to ffprobe.texi, though, since
this is no longer an option.
---
 fftools/ffprobe.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Derek Buitenhuis June 30, 2020, 3:24 p.m. UTC | #1
On 28/06/2020 21:58, Derek Buitenhuis wrote:
> This useful, because by ffprobe's very nature, you use it to probe
> a file and find out what it is. Requiring every format private option
> to be known to the demuxer forces one to run ffprobe twice, if one
> wants to use ffprobe in a generic way.
> 
> For example, say one wants to probe all user-uploaded files, while
> also ignoring edit lists for any MP4s that are uploaded. Currently,
> you'd have to run ffprobe twice: once to identify the format, and
> once again to actually probe the metadata you want. After this
> patch, you could set -ignore_editlist 1 on every call and only
> probe once.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> Changed to just be the main behavior instead of behind an option,
> as Michael suggested.
> 
> Didn't really know what would be added to ffprobe.texi, though, since
> this is no longer an option.
> ---
>  fftools/ffprobe.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Ping.

If nobody objects, I'll push this tomorrow.

- Derek
Marton Balint June 30, 2020, 8:48 p.m. UTC | #2
On Sun, 28 Jun 2020, Derek Buitenhuis wrote:

> This useful, because by ffprobe's very nature, you use it to probe
> a file and find out what it is. Requiring every format private option
> to be known to the demuxer forces one to run ffprobe twice, if one
> wants to use ffprobe in a generic way.
>
> For example, say one wants to probe all user-uploaded files, while
> also ignoring edit lists for any MP4s that are uploaded. Currently,
> you'd have to run ffprobe twice: once to identify the format, and
> once again to actually probe the metadata you want. After this
> patch, you could set -ignore_editlist 1 on every call and only
> probe once.
>
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> Changed to just be the main behavior instead of behind an option,
> as Michael suggested.
>
> Didn't really know what would be added to ffprobe.texi, though, since
> this is no longer an option.
> ---
> fftools/ffprobe.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 5515e1b31b..61191add6b 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -2879,10 +2879,8 @@ static int open_input_file(InputFile *ifile, const char *filename,
>     ifile->fmt_ctx = fmt_ctx;
>     if (scan_all_pmts_set)
>         av_dict_set(&format_opts, "scan_all_pmts", NULL, AV_DICT_MATCH_CASE);
> -    if ((t = av_dict_get(format_opts, "", NULL, AV_DICT_IGNORE_SUFFIX))) {
> -        av_log(NULL, AV_LOG_ERROR, "Option %s not found.\n", t->key);
> -        return AVERROR_OPTION_NOT_FOUND;
> -    }
> +    if ((t = av_dict_get(format_opts, "", NULL, AV_DICT_IGNORE_SUFFIX)))
> +        av_log(NULL, AV_LOG_WARNING, "Option %s skipped - not known to demuxer.\n", t->key);

This only logs the first skipped option. I think it is best if you 
iterate through all of them.

Regards,
Marton
Derek Buitenhuis July 1, 2020, 3:34 p.m. UTC | #3
On 30/06/2020 21:48, Marton Balint wrote:
> This only logs the first skipped option. I think it is best if you 
> iterate through all of them.

Done. v3 sent.

- Derek
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 5515e1b31b..61191add6b 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2879,10 +2879,8 @@  static int open_input_file(InputFile *ifile, const char *filename,
     ifile->fmt_ctx = fmt_ctx;
     if (scan_all_pmts_set)
         av_dict_set(&format_opts, "scan_all_pmts", NULL, AV_DICT_MATCH_CASE);
-    if ((t = av_dict_get(format_opts, "", NULL, AV_DICT_IGNORE_SUFFIX))) {
-        av_log(NULL, AV_LOG_ERROR, "Option %s not found.\n", t->key);
-        return AVERROR_OPTION_NOT_FOUND;
-    }
+    if ((t = av_dict_get(format_opts, "", NULL, AV_DICT_IGNORE_SUFFIX)))
+        av_log(NULL, AV_LOG_WARNING, "Option %s skipped - not known to demuxer.\n", t->key);
 
     if (find_stream_info) {
         AVDictionary **opts = setup_find_stream_info_opts(fmt_ctx, codec_opts);