> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Andreas Håkon
> Sent: Monday, April 22, 2019 8:21 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code with
> CO3 define
>
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Monday, 22 de April de 2019 13:33, Li, Zhong <zhong...@intel.com>
> wrote:
>
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> > > Behalf Of Andreas Håkon via ffmpeg-devel
> > > Sent: Thursday, April 18, 2019 6:11 PM
> > > To: FFmpeg development discussions and patches
> > > ffmpeg-devel@ffmpeg.org
> > > Cc: Andreas Håkon andreas.ha...@protonmail.com
> > > Subject: [FFmpeg-devel] [PATCH] libavcodec: QSV protect GPB code
> > > with
> > > CO3 define
> > > Hi,
> > > In response to this ticket I provide the first part of the patch:
> > > https://trac.ffmpeg.org/ticket/7839
> > > This QSV_HAVE_GPB code needs to be protected by QSV_HAVE_CO3.
> > > Regards.
> > > A.H.
> >
> > Why it is must? It is impossible that QSV_HAVE_GPB is enabled but
> QSV_HAVE_CO3 is not enabled.
> > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API
> V1.11.
> >
>
> Hi Li,
>
> > Why it is must?
>
> Let me to explain why:
>
> - The "#if QSV_HAVE_GPB" only appears two times inside
> "/libavcodec/qsvenc.c":
> 1.
> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L75
> 6
> 2.
> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvenc.c#L27
> 0
>
> In the first occurrence (line #756) the code is protected by one "#if
> QSV_HAVE_CO3". This is necessary because if QSV_HAVE_CO3 is not enabled
> then the code fails when using "extco3.GPB". The original author (which I
> think was you) included the test with sound common sense.
I belive they are different. If you extend the MARCIO, they are actually:
Case 1:
#if QSV_VERSION_ATLEAST(1, 11)
q->extco3.Header.BufferId = MFX_EXTBUFF_CODING_OPTION3;
q->extco3.Header.BufferSz = sizeof(q->extco3);
#if QSV_VERSION_ATLEAST(1, 18)
if (avctx->codec_id == AV_CODEC_ID_HEVC)
q->extco3.GPB = q->gpb ? MFX_CODINGOPTION_ON :
MFX_CODINGOPTION_OFF;
#endif
q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer
*)&q->extco3;
#endif
And Case2:
#if QSV_VERSION_ATLEAST(1, 18)
if (avctx->codec_id == AV_CODEC_ID_HEVC)
av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB));
#endif
You must add "#if QSV_VERSION_ATLEAST(1, 11)" for case 1, else "
q->extco3.Header.BufferId = MFX_EXTBUFF_CODING_OPTION3;" is broken.
But you don't need change Case 2 to be:
#if QSV_VERSION_ATLEAST(1, 11)
#if QSV_VERSION_ATLEAST(1, 18)
if (avctx->codec_id == AV_CODEC_ID_HEVC)
av_log(avctx, AV_LOG_VERBOSE,"GPB: %s\n", print_threestate(co3->GPB));
#endif
#endif
> In the second occurrence (line #270) where my patch is applied, the code
> isn't protected. The reason is because this code is changed **after**. And
> the developer forgot to protect it.
>
> So my patch is a simple fixing.
>
> > It is impossible that QSV_HAVE_GPB is enabled but QSV_HAVE_CO3 is not
> enabled.
> > QSV_HAVE_GPB is enabled by MSDK API v1.18, but QSV_HAVE_CO3 is API
> V1.11.
>
> This isn't true in all cases. As described in
> "https://trac.ffmpeg.org/ticket/7839";
> one current bug breaks the "mpeg2_qsv" encoder. One solution is to
> MANUALLY disable the QVBR. This can be done with a simple "#define
> QSV_HAVE_QVBR 0". Even if you compile with a recent version of the SDK >
> v1.11.
>
> But the problem is then that this implies too to disable "QSV_HAVE_CO3".
> And doing this the code doesn't compile as this patch isn't applied.
>
> So, the best solution is to merge this patch. It enables then the option to
> disable manually what you like, and the code compiles clean.
>
> Please, accept the patch.
IMHO, your patch is only needed when disable "QSV_HAVE_CO3", but tiket#7839 is
not root caused now.
I will consider to accept it when tiket#7839 is root caused.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
