[FFmpeg-devel] [PATCH] avcodec/qsv: fix coding options

[Ivan Uskov]

Hello Kyle,

Saturday, May 28, 2016, 8:07:07 AM, you wrote:

zgc> From: Kyle Schwarz <zeranoe at gmail.com>

zgc> ---
zgc>  libavcodec/qsvenc.c | 34 ++--------------------------------
zgc>  1 file changed, 2 insertions(+), 32 deletions(-)

zgc> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
zgc> index 132cf47..b4821fc 100644
zgc> --- a/libavcodec/qsvenc.c
zgc> +++ b/libavcodec/qsvenc.c
zgc> @@ -132,9 +132,6 @@ static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q,
zgc>  #if QSV_HAVE_CO2
zgc>      mfxExtCodingOption2 *co2 = (mfxExtCodingOption2*)coding_opts[1];
zgc>  #endif
zgc> -#if QSV_HAVE_CO3
zgc> -    mfxExtCodingOption3 *co3 = (mfxExtCodingOption3*)coding_opts[2];
zgc> -#endif
zgc>  
zgc>      av_log(avctx, AV_LOG_VERBOSE, "profile: %s; level: %"PRIu16"\n",
zgc>             print_profile(info->CodecProfile), info->CodecLevel);
zgc> @@ -186,13 +183,6 @@ static void dump_video_param(AVCodecContext *avctx, QSVEncContext *q,
zgc>                 info->ICQQuality, co2->LookAheadDepth);
zgc>      }
zgc>  #endif
zgc> -#if QSV_HAVE_QVBR
-    else if (info->>RateControlMethod == MFX_RATECONTROL_QVBR) {
zgc> -        av_log(avctx, AV_LOG_VERBOSE, "QVBRQuality: %"PRIu16"\n",
zgc> -               co3->QVBRQuality);
zgc> -    }
zgc> -#endif
zgc> -
zgc>      av_log(avctx, AV_LOG_VERBOSE, "NumSlice: %"PRIu16"; NumRefFrame: %"PRIu16"\n",
zgc>             info->NumSlice, info->NumRefFrame);
zgc>      av_log(avctx, AV_LOG_VERBOSE, "RateDistortionOpt: %s\n",
zgc> @@ -567,31 +557,11 @@ static int qsv_retrieve_enc_params(AVCodecContext *avctx, QSVEncContext *q)
zgc>          .PPSBuffer = pps_buf, .PPSBufSize = sizeof(pps_buf)
zgc>      };
zgc>  
zgc> -    mfxExtCodingOption co = {
zgc> -        .Header.BufferId = MFX_EXTBUFF_CODING_OPTION,
zgc> -        .Header.BufferSz = sizeof(co),
zgc> -    };
zgc> -#if QSV_HAVE_CO2
zgc> -    mfxExtCodingOption2 co2 = {
zgc> -        .Header.BufferId = MFX_EXTBUFF_CODING_OPTION2,
zgc> -        .Header.BufferSz = sizeof(co2),
zgc> -    };
zgc> -#endif
zgc> -#if QSV_HAVE_CO3
zgc> -    mfxExtCodingOption3 co3 = {
zgc> -        .Header.BufferId = MFX_EXTBUFF_CODING_OPTION3,
zgc> -        .Header.BufferSz = sizeof(co3),
zgc> -    };
zgc> -#endif
zgc> -
zgc>      mfxExtBuffer *ext_buffers[] = {
zgc>          (mfxExtBuffer*)&extradata,
zgc> -        (mfxExtBuffer*)&co,
zgc> +        (mfxExtBuffer*)&q->extco,
zgc>  #if QSV_HAVE_CO2
zgc> -        (mfxExtBuffer*)&co2,
zgc> -#endif
zgc> -#if QSV_HAVE_CO3
zgc> -        (mfxExtBuffer*)&co3,
zgc> +        (mfxExtBuffer*)&q->extco2,
zgc>  #endif
zgc>      };
zgc>  
Could you please explain what exactly you are trying to fix by this patch?
I do not like this changes by two reasons:
1.   The   dump_video_param()   intended   to   dump   parameters  returned  by
MFXVideoENCODE_GetVideoParam();  You can  not use q->extco and q->extco2 here
because  are  can be differ that values really used by encoder, here internal
adjustments possible.
2. Disabling preprocessor conditions does break compatibility with different MSDK
versions.  For  example  MFX_EXTBUFF_CODING_OPTION2  existing since MSDK 1.6,
MFX_EXTBUFF_CODING_OPTION3 existing since MSDK 1.11



-- 
Best regards,
 Ivan                            mailto:ivan.uskov at nablet.com