Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/3] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used
@ 2024-06-27  8:44 Anton Khirnov
  2024-06-27  8:44 ` [FFmpeg-devel] [PATCH 2/3] fftools/ffmpeg_mux_init: apply encoder options manually Anton Khirnov
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Anton Khirnov @ 2024-06-27  8:44 UTC (permalink / raw)
  To: ffmpeg-devel

Share the code between encoding and decoding. Instead of checking every
stream's options dictionary (which is also used for other purposes),
track all used options in a dedicated dictionary.
---
 fftools/cmdutils.c        | 17 ++++++++----
 fftools/cmdutils.h        |  4 ++-
 fftools/ffmpeg.c          | 50 ++++++++++++++++++++++++++++++++++
 fftools/ffmpeg.h          |  4 ++-
 fftools/ffmpeg_demux.c    | 50 ++++++++--------------------------
 fftools/ffmpeg_mux.c      |  1 +
 fftools/ffmpeg_mux.h      |  3 +++
 fftools/ffmpeg_mux_init.c | 56 +++++----------------------------------
 fftools/ffmpeg_opt.c      | 18 -------------
 fftools/ffplay.c          |  2 +-
 fftools/ffprobe.c         |  2 +-
 11 files changed, 91 insertions(+), 116 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index a504acb3e4..9b18cf5e4d 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -986,7 +986,7 @@ int check_stream_specifier(AVFormatContext *s, AVStream *st, const char *spec)
 
 int filter_codec_opts(const AVDictionary *opts, enum AVCodecID codec_id,
                       AVFormatContext *s, AVStream *st, const AVCodec *codec,
-                      AVDictionary **dst)
+                      AVDictionary **dst, AVDictionary **opts_used)
 {
     AVDictionary    *ret = NULL;
     const AVDictionaryEntry *t = NULL;
@@ -1013,6 +1013,7 @@ int filter_codec_opts(const AVDictionary *opts, enum AVCodecID codec_id,
     while (t = av_dict_iterate(opts, t)) {
         const AVClass *priv_class;
         char *p = strchr(t->key, ':');
+        int used = 0;
 
         /* check stream specification in opt name */
         if (p) {
@@ -1030,15 +1031,21 @@ int filter_codec_opts(const AVDictionary *opts, enum AVCodecID codec_id,
             !codec ||
             ((priv_class = codec->priv_class) &&
              av_opt_find(&priv_class, t->key, NULL, flags,
-                         AV_OPT_SEARCH_FAKE_OBJ)))
+                         AV_OPT_SEARCH_FAKE_OBJ))) {
             av_dict_set(&ret, t->key, t->value, 0);
-        else if (t->key[0] == prefix &&
+            used = 1;
+        } else if (t->key[0] == prefix &&
                  av_opt_find(&cc, t->key + 1, NULL, flags,
-                             AV_OPT_SEARCH_FAKE_OBJ))
+                             AV_OPT_SEARCH_FAKE_OBJ)) {
             av_dict_set(&ret, t->key + 1, t->value, 0);
+            used = 1;
+        }
 
         if (p)
             *p = ':';
+
+        if (used && opts_used)
+            av_dict_set(opts_used, t->key, "", 0);
     }
 
     *dst = ret;
@@ -1063,7 +1070,7 @@ int setup_find_stream_info_opts(AVFormatContext *s,
 
     for (int i = 0; i < s->nb_streams; i++) {
         ret = filter_codec_opts(codec_opts, s->streams[i]->codecpar->codec_id,
-                                s, s->streams[i], NULL, &opts[i]);
+                                s, s->streams[i], NULL, &opts[i], NULL);
         if (ret < 0)
             goto fail;
     }
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 2125f791d0..14340dff7d 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -371,11 +371,13 @@ int check_stream_specifier(AVFormatContext *s, AVStream *st, const char *spec);
  * @param codec The particular codec for which the options should be filtered.
  *              If null, the default one is looked up according to the codec id.
  * @param dst a pointer to the created dictionary
+ * @param opts_used if non-NULL, every option stored in dst is also stored here,
+ *                  with specifiers preserved
  * @return a non-negative number on success, a negative error code on failure
  */
 int filter_codec_opts(const AVDictionary *opts, enum AVCodecID codec_id,
                       AVFormatContext *s, AVStream *st, const AVCodec *codec,
-                      AVDictionary **dst);
+                      AVDictionary **dst, AVDictionary **opts_used);
 
 /**
  * Setup AVCodecContext options for avformat_find_stream_info().
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 88ce3007e8..61593c842f 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -473,6 +473,56 @@ const FrameData *packet_data_c(AVPacket *pkt)
     return ret < 0 ? NULL : (const FrameData*)pkt->opaque_ref->data;
 }
 
+int check_avoptions_used(const AVDictionary *opts, const AVDictionary *opts_used,
+                         void *logctx, int decode)
+{
+    const AVClass  *class = avcodec_get_class();
+    const AVClass *fclass = avformat_get_class();
+
+    const int flag = decode ? AV_OPT_FLAG_DECODING_PARAM :
+                              AV_OPT_FLAG_ENCODING_PARAM;
+    const AVDictionaryEntry *e = NULL;
+
+    while ((e = av_dict_iterate(opts, e))) {
+        const AVOption *option, *foption;
+        char *optname, *p;
+
+        if (av_dict_get(opts_used, e->key, NULL, 0))
+            continue;
+
+        optname = av_strdup(e->key);
+        if (!optname)
+            return AVERROR(ENOMEM);
+
+        p = strchr(optname, ':');
+        if (p)
+            *p = 0;
+
+        option = av_opt_find(&class, optname, NULL, 0,
+                             AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
+        foption = av_opt_find(&fclass, optname, NULL, 0,
+                              AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
+        av_freep(&optname);
+        if (!option || foption)
+            continue;
+
+        if (!(option->flags & flag)) {
+            av_log(logctx, AV_LOG_ERROR, "Codec AVOption %s (%s) is not a %s "
+                   "option.\n", option->name, option->help ? option->help : "",
+                   decode ? "decoding" : "encoding");
+            return AVERROR(EINVAL);
+        }
+
+        av_log(logctx, AV_LOG_WARNING, "Codec AVOption %s (%s) has not been used "
+               "for any stream. The most likely reason is either wrong type "
+               "(e.g. a video option with no video streams) or that it is a "
+               "private option of some decoder which was not actually used "
+               "for any stream.\n", option->name, option->help ? option->help : "");
+    }
+
+    return 0;
+}
+
 void update_benchmark(const char *fmt, ...)
 {
     if (do_benchmark_all) {
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index fe75706afd..8e849bbedc 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -710,8 +710,10 @@ void term_exit(void);
 
 void show_usage(void);
 
+int check_avoptions_used(const AVDictionary *opts, const AVDictionary *opts_used,
+                         void *logctx, int decode);
+
 int assert_file_overwrite(const char *filename);
-AVDictionary *strip_specifiers(const AVDictionary *dict);
 int find_codec(void *logctx, const char *name,
                enum AVMediaType type, int encoder, const AVCodec **codec);
 int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, int st_idx, int is_global);
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 1ca8d804ae..3762d589e3 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1207,7 +1207,7 @@ static DemuxStream *demux_stream_alloc(Demuxer *d, AVStream *st)
     return ds;
 }
 
-static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
+static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st, AVDictionary **opts_used)
 {
     AVFormatContext *ic = d->f.ctx;
     AVCodecParameters *par = st->codecpar;
@@ -1334,7 +1334,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
 
     if (ist->dec) {
         ret = filter_codec_opts(o->g->codec_opts, ist->st->codecpar->codec_id,
-                                ic, st, ist->dec, &ds->decoder_opts);
+                                ic, st, ist->dec, &ds->decoder_opts, opts_used);
         if (ret < 0)
             return ret;
     }
@@ -1553,8 +1553,7 @@ int ifile_open(const OptionsContext *o, const char *filename, Scheduler *sch)
     const AVInputFormat *file_iformat = NULL;
     int err, i, ret = 0;
     int64_t timestamp;
-    AVDictionary *unused_opts = NULL;
-    const AVDictionaryEntry *e = NULL;
+    AVDictionary *opts_used = NULL;
     const char*    video_codec_name = NULL;
     const char*    audio_codec_name = NULL;
     const char* subtitle_codec_name = NULL;
@@ -1826,48 +1825,21 @@ int ifile_open(const OptionsContext *o, const char *filename, Scheduler *sch)
 
     /* Add all the streams from the given input file to the demuxer */
     for (int i = 0; i < ic->nb_streams; i++) {
-        ret = ist_add(o, d, ic->streams[i]);
-        if (ret < 0)
+        ret = ist_add(o, d, ic->streams[i], &opts_used);
+        if (ret < 0) {
+            av_dict_free(&opts_used);
             return ret;
+        }
     }
 
     /* dump the file content */
     av_dump_format(ic, f->index, filename, 0);
 
     /* check if all codec options have been used */
-    unused_opts = strip_specifiers(o->g->codec_opts);
-    for (i = 0; i < f->nb_streams; i++) {
-        DemuxStream *ds = ds_from_ist(f->streams[i]);
-        e = NULL;
-        while ((e = av_dict_iterate(ds->decoder_opts, e)))
-            av_dict_set(&unused_opts, e->key, NULL, 0);
-    }
-
-    e = NULL;
-    while ((e = av_dict_iterate(unused_opts, e))) {
-        const AVClass *class = avcodec_get_class();
-        const AVOption *option = av_opt_find(&class, e->key, NULL, 0,
-                                             AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
-        const AVClass *fclass = avformat_get_class();
-        const AVOption *foption = av_opt_find(&fclass, e->key, NULL, 0,
-                                             AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
-        if (!option || foption)
-            continue;
-
-
-        if (!(option->flags & AV_OPT_FLAG_DECODING_PARAM)) {
-            av_log(d, AV_LOG_ERROR, "Codec AVOption %s (%s) is not a decoding "
-                   "option.\n", e->key, option->help ? option->help : "");
-            return AVERROR(EINVAL);
-        }
-
-        av_log(d, AV_LOG_WARNING, "Codec AVOption %s (%s) has not been used "
-               "for any stream. The most likely reason is either wrong type "
-               "(e.g. a video option with no video streams) or that it is a "
-               "private option of some decoder which was not actually used "
-               "for any stream.\n", e->key, option->help ? option->help : "");
-    }
-    av_dict_free(&unused_opts);
+    ret = check_avoptions_used(o->g->codec_opts, opts_used, d, 1);
+    av_dict_free(&opts_used);
+    if (ret < 0)
+        return ret;
 
     for (i = 0; i < o->dump_attachment.nb_opt; i++) {
         int j;
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index a1583edd61..055e2f3678 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -865,6 +865,7 @@ void of_free(OutputFile **pof)
     av_freep(&mux->sch_stream_idx);
 
     av_dict_free(&mux->opts);
+    av_dict_free(&mux->enc_opts_used);
 
     av_packet_free(&mux->sq_pkt);
 
diff --git a/fftools/ffmpeg_mux.h b/fftools/ffmpeg_mux.h
index 1e9ea35412..1c1b407484 100644
--- a/fftools/ffmpeg_mux.h
+++ b/fftools/ffmpeg_mux.h
@@ -99,6 +99,9 @@ typedef struct Muxer {
 
     AVDictionary           *opts;
 
+    // used to validate that all encoder avoptions have been actually used
+    AVDictionary           *enc_opts_used;
+
     /* filesize limit expressed in bytes */
     int64_t                 limit_filesize;
     atomic_int_least64_t    last_filesize;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 1f19d9283a..b1250d668f 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1160,7 +1160,8 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         const char *enc_time_base = NULL;
 
         ret = filter_codec_opts(o->g->codec_opts, enc->codec_id,
-                                oc, st, enc->codec, &ost->encoder_opts);
+                                oc, st, enc->codec, &ost->encoder_opts,
+                                &mux->enc_opts_used);
         if (ret < 0)
             return ret;
 
@@ -1265,7 +1266,8 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         }
     } else {
         ret = filter_codec_opts(o->g->codec_opts, AV_CODEC_ID_NONE, oc, st,
-                                NULL, &ost->encoder_opts);
+                                NULL, &ost->encoder_opts,
+                                &mux->enc_opts_used);
         if (ret < 0)
             return ret;
     }
@@ -3118,53 +3120,6 @@ static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
     return 0;
 }
 
-static int validate_enc_avopt(Muxer *mux, const AVDictionary *codec_avopt)
-{
-    const AVClass *class  = avcodec_get_class();
-    const AVClass *fclass = avformat_get_class();
-    const OutputFile *of = &mux->of;
-
-    AVDictionary *unused_opts;
-    const AVDictionaryEntry *e;
-
-    unused_opts = strip_specifiers(codec_avopt);
-    for (int i = 0; i < of->nb_streams; i++) {
-        e = NULL;
-        while ((e = av_dict_iterate(of->streams[i]->encoder_opts, e)))
-            av_dict_set(&unused_opts, e->key, NULL, 0);
-    }
-
-    e = NULL;
-    while ((e = av_dict_iterate(unused_opts, e))) {
-        const AVOption *option = av_opt_find(&class, e->key, NULL, 0,
-                                             AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
-        const AVOption *foption = av_opt_find(&fclass, e->key, NULL, 0,
-                                              AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ);
-        if (!option || foption)
-            continue;
-
-        if (!(option->flags & AV_OPT_FLAG_ENCODING_PARAM)) {
-            av_log(mux, AV_LOG_ERROR, "Codec AVOption %s (%s) is not an "
-                   "encoding option.\n", e->key, option->help ? option->help : "");
-            av_dict_free(&unused_opts);
-            return AVERROR(EINVAL);
-        }
-
-        // gop_timecode is injected by generic code but not always used
-        if (!strcmp(e->key, "gop_timecode"))
-            continue;
-
-        av_log(mux, AV_LOG_WARNING, "Codec AVOption %s (%s) has not been used "
-               "for any stream. The most likely reason is either wrong type "
-               "(e.g. a video option with no video streams) or that it is a "
-               "private option of some encoder which was not actually used for "
-               "any stream.\n", e->key, option->help ? option->help : "");
-    }
-    av_dict_free(&unused_opts);
-
-    return 0;
-}
-
 static const char *output_file_item_name(void *obj)
 {
     const Muxer *mux = obj;
@@ -3272,7 +3227,8 @@ int of_open(const OptionsContext *o, const char *filename, Scheduler *sch)
         return err;
 
     /* check if all codec options have been used */
-    err = validate_enc_avopt(mux, o->g->codec_opts);
+    err = check_avoptions_used(o->g->codec_opts, mux->enc_opts_used, mux, 0);
+    av_dict_free(&mux->enc_opts_used);
     if (err < 0)
         return err;
 
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 910e4a336b..b256fc9372 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -151,24 +151,6 @@ static int show_hwaccels(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
-/* return a copy of the input with the stream specifiers removed from the keys */
-AVDictionary *strip_specifiers(const AVDictionary *dict)
-{
-    const AVDictionaryEntry *e = NULL;
-    AVDictionary    *ret = NULL;
-
-    while ((e = av_dict_iterate(dict, e))) {
-        char *p = strchr(e->key, ':');
-
-        if (p)
-            *p = 0;
-        av_dict_set(&ret, e->key, e->value, 0);
-        if (p)
-            *p = ':';
-    }
-    return ret;
-}
-
 const char *opt_match_per_type_str(const SpecifierOptList *sol,
                                    char mediatype)
 {
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 1d0511b254..30719e9417 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -2673,7 +2673,7 @@ static int stream_component_open(VideoState *is, int stream_index)
         avctx->flags2 |= AV_CODEC_FLAG2_FAST;
 
     ret = filter_codec_opts(codec_opts, avctx->codec_id, ic,
-                            ic->streams[stream_index], codec, &opts);
+                            ic->streams[stream_index], codec, &opts, NULL);
     if (ret < 0)
         goto fail;
 
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index d7ba980ff9..e7e27d31ba 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3927,7 +3927,7 @@ static int open_input_file(InputFile *ifile, const char *filename,
             AVDictionary *opts;
 
             err = filter_codec_opts(codec_opts, stream->codecpar->codec_id,
-                                    fmt_ctx, stream, codec, &opts);
+                                    fmt_ctx, stream, codec, &opts, NULL);
             if (err < 0)
                 exit(1);
 
-- 
2.43.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-02  9:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-27  8:44 [FFmpeg-devel] [PATCH 1/3] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
2024-06-27  8:44 ` [FFmpeg-devel] [PATCH 2/3] fftools/ffmpeg_mux_init: apply encoder options manually Anton Khirnov
2024-06-27  8:44 ` [FFmpeg-devel] [PATCH 3/3] fftools/ffmpeg_mux_init: make encoder_opts local to ost_add() Anton Khirnov
2024-06-27 18:36 ` [FFmpeg-devel] [PATCH 1/3] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Marton Balint
2024-07-02  9:14   ` Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git