> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Li, Zhong > Sent: Thursday, March 28, 2019 7:12 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] qsv: fix the dangerous macro > definitions > > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > Behalf > > Of Carl Eugen Hoyos > > Sent: Thursday, March 28, 2019 6:55 PM > > To: FFmpeg development discussions and patches > > <ffmpeg-devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH] qsv: fix the dangerous macro > > definitions > > > > 2019-03-28 10:09 GMT+01:00, Li, Zhong <zhong...@intel.com>: > > >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On > > Behalf > > >> Of Mark Thompson > > >> Sent: Thursday, March 28, 2019 6:23 AM > > >> To: ffmpeg-devel@ffmpeg.org > > >> Subject: Re: [FFmpeg-devel] [PATCH] qsv: fix the dangerous macro > > >> definitions > > >> > > >> On 27/03/2019 10:24, Zhong Li wrote: > > >> > Signed-off-by: Zhong Li <zhong...@intel.com> > > >> > --- > > >> > libavcodec/qsv_internal.h | 8 ++++---- > > >> > libavfilter/qsvvpp.h | 8 ++++---- > > >> > 2 files changed, 8 insertions(+), 8 deletions(-) > > >> > > > >> > diff --git a/libavcodec/qsv_internal.h > > >> > b/libavcodec/qsv_internal.h index 394c558883..86a5dbad98 100644 > > >> > --- a/libavcodec/qsv_internal.h > > >> > +++ b/libavcodec/qsv_internal.h > > >> > @@ -35,12 +35,12 @@ > > >> > #define QSV_MAX_ENC_PAYLOAD 2 // # of mfxEncodeCtrl > > >> payloads supported > > >> > > > >> > #define QSV_VERSION_ATLEAST(MAJOR, MINOR) \ > > >> > - (MFX_VERSION_MAJOR > (MAJOR) || \ > > >> > - MFX_VERSION_MAJOR == (MAJOR) && > > MFX_VERSION_MINOR >= > > >> (MINOR)) > > >> > + ((MFX_VERSION_MAJOR > (MAJOR) || \ > > >> > + MFX_VERSION_MAJOR == (MAJOR) && > > MFX_VERSION_MINOR >= > > >> (MINOR))) > > >> > > >> I don't understand why this makes a difference? > > >> > > >> Removing the whitespace, I see: > > >> > > >> - (MFX_VERSION_MAJOR > (MAJOR) || MFX_VERSION_MAJOR == > > (MAJOR) && > > >> MFX_VERSION_MINOR >= (MINOR)) > > >> + ((MFX_VERSION_MAJOR > (MAJOR) || MFX_VERSION_MAJOR == > > (MAJOR) > > >> && > > >> + MFX_VERSION_MINOR >= (MINOR))) > > >> > > >> which looks like you've just added redundant parentheses around the > > >> whole thing which already had them? > > >> > > >> (Alternative question: what case is this trying to fix? > > > > > > It is to fix the dangerous case of QSV_RUNTIME_VERSION_ATLEAST. > > > > > It is introducing a big issue, e:g > > > if ((avctx->codec_id != AV_CODEC_ID_HEVC) || > > > QSV_RUNTIME_VERSION_ATLEAST(q->ver, 1, 28)) it is equal to: > > > (avctx->codec_id != AV_CODEC_ID_HEVC) || (MFX_VERSION.Major > > > (MAJOR)) > > > || (MFX_VERSION.Major == (MAJOR) && MFX_VERSION.Minor >= > > (MINOR)) > > > > No, they are not equal, you added the second ")" after MAJOR. > > > > Carl Eugen > > I can find the second "}" you mentioned. This is the original define:
Sorry for the typo, can -> can't _______________________________________________ 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".
