diff mbox series

[FFmpeg-devel] libavcodec/qsvenc.c fixed handling of closed gop flag

Message ID 20200424094554.6359123b@debbieb1
State New
Headers show
Series [FFmpeg-devel] libavcodec/qsvenc.c fixed handling of closed gop flag | expand

Checks

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

Commit Message

Elio Blanca April 24, 2020, 7:45 a.m. UTC 
Hello,
this is my very first message to this list, so first of all: thank you for your huge work!

Now, to the issue: I made this patch for libavcodec/qsvenc.c in order to honor the GopOptFlag flag for hevc_qsv encodings (maybe others will be affected though).
In statement 513 (the third fix) a wrong value was assigned in case of not-closed-gop. Zero is not an allowed value, as the enum mfx.GopOptFlag refers to uses only 1 and 2. What the encoding engine will do when feeded with a non-valid value is unpredictable.
Then, in statements 148 and 150 I changed those 'if' replacing '&' with '==' because of the enum, which is different from a bitmask.

Apart from those, it sound to me quite suspicious having to set -flags 0 in my command line in order NOT to get closed gops, as the default is a closed gop (I see weird having this as default).
Further, I usually avoid using a signed 'int' for a flag field, which needs to be handled with bitmasks, so into libavcodec/avcodec.h I would change those 'int flags;' and 'int flags2;' using unsigned instead; unless, of course, there are solid reasons to have them defined this way.

Elio
diff mbox series

Patch

diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 9bf8574e30..d363dae633 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -145,9 +145,9 @@  static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q,
 
     av_log(avctx, AV_LOG_VERBOSE, "GopPicSize: %"PRIu16"; GopRefDist: %"PRIu16"; GopOptFlag: ",
            info->GopPicSize, info->GopRefDist);
-    if (info->GopOptFlag & MFX_GOP_CLOSED)
+    if (info->GopOptFlag == MFX_GOP_CLOSED)
         av_log(avctx, AV_LOG_VERBOSE, "closed ");
-    if (info->GopOptFlag & MFX_GOP_STRICT)
+    if (info->GopOptFlag == MFX_GOP_STRICT)
         av_log(avctx, AV_LOG_VERBOSE, "strict ");
     av_log(avctx, AV_LOG_VERBOSE, "; IdrInterval: %"PRIu16"\n", info->IdrInterval);
 
@@ -510,7 +510,7 @@  static int init_video_param(AVCodecContext *avctx, QSVEncContext *q)
     q->param.mfx.GopPicSize         = FFMAX(0, avctx->gop_size);
     q->param.mfx.GopRefDist         = FFMAX(-1, avctx->max_b_frames) + 1;
     q->param.mfx.GopOptFlag         = avctx->flags & AV_CODEC_FLAG_CLOSED_GOP ?
-                                      MFX_GOP_CLOSED : 0;
+                                      MFX_GOP_CLOSED : MFX_GOP_STRICT;
     q->param.mfx.IdrInterval        = q->idr_interval;
     q->param.mfx.NumSlice           = avctx->slices;
     q->param.mfx.NumRefFrame        = FFMAX(0, avctx->refs);