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/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame
@ 2024-05-23  9:03 Anton Khirnov
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Anton Khirnov @ 2024-05-23  9:03 UTC (permalink / raw)
  To: ffmpeg-devel

It conflicts with the AVCodecContext option of the same name.
---
 libavcodec/qsvenc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
index e3eb083746..d69dd19049 100644
--- a/libavcodec/qsvenc.h
+++ b/libavcodec/qsvenc.h
@@ -139,7 +139,7 @@
 { "avbr_convergence", "Convergence of the AVBR ratecontrol (unit of 100 frames)", OFFSET(qsv.avbr_convergence), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, UINT16_MAX, VE },
 
 #define QSV_OPTION_SKIP_FRAME \
-{ "skip_frame",     "Allow frame skipping", OFFSET(qsv.skip_frame),  AV_OPT_TYPE_INT, { .i64 = MFX_SKIPFRAME_NO_SKIP }, \
+{ "qsv_skip_frame",     "Allow frame skipping", OFFSET(qsv.skip_frame),  AV_OPT_TYPE_INT, { .i64 = MFX_SKIPFRAME_NO_SKIP }, \
    MFX_SKIPFRAME_NO_SKIP, MFX_SKIPFRAME_BRC_ONLY, VE, .unit = "skip_frame" }, \
 { "no_skip",        "Frame skipping is disabled", \
     0, AV_OPT_TYPE_CONST, { .i64 = MFX_SKIPFRAME_NO_SKIP },           .flags = VE, .unit = "skip_frame" },        \
-- 
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] 7+ messages in thread

