diff mbox series

[FFmpeg-devel,14/22] fftools/ffmpeg_filter: stop accessing encoder from pixfmt selection

Message ID 20230707094847.25324-14-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/22] lavc/encode: print separate messages for unknown and unsupported formats | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov July 7, 2023, 9:48 a.m. UTC 
ffmpeg CLI pixel format selection for filtering currently special-cases
MJPEG encoding, where it will restrict the supported list of pixel
formats depending on the value of the -strict option. In order to get
that value it will apply it from the options dict into the encoder
context, which is a highly invasive action even now, and would become a
race once encoding is moved to its own thread.

The ugliness of this code can be much reduced by moving the special
handling of MJPEG into ofilter_bind_ost(), which is called from encoder
init and is thus synchronized with it. There is also no need to write
anything to the encoder context, we can evaluate the option into our
stack variable.

There is also no need to access AVCodec at all during pixel format
selection, as the pixel formats array is already stored in
OutputFilterPriv.
---
 fftools/ffmpeg_filter.c | 64 ++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 33 deletions(-)
diff mbox series

Patch

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index c283ee2b7a..67a5f48245 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -262,21 +262,6 @@  static void sub2video_update(InputFilterPriv *ifp, int64_t heartbeat_pts,
     ifp->sub2video.initialize = 0;
 }
 
-// FIXME: YUV420P etc. are actually supported with full color range,
-// yet the latter information isn't available here.
-static const enum AVPixelFormat *get_compliance_normal_pix_fmts(const AVCodec *codec, const enum AVPixelFormat default_formats[])
-{
-    static const enum AVPixelFormat mjpeg_formats[] =
-        { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
-          AV_PIX_FMT_NONE };
-
-    if (!strcmp(codec->name, "mjpeg")) {
-        return mjpeg_formats;
-    } else {
-        return default_formats;
-    }
-}
-
 /* May return NULL (no pixel format found), a static string or a string
  * backed by the bprint. Nothing has been written to the AVBPrint in case
  * NULL is returned. The AVBPrint provided should be clean. */
@@ -284,26 +269,15 @@  static const char *choose_pix_fmts(OutputFilter *ofilter, AVBPrint *bprint)
 {
     OutputFilterPriv *ofp = ofp_from_ofilter(ofilter);
     OutputStream *ost = ofilter->ost;
-    AVCodecContext *enc = ost->enc_ctx;
-    const AVDictionaryEntry *strict_dict = av_dict_get(ost->encoder_opts, "strict", NULL, 0);
-    if (strict_dict)
-        // used by choose_pixel_fmt() and below
-        av_opt_set(ost->enc_ctx, "strict", strict_dict->value, 0);
 
-     if (ost->keep_pix_fmt) {
-        if (ost->enc_ctx->pix_fmt == AV_PIX_FMT_NONE)
-            return NULL;
-        return av_get_pix_fmt_name(ost->enc_ctx->pix_fmt);
+    if (ost->keep_pix_fmt) {
+        return ofp->format == AV_PIX_FMT_NONE ? NULL :
+               av_get_pix_fmt_name(ofp->format);
     }
-    if (ost->enc_ctx->pix_fmt != AV_PIX_FMT_NONE) {
-        return av_get_pix_fmt_name(enc->pix_fmt);
-    } else if (enc->codec->pix_fmts) {
-        const enum AVPixelFormat *p;
-
-        p = enc->codec->pix_fmts;
-        if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
-            p = get_compliance_normal_pix_fmts(enc->codec, p);
-        }
+    if (ofp->format != AV_PIX_FMT_NONE) {
+        return av_get_pix_fmt_name(ofp->format);
+    } else if (ofp->formats) {
+        const enum AVPixelFormat *p = ofp->formats;
 
         for (; *p != AV_PIX_FMT_NONE; p++) {
             const char *name = av_get_pix_fmt_name(*p);
@@ -662,6 +636,30 @@  void ofilter_bind_ost(OutputFilter *ofilter, OutputStream *ost)
             ofp->format = ost->enc_ctx->pix_fmt;
         } else {
             ofp->formats = c->pix_fmts;
+
+            // 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
+            // has been specified.
+            if (!strcmp(c->name, "mjpeg")) {
+                // FIXME: YUV420P etc. are actually supported with full color range,
+                // yet the latter information isn't available here.
+                static const enum AVPixelFormat mjpeg_formats[] =
+                    { 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)
+                    ofp->formats = mjpeg_formats;
+            }
         }
 
         fgp->disable_conversions |= ost->keep_pix_fmt;