> 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:
#define QSV_RUNTIME_VERSION_ATLEAST(MFX_VERSION, MAJOR, MINOR) \
(MFX_VERSION.Major > (MAJOR)) || \
(MFX_VERSION.Major == (MAJOR) && MFX_VERSION.Minor >= (MINOR))
_______________________________________________
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".