* [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used
  2024-05-23  9:03 [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Anton Khirnov
@ 2024-05-23  9:03 ` Anton Khirnov
  2024-05-23 18:03   ` Marton Balint
  2024-05-24 19:41   ` Michael Niedermayer
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_mux_init: apply encoder options manually Anton Khirnov
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 7+ messages in thread
From: Anton Khirnov @ 2024-05-23  9:03 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          | 49 ++++++++++++++++++++++++++++++++++
 fftools/ffmpeg.h          |  3 ++-
 fftools/ffmpeg_demux.c    | 50 ++++++++---------------------------
 fftools/ffmpeg_mux.c      |  1 +
 fftools/ffmpeg_mux.h      |  3 +++
 fftools/ffmpeg_mux_init.c | 55 +++++----------------------------------
 fftools/ffmpeg_opt.c      | 18 -------------
 fftools/ffplay.c          |  2 +-
 fftools/ffprobe.c         |  2 +-
 11 files changed, 89 insertions(+), 115 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index a8f5c6d89b..265ce5c04c 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 d0c773663b..5966501d84 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 c86fd5065e..5e27f073aa 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -493,6 +493,55 @@ int check_avoptions(AVDictionary *m)
     return 0;
 }
 
+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;
+
+        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);
+        }
+
+        if (!av_dict_get(opts_used, e->key, NULL, 0)) {
+            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 885a7c0c10..9cab8148ca 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -712,9 +712,10 @@ void show_usage(void);
 
 void remove_avoptions(AVDictionary **a, AVDictionary *b);
 int check_avoptions(AVDictionary *m);
+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 cba63dab5f..52e427ea49 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;
     }
@@ -1532,8 +1532,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;
@@ -1805,48 +1804,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 8797265145..41afe8259d 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;
     }
@@ -3114,52 +3116,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 : "");
-            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;
@@ -3267,7 +3223,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 5a66bfa38d..14999349ac 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -2674,7 +2674,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 2d38e5dfdc..ce1b0fb6ba 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3922,7 +3922,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] 7+ messages in thread

* [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_mux_init: apply encoder options manually
  2024-05-23  9:03 [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Anton Khirnov
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
@ 2024-05-23  9:03 ` Anton Khirnov
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_mux_init: make encoder_opts local to ost_add() Anton Khirnov
  2024-05-24  2:36 ` [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Xiang, Haihao
  3 siblings, 0 replies; 7+ messages in thread
From: Anton Khirnov @ 2024-05-23  9:03 UTC (permalink / raw)
  To: ffmpeg-devel

Do not pass an options dictionary to the avcodec_open2() in enc_open().

This is cleaner and more robust, as previously various bits of code
would try to interpret the contents of the options dictionary, with
varying degrees of correctness. Now they can just access the encoder
AVCodecContext directly.

Cf. 372c78dd42f2b1ca743473b9c32fad71c65919e0 - analogous change for
decoding.

A non-progressive field order is now written on the container level in
interlaced ProRes encoding tests.
---
 fftools/ffmpeg_enc.c                        | 18 ++----
 fftools/ffmpeg_mux_init.c                   | 66 +++++++++++----------
 tests/ref/vsynth/vsynth1-prores_444_int     |  2 +-
 tests/ref/vsynth/vsynth1-prores_int         |  2 +-
 tests/ref/vsynth/vsynth2-prores_444_int     |  2 +-
 tests/ref/vsynth/vsynth2-prores_int         |  2 +-
 tests/ref/vsynth/vsynth3-prores_444_int     |  2 +-
 tests/ref/vsynth/vsynth3-prores_int         |  2 +-
 tests/ref/vsynth/vsynth_lena-prores_444_int |  2 +-
 tests/ref/vsynth/vsynth_lena-prores_int     |  2 +-
 10 files changed, 47 insertions(+), 53 deletions(-)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 618ba193ff..029980063d 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -307,16 +307,10 @@ int enc_open(void *opaque, const AVFrame *frame)
     if (ost->bitexact)
         enc_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
 
-    if (!av_dict_get(ost->encoder_opts, "threads", NULL, 0))
-        av_dict_set(&ost->encoder_opts, "threads", "auto", 0);
+    if (enc->capabilities & AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE)
+        enc_ctx->flags |= AV_CODEC_FLAG_COPY_OPAQUE;
 
-    if (enc->capabilities & AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE) {
-        ret = av_dict_set(&ost->encoder_opts, "flags", "+copy_opaque", AV_DICT_MULTIKEY);
-        if (ret < 0)
-            return ret;
-    }
-
-    av_dict_set(&ost->encoder_opts, "flags", "+frame_duration", AV_DICT_MULTIKEY);
+    enc_ctx->flags |= AV_CODEC_FLAG_FRAME_DURATION;
 
     ret = hw_device_setup_for_encode(ost, frame ? frame->hw_frames_ctx : NULL);
     if (ret < 0) {
@@ -325,7 +319,7 @@ int enc_open(void *opaque, const AVFrame *frame)
         return ret;
     }
 
-    if ((ret = avcodec_open2(ost->enc_ctx, enc, &ost->encoder_opts)) < 0) {
+    if ((ret = avcodec_open2(ost->enc_ctx, enc, NULL)) < 0) {
         if (ret != AVERROR_EXPERIMENTAL)
             av_log(ost, AV_LOG_ERROR, "Error while opening encoder - maybe "
                    "incorrect parameters such as bit_rate, rate, width or height.\n");
@@ -337,10 +331,6 @@ int enc_open(void *opaque, const AVFrame *frame)
     if (ost->enc_ctx->frame_size)
         frame_samples = ost->enc_ctx->frame_size;
 
-    ret = check_avoptions(ost->encoder_opts);
-    if (ret < 0)
-        return ret;
-
     if (ost->enc_ctx->bit_rate && ost->enc_ctx->bit_rate < 1000 &&
         ost->enc_ctx->codec_id != AV_CODEC_ID_CODEC2 /* don't complain about 700 bit/s modes */)
         av_log(ost, AV_LOG_WARNING, "The bitrate parameter is set too low."
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 41afe8259d..61a0d8658f 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -711,14 +711,10 @@ static int new_stream_video(Muxer *mux, const OptionsContext *o,
         /* two pass mode */
         MATCH_PER_STREAM_OPT(pass, i, do_pass, oc, st);
         if (do_pass) {
-            if (do_pass & 1) {
+            if (do_pass & 1)
                 video_enc->flags |= AV_CODEC_FLAG_PASS1;
-                av_dict_set(&ost->encoder_opts, "flags", "+pass1", AV_DICT_APPEND);
-            }
-            if (do_pass & 2) {
+            if (do_pass & 2)
                 video_enc->flags |= AV_CODEC_FLAG_PASS2;
-                av_dict_set(&ost->encoder_opts, "flags", "+pass2", AV_DICT_APPEND);
-            }
         }
 
         MATCH_PER_STREAM_OPT(passlogfiles, str, ost->logfile_prefix, oc, st);
@@ -740,7 +736,10 @@ static int new_stream_video(Muxer *mux, const OptionsContext *o,
                                            DEFAULT_PASS_LOGFILENAME_PREFIX,
                      ost_idx);
             if (!strcmp(ost->enc_ctx->codec->name, "libx264")) {
-                av_dict_set(&ost->encoder_opts, "stats", logfilename, AV_DICT_DONT_OVERWRITE);
+                if (av_opt_is_set_to_default_by_name(ost->enc_ctx, "stats",
+                                                     AV_OPT_SEARCH_CHILDREN) > 0)
+                    av_opt_set(ost->enc_ctx, "stats", logfilename,
+                               AV_OPT_SEARCH_CHILDREN);
             } else {
                 if (video_enc->flags & AV_CODEC_FLAG_PASS2) {
                     char  *logbuffer = file_read(logfilename);
@@ -1041,6 +1040,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     const AVCodec *enc;
     AVStream *st;
     int ret = 0, keep_pix_fmt = 0, autoscale = 1;
+    int threads_manual = 0;
     AVRational enc_tb = { 0, 0 };
     enum VideoSyncMethod vsync_method = VSYNC_AUTO;
     const char *bsfs = NULL, *time_base = NULL;
@@ -1264,6 +1264,23 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
             enc_tb = q;
         }
+
+        threads_manual = !!av_dict_get(ost->encoder_opts, "threads", NULL, 0);
+
+        ret = av_opt_set_dict2(ost->enc_ctx, &ost->encoder_opts, AV_OPT_SEARCH_CHILDREN);
+        if (ret < 0) {
+            av_log(ost, AV_LOG_ERROR, "Error applying encoder options: %s\n",
+                   av_err2str(ret));
+            return ret;
+        }
+
+        ret = check_avoptions(ost->encoder_opts);
+        if (ret < 0)
+            return ret;
+
+        // default to automatic thread count
+        if (!threads_manual)
+            ost->enc_ctx->thread_count = 0;
     } else {
         ret = filter_codec_opts(o->g->codec_opts, AV_CODEC_ID_NONE, oc, st,
                                 NULL, &ost->encoder_opts,
@@ -1276,8 +1293,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     if (o->bitexact) {
         ost->bitexact        = 1;
     } else if (ost->enc_ctx) {
-        ost->bitexact        = check_opt_bitexact(ost->enc_ctx, ost->encoder_opts, "flags",
-                                                  AV_CODEC_FLAG_BITEXACT);
+        ost->bitexact        = !!(ost->enc_ctx->flags & AV_CODEC_FLAG_BITEXACT);
     }
 
     MATCH_PER_STREAM_OPT(time_bases, str, time_base, oc, st);
@@ -1372,7 +1388,6 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
     if (ost->enc &&
         (type == AVMEDIA_TYPE_VIDEO || type == AVMEDIA_TYPE_AUDIO)) {
-        const AVDictionaryEntry *e;
         char name[16];
         OutputFilterOptions opts = {
             .enc = enc,
@@ -1398,10 +1413,6 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
         snprintf(name, sizeof(name), "#%d:%d", mux->of.index, ost->index);
 
-        e = av_dict_get(ost->encoder_opts, "threads", NULL, 0);
-        if (e)
-            opts.nb_threads = e->value;
-
         // MJPEG encoder exports a full list of supported pixel formats,
         // but the full-range ones are experimental-only.
         // Restrict the auto-conversion list unless -strict experimental
@@ -1413,33 +1424,26 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
                 { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
                   AV_PIX_FMT_NONE };
 
-            const AVDictionaryEntry *strict = av_dict_get(ost->encoder_opts, "strict", NULL, 0);
-            int strict_val = ost->enc_ctx->strict_std_compliance;
-
-            if (strict) {
-                const AVOption *o = av_opt_find(ost->enc_ctx, strict->key, NULL, 0, 0);
-                av_assert0(o);
-                av_opt_eval_int(ost->enc_ctx, o, strict->value, &strict_val);
-            }
-
-            if (strict_val > FF_COMPLIANCE_UNOFFICIAL)
+            if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL)
                 opts.pix_fmts = mjpeg_formats;
         }
 
+        if (threads_manual) {
+            ret = av_opt_get(ost->enc_ctx, "threads", 0, (uint8_t**)&opts.nb_threads);
+            if (ret < 0)
+                return ret;
+        }
+
         if (ofilter) {
             ost->filter       = ofilter;
             ret = ofilter_bind_ost(ofilter, ost, ms->sch_idx_enc, &opts);
-            if (ret < 0)
-                return ret;
         } else {
             ret = init_simple_filtergraph(ost->ist, ost, filters,
                                           mux->sch, ms->sch_idx_enc, &opts);
-            if (ret < 0) {
-                av_log(ost, AV_LOG_ERROR,
-                       "Error initializing a simple filtergraph\n");
-                return ret;
-            }
         }
+        av_freep(&opts.nb_threads);
+        if (ret < 0)
+            return ret;
 
         ret = sch_connect(mux->sch, SCH_ENC(ms->sch_idx_enc),
                                     SCH_MSTREAM(mux->sch_idx, ms->sch_idx));
diff --git a/tests/ref/vsynth/vsynth1-prores_444_int b/tests/ref/vsynth/vsynth1-prores_444_int
index 76db62d4e9..3d4c616c7a 100644
--- a/tests/ref/vsynth/vsynth1-prores_444_int
+++ b/tests/ref/vsynth/vsynth1-prores_444_int
@@ -1,4 +1,4 @@
-fd2a2f49c61817c2338f39d5736d5fd2 *tests/data/fate/vsynth1-prores_444_int.mov
+bd1502671ce7144c106a6a460b2af404 *tests/data/fate/vsynth1-prores_444_int.mov
 9940947 tests/data/fate/vsynth1-prores_444_int.mov
 732ceeb6887524e0aee98762fe50578b *tests/data/fate/vsynth1-prores_444_int.out.rawvideo
 stddev:    2.83 PSNR: 39.08 MAXDIFF:   45 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth1-prores_int b/tests/ref/vsynth/vsynth1-prores_int
index 3e2bbeff2a..c3b57631c8 100644
--- a/tests/ref/vsynth/vsynth1-prores_int
+++ b/tests/ref/vsynth/vsynth1-prores_int
@@ -1,4 +1,4 @@
-1f1b246dfabe028f04c78887e5da51ed *tests/data/fate/vsynth1-prores_int.mov
+842f92426e56cf6208cc94360d29fc69 *tests/data/fate/vsynth1-prores_int.mov
 6308688 tests/data/fate/vsynth1-prores_int.mov
 164a4ca890695cf594293d1acec9463c *tests/data/fate/vsynth1-prores_int.out.rawvideo
 stddev:    2.66 PSNR: 39.62 MAXDIFF:   34 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-prores_444_int b/tests/ref/vsynth/vsynth2-prores_444_int
index a2ee569c49..8d2a13b91b 100644
--- a/tests/ref/vsynth/vsynth2-prores_444_int
+++ b/tests/ref/vsynth/vsynth2-prores_444_int
@@ -1,4 +1,4 @@
-5ac517fc2380a6cf11b7d86d2fafee0a *tests/data/fate/vsynth2-prores_444_int.mov
+6f5fa77609698fbed3a7eef1c91bb9ea *tests/data/fate/vsynth2-prores_444_int.mov
 6420787 tests/data/fate/vsynth2-prores_444_int.mov
 33a5db4f0423168d4ae4f1db3610928e *tests/data/fate/vsynth2-prores_444_int.out.rawvideo
 stddev:    0.93 PSNR: 48.73 MAXDIFF:   14 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-prores_int b/tests/ref/vsynth/vsynth2-prores_int
index 72139ee6c3..f80ff34601 100644
--- a/tests/ref/vsynth/vsynth2-prores_int
+++ b/tests/ref/vsynth/vsynth2-prores_int
@@ -1,4 +1,4 @@
-4062c74196d95a64e642bd917377ed93 *tests/data/fate/vsynth2-prores_int.mov
+a9a9812dd8944a58c2c2b3b0ce41247d *tests/data/fate/vsynth2-prores_int.mov
 4070996 tests/data/fate/vsynth2-prores_int.mov
 bef9e38387a1fbb1ce2e4401b6d41674 *tests/data/fate/vsynth2-prores_int.out.rawvideo
 stddev:    1.54 PSNR: 44.37 MAXDIFF:   13 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth3-prores_444_int b/tests/ref/vsynth/vsynth3-prores_444_int
index ac30691143..ca30db5383 100644
--- a/tests/ref/vsynth/vsynth3-prores_444_int
+++ b/tests/ref/vsynth/vsynth3-prores_444_int
@@ -1,4 +1,4 @@
-50db4bbc4674de3dfdd41f306af1cb17 *tests/data/fate/vsynth3-prores_444_int.mov
+b61864cafb35ad8e592b4f899bdd84b6 *tests/data/fate/vsynth3-prores_444_int.mov
 184397 tests/data/fate/vsynth3-prores_444_int.mov
 a8852aa2841c2ce5f2aa86176ceda4ef *tests/data/fate/vsynth3-prores_444_int.out.rawvideo
 stddev:    3.24 PSNR: 37.91 MAXDIFF:   41 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth3-prores_int b/tests/ref/vsynth/vsynth3-prores_int
index 86fc2266b5..0feee26de1 100644
--- a/tests/ref/vsynth/vsynth3-prores_int
+++ b/tests/ref/vsynth/vsynth3-prores_int
@@ -1,4 +1,4 @@
-24b765064b4aec754fdd0cc3658bba19 *tests/data/fate/vsynth3-prores_int.mov
+2e89dfb5e2b5146337c1d65ccc4fe196 *tests/data/fate/vsynth3-prores_int.mov
 120484 tests/data/fate/vsynth3-prores_int.mov
 e5859ba47a99f9e53c1ddcaa68a8f8f8 *tests/data/fate/vsynth3-prores_int.out.rawvideo
 stddev:    2.92 PSNR: 38.81 MAXDIFF:   29 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth_lena-prores_444_int b/tests/ref/vsynth/vsynth_lena-prores_444_int
index f05f1fe775..b13a7a126a 100644
--- a/tests/ref/vsynth/vsynth_lena-prores_444_int
+++ b/tests/ref/vsynth/vsynth_lena-prores_444_int
@@ -1,4 +1,4 @@
-09b5dffd1a484e2152a3b5a0bcceed32 *tests/data/fate/vsynth_lena-prores_444_int.mov
+e76d98c772c65c7ac75f7d43484883d3 *tests/data/fate/vsynth_lena-prores_444_int.mov
 5696258 tests/data/fate/vsynth_lena-prores_444_int.mov
 466380156e4d2b811f4ffb9c5a8bca72 *tests/data/fate/vsynth_lena-prores_444_int.out.rawvideo
 stddev:    0.88 PSNR: 49.23 MAXDIFF:    9 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth_lena-prores_int b/tests/ref/vsynth/vsynth_lena-prores_int
index ea08089745..cec45a59b9 100644
--- a/tests/ref/vsynth/vsynth_lena-prores_int
+++ b/tests/ref/vsynth/vsynth_lena-prores_int
@@ -1,4 +1,4 @@
-c7e9a61054f44fe372a3bce619b68ce9 *tests/data/fate/vsynth_lena-prores_int.mov
+21d328216b194500a3c87c45afa5e927 *tests/data/fate/vsynth_lena-prores_int.mov
 3532698 tests/data/fate/vsynth_lena-prores_int.mov
 eb5caa9824ca294f403cd13f33c40f23 *tests/data/fate/vsynth_lena-prores_int.out.rawvideo
 stddev:    1.47 PSNR: 44.78 MAXDIFF:   12 bytes:  7603200/  7603200
-- 
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] 7+ messages in thread

* [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_mux_init: make encoder_opts local to ost_add()
  2024-05-23  9:03 [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Anton Khirnov
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_mux_init: apply encoder options manually Anton Khirnov
@ 2024-05-23  9:03 ` Anton Khirnov
  2024-05-24  2:36 ` [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Xiang, Haihao
  3 siblings, 0 replies; 7+ messages in thread
From: Anton Khirnov @ 2024-05-23  9:03 UTC (permalink / raw)
  To: ffmpeg-devel

It is no longer needed after this function returns.
---
 fftools/ffmpeg.h          |  2 --
 fftools/ffmpeg_mux.c      |  1 -
 fftools/ffmpeg_mux_init.c | 71 ++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 9cab8148ca..51aee0679a 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -586,8 +586,6 @@ typedef struct OutputStream {
     FilterGraph  *fg_simple;
     OutputFilter *filter;
 
-    AVDictionary *encoder_opts;
-
     char *attachment_filename;
 
     /* stats */
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 055e2f3678..de3e052152 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -813,7 +813,6 @@ static void ost_free(OutputStream **post)
     av_packet_free(&ms->bsf_pkt);
 
     av_packet_free(&ms->pkt);
-    av_dict_free(&ost->encoder_opts);
 
     av_freep(&ost->kf.pts);
     av_expr_free(ost->kf.pexpr);
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 61a0d8658f..b8dc6017a9 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -905,7 +905,7 @@ static int new_stream_subtitle(Muxer *mux, const OptionsContext *o,
     return 0;
 }
 
-static int streamcopy_init(const Muxer *mux, OutputStream *ost)
+static int streamcopy_init(const Muxer *mux, OutputStream *ost, AVDictionary **encoder_opts)
 {
     MuxStream           *ms         = ms_from_ost(ost);
 
@@ -928,7 +928,7 @@ static int streamcopy_init(const Muxer *mux, OutputStream *ost)
 
     ret = avcodec_parameters_to_context(codec_ctx, ist->par);
     if (ret >= 0)
-        ret = av_opt_set_dict(codec_ctx, &ost->encoder_opts);
+        ret = av_opt_set_dict(codec_ctx, encoder_opts);
     if (ret < 0) {
         av_log(ost, AV_LOG_FATAL,
                "Error setting up codec context options.\n");
@@ -1039,6 +1039,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     OutputStream *ost;
     const AVCodec *enc;
     AVStream *st;
+    AVDictionary *encoder_opts = NULL;
     int ret = 0, keep_pix_fmt = 0, autoscale = 1;
     int threads_manual = 0;
     AVRational enc_tb = { 0, 0 };
@@ -1160,10 +1161,10 @@ 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, &encoder_opts,
                                 &mux->enc_opts_used);
         if (ret < 0)
-            return ret;
+            goto fail;
 
         MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
 
@@ -1187,7 +1188,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
                     break;
                 }
                 *arg++ = 0;
-                av_dict_set(&ost->encoder_opts, buf, arg, AV_DICT_DONT_OVERWRITE);
+                av_dict_set(&encoder_opts, buf, arg, AV_DICT_DONT_OVERWRITE);
             } while (!s->eof_reached);
             av_bprint_finalize(&bprint, NULL);
             avio_closep(&s);
@@ -1195,7 +1196,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         if (ret) {
             av_log(ost, AV_LOG_FATAL,
                    "Preset %s specified, but could not be opened.\n", preset);
-            return ret;
+            goto fail;
         }
 
         MATCH_PER_STREAM_OPT(enc_stats_pre, str, enc_stats_pre, oc, st);
@@ -1207,7 +1208,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
             ret = enc_stats_init(ost, &ost->enc_stats_pre, 1, enc_stats_pre, format);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
 
         MATCH_PER_STREAM_OPT(enc_stats_post, str, enc_stats_post, oc, st);
@@ -1219,7 +1220,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
             ret = enc_stats_init(ost, &ost->enc_stats_post, 0, enc_stats_post, format);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
 
         MATCH_PER_STREAM_OPT(mux_stats, str, mux_stats, oc, st);
@@ -1231,7 +1232,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 
             ret = enc_stats_init(ost, &ms->stats, 0, mux_stats, format);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
 
         MATCH_PER_STREAM_OPT(enc_time_bases, str, enc_time_base, oc, st);
@@ -1253,7 +1254,8 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
 #endif
                     ) {
                     av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", enc_time_base);
-                    return ret < 0 ? ret : AVERROR(EINVAL);
+                    ret = ret < 0 ? ret : AVERROR(EINVAL);
+                    goto fail;
                 }
 #if FFMPEG_OPT_ENC_TIME_BASE_NUM
                 if (q.num < 0)
@@ -1265,28 +1267,28 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
             enc_tb = q;
         }
 
-        threads_manual = !!av_dict_get(ost->encoder_opts, "threads", NULL, 0);
+        threads_manual = !!av_dict_get(encoder_opts, "threads", NULL, 0);
 
-        ret = av_opt_set_dict2(ost->enc_ctx, &ost->encoder_opts, AV_OPT_SEARCH_CHILDREN);
+        ret = av_opt_set_dict2(ost->enc_ctx, &encoder_opts, AV_OPT_SEARCH_CHILDREN);
         if (ret < 0) {
             av_log(ost, AV_LOG_ERROR, "Error applying encoder options: %s\n",
                    av_err2str(ret));
-            return ret;
+            goto fail;
         }
 
-        ret = check_avoptions(ost->encoder_opts);
+        ret = check_avoptions(encoder_opts);
         if (ret < 0)
-            return ret;
+            goto fail;
 
         // default to automatic thread count
         if (!threads_manual)
             ost->enc_ctx->thread_count = 0;
     } else {
         ret = filter_codec_opts(o->g->codec_opts, AV_CODEC_ID_NONE, oc, st,
-                                NULL, &ost->encoder_opts,
+                                NULL, &encoder_opts,
                                 &mux->enc_opts_used);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
 
 
@@ -1302,7 +1304,8 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         if (av_parse_ratio(&q, time_base, INT_MAX, 0, NULL) < 0 ||
             q.num <= 0 || q.den <= 0) {
             av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", time_base);
-            return AVERROR(EINVAL);
+            ret = AVERROR(EINVAL);
+            goto fail;
         }
         st->time_base = q;
     }
@@ -1325,7 +1328,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         ret = av_bsf_list_parse_str(bsfs, &ms->bsf_ctx);
         if (ret < 0) {
             av_log(ost, AV_LOG_ERROR, "Error parsing bitstream filter sequence '%s': %s\n", bsfs, av_err2str(ret));
-            return ret;
+            goto fail;
         }
     }
 
@@ -1378,12 +1381,12 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     case AVMEDIA_TYPE_SUBTITLE:   ret = new_stream_subtitle  (mux, o, ost); break;
     }
     if (ret < 0)
-        return ret;
+        goto fail;
 
     if (type == AVMEDIA_TYPE_VIDEO || type == AVMEDIA_TYPE_AUDIO) {
         ret = ost_get_filters(o, oc, ost, &filters);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
 
     if (ost->enc &&
@@ -1431,7 +1434,7 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         if (threads_manual) {
             ret = av_opt_get(ost->enc_ctx, "threads", 0, (uint8_t**)&opts.nb_threads);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
 
         if (ofilter) {
@@ -1443,18 +1446,19 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
         }
         av_freep(&opts.nb_threads);
         if (ret < 0)
-            return ret;
+            goto fail;
 
         ret = sch_connect(mux->sch, SCH_ENC(ms->sch_idx_enc),
                                     SCH_MSTREAM(mux->sch_idx, ms->sch_idx));
         if (ret < 0)
-            return ret;
+            goto fail;
     } else if (ost->ist) {
         int sched_idx = ist_output_add(ost->ist, ost);
         if (sched_idx < 0) {
             av_log(ost, AV_LOG_ERROR,
                    "Error binding an input stream\n");
-            return sched_idx;
+            ret = sched_idx;
+            goto fail;
         }
         ms->sch_idx_src = sched_idx;
 
@@ -1462,24 +1466,24 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
             ret = sch_connect(mux->sch, SCH_DEC(sched_idx),
                                         SCH_ENC(ms->sch_idx_enc));
             if (ret < 0)
-                return ret;
+                goto fail;
 
             ret = sch_connect(mux->sch, SCH_ENC(ms->sch_idx_enc),
                                         SCH_MSTREAM(mux->sch_idx, ms->sch_idx));
             if (ret < 0)
-                return ret;
+                goto fail;
         } else {
             ret = sch_connect(mux->sch, SCH_DSTREAM(ost->ist->file->index, sched_idx),
                                         SCH_MSTREAM(ost->file->index, ms->sch_idx));
             if (ret < 0)
-                return ret;
+                goto fail;
         }
     }
 
     if (ost->ist && !ost->enc) {
-        ret = streamcopy_init(mux, ost);
+        ret = streamcopy_init(mux, ost, &encoder_opts);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
 
     // copy estimated duration as a hint to the muxer
@@ -1491,7 +1495,12 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     if (post)
         *post = ost;
 
-    return 0;
+    ret = 0;
+
+fail:
+    av_dict_free(&encoder_opts);
+
+    return ret;
 }
 
 static int map_auto_video(Muxer *mux, const OptionsContext *o)
-- 
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] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
@ 2024-05-23 18:03   ` Marton Balint
  2024-05-24 19:41   ` Michael Niedermayer
  1 sibling, 0 replies; 7+ messages in thread
From: Marton Balint @ 2024-05-23 18:03 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Thu, 23 May 2024, Anton Khirnov wrote:

> 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          | 49 ++++++++++++++++++++++++++++++++++
> fftools/ffmpeg.h          |  3 ++-
> fftools/ffmpeg_demux.c    | 50 ++++++++---------------------------
> fftools/ffmpeg_mux.c      |  1 +
> fftools/ffmpeg_mux.h      |  3 +++
> fftools/ffmpeg_mux_init.c | 55 +++++----------------------------------
> fftools/ffmpeg_opt.c      | 18 -------------
> fftools/ffplay.c          |  2 +-
> fftools/ffprobe.c         |  2 +-
> 11 files changed, 89 insertions(+), 115 deletions(-)
>

[...]

> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -493,6 +493,55 @@ int check_avoptions(AVDictionary *m)
>     return 0;
> }
>
> +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;
> +
> +        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 : "",

Why the change of e->key to option->name? The full user specified option 
should be printed with specifier, so the user will know exactly which 
specifier matched the wrong stream type.

> +                   decode ? "decoding" : "encoding");
> +            return AVERROR(EINVAL);
> +        }
> +
> +        if (!av_dict_get(opts_used, e->key, NULL, 0)) {
> +            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 : "");

Same here. The non-matching specifer should also be printed, not only the 
option name, so please keep using e->key in the warning message.

Thanks,
Marton
_______________________________________________
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] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame
  2024-05-23  9:03 [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Anton Khirnov
                   ` (2 preceding siblings ...)
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_mux_init: make encoder_opts local to ost_add() Anton Khirnov
@ 2024-05-24  2:36 ` Xiang, Haihao
  3 siblings, 0 replies; 7+ messages in thread
From: Xiang, Haihao @ 2024-05-24  2:36 UTC (permalink / raw)
  To: ffmpeg-devel

On Do, 2024-05-23 at 11:03 +0200, Anton Khirnov wrote:
> It conflicts with the AVCodecContext option of the same name.
> ---

The skip_frame option in AVCodecContext is for decoding only, may we not use
this option for encoding ? 


>  libavcodec/qsvenc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index e3eb083746..d69dd19049 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -139,7 +139,7 @@
>  { "avbr_convergence", "Convergence of the AVBR ratecontrol (unit of 100
> frames)", OFFSET(qsv.avbr_convergence), AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
> UINT16_MAX, VE },
>  
>  #define QSV_OPTION_SKIP_FRAME \
> -{ "skip_frame",     "Allow frame skipping", OFFSET(qsv.skip_frame), 
> AV_OPT_TYPE_INT, { .i64 = MFX_SKIPFRAME_NO_SKIP }, \
> +{ "qsv_skip_frame",     "Allow frame skipping", OFFSET(qsv.skip_frame), 
> AV_OPT_TYPE_INT, { .i64 = MFX_SKIPFRAME_NO_SKIP }, \
>     MFX_SKIPFRAME_NO_SKIP, MFX_SKIPFRAME_BRC_ONLY, VE, .unit = "skip_frame" },

How about changing the unit to qsv_skip_frame too ? 

Thanks
Haihao


> \
>  { "no_skip",        "Frame skipping is disabled", \
>      0, AV_OPT_TYPE_CONST, { .i64 = MFX_SKIPFRAME_NO_SKIP },           .flags
> = VE, .unit = "skip_frame" },        \

_______________________________________________
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] 7+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used
  2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
  2024-05-23 18:03   ` Marton Balint
@ 2024-05-24 19:41   ` Michael Niedermayer
  1 sibling, 0 replies; 7+ messages in thread
From: Michael Niedermayer @ 2024-05-24 19:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1592 bytes --]

On Thu, May 23, 2024 at 11:03:37AM +0200, Anton Khirnov wrote:
> 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          | 49 ++++++++++++++++++++++++++++++++++
>  fftools/ffmpeg.h          |  3 ++-
>  fftools/ffmpeg_demux.c    | 50 ++++++++---------------------------
>  fftools/ffmpeg_mux.c      |  1 +
>  fftools/ffmpeg_mux.h      |  3 +++
>  fftools/ffmpeg_mux_init.c | 55 +++++----------------------------------
>  fftools/ffmpeg_opt.c      | 18 -------------
>  fftools/ffplay.c          |  2 +-
>  fftools/ffprobe.c         |  2 +-
>  11 files changed, 89 insertions(+), 115 deletions(-)

breaks https://samples.ffmpeg.org/image-samples/exr/sub.0030.exr

./ffmpeg -layer RenderLayer.Color -gamma 2.2  -i sub.0030.exr -bitexact  /tmp/test-color2.2.jpg

Input #0, exr_pipe, from 'sub.0030.exr':
  Duration: N/A, bitrate: N/A
  Stream #0:0: Video: exr, gbrapf32le, 960x540 [SAR 1:1 DAR 16:9], 25 fps, 25 tbr, 25 tbn
[in#0/exr_pipe @ 0x55c06b88b7c0] Codec AVOption gamma (set gamma) is not a decoding option.
Error opening input file sub.0030.exr.
Error opening input files: Invalid argument

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] 7+ messages in thread

end of thread, other threads:[~2024-05-24 19:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-23  9:03 [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Anton Khirnov
2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 2/4] fftools/ffmpeg: rewrite checking whether codec AVOptions have been used Anton Khirnov
2024-05-23 18:03   ` Marton Balint
2024-05-24 19:41   ` Michael Niedermayer
2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 3/4] fftools/ffmpeg_mux_init: apply encoder options manually Anton Khirnov
2024-05-23  9:03 ` [FFmpeg-devel] [PATCH 4/4] fftools/ffmpeg_mux_init: make encoder_opts local to ost_add() Anton Khirnov
2024-05-24  2:36 ` [FFmpeg-devel] [PATCH 1/4] lavc/qsvenc: rename the skip_frame private option to qsv_skip_frame Xiang, Haihao

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