On Fri, 2020-08-21 at 20:11 +0000, Soft Works wrote: > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > Soft Works > > Sent: Friday, August 21, 2020 9:45 AM > > To: FFmpeg development discussions and patches <ffmpeg- > > de...@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsv: needn't load user plugin > > since libmfx 1.28 > > > > > > > > > -----Original Message----- > > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > > > Xiang, Haihao > > > Sent: Friday, August 21, 2020 9:29 AM > > > To: ffmpeg-devel@ffmpeg.org > > > Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] qsv: needn't load user > > > plugin since libmfx 1.28 > > > > > > On Fri, 2020-08-21 at 05:48 +0000, Soft Works wrote: > > > > > -----Original Message----- > > > > > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf > > > > Of > > > > > Haihao Xiang > > > > > Sent: Friday, August 21, 2020 7:23 AM > > > > > To: ffmpeg-devel@ffmpeg.org > > > > > Cc: Haihao Xiang <haihao.xi...@intel.com> > > > > > Subject: [FFmpeg-devel] [PATCH v2 1/2] qsv: needn't load user > > > > > plugin since libmfx 1.28 > > > > > > > > > > MFXVideoUSER_Load call is redundant since libmfx 1.28 > > > > > --- > > > > > Fixed merge conflict when applying this patch by 'git am' > > > > > > > > > > libavcodec/qsv.c | 9 ++++++++- > > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c index > > > > > 17720070f1..56a30ad642 100644 > > > > > --- a/libavcodec/qsv.c > > > > > +++ b/libavcodec/qsv.c > > > > > @@ -19,7 +19,6 @@ > > > > > */ > > > > > > > > > > #include <mfx/mfxvideo.h> > > > > > -#include <mfx/mfxplugin.h> > > > > > #include <mfx/mfxjpeg.h> > > > > > > > > > > #include <stdio.h> > > > > > @@ -36,6 +35,10 @@ > > > > > #include "avcodec.h" > > > > > #include "qsv_internal.h" > > > > > > > > > > +#if !QSV_VERSION_ATLEAST(1, 28) > > > > > +#include <mfx/mfxplugin.h> > > > > > +#endif > > > > > + > > > > > #if QSV_VERSION_ATLEAST(1, 12) > > > > > #include "mfx/mfxvp8.h" > > > > > #endif > > > > > @@ -295,6 +298,9 @@ enum AVPictureType ff_qsv_map_pictype(int > > > > > mfx_pic_type) static int qsv_load_plugins(mfxSession session, > > > > > const char *load_plugins, > > > > > void *logctx) { > > > > > +#if QSV_VERSION_ATLEAST(1, 28) > > > > > + return 0; > > > > > +#else > > > > > if (!load_plugins || !*load_plugins) > > > > > return 0; > > > > > > > > > > @@ -340,6 +346,7 @@ load_plugin_fail: > > > > > } > > > > > > > > > > return 0; > > > > > +#endif > > > > > > > > > > } > > > > > > > > > > > > Hi, > > > > > > > > Are you sure this check is right? You are checking the libmfx > > > > version against which ffmpeg is compiled. > > > > > > > > What happens, when a graphics driver supports only API level 1.21? > > > > > > > > > > I understand your concern, however lots of features in FFmpeg are > > > disabled/enabled against api version at compile-time. > > > > That is in no way an argument to break compatibility with downlevel drivers > > without any need. Things are working fine without that patch for all API > > versions. > > > > If you really want to avoid the plugin-load, then you should check the > > driver's > > API level at runtime. > > Haihao, > > let me add a few more points to this subject: > > - Backwards-Compatibility is a key feature of the Intel Media SDK > Your colleagues which are doing the MSDK development are continuously > taking care to make that possible > > - When you look at the latest release notes (MSDK 2020 R1, API 1.32), > you can see that 3rd, 4th and 5th gen CPUs are still supported > The latest 3rd gen drivers are API level 1.11 > The latest 4th and 5th gen drivers are API level 1.20 > Also later CPUs do not always have the latest drivers (> 1.28) installed > > - Even with libmfx 1.32, it is still possible to use those CPUs or older > driver versions, and that applies to ffmpeg as well, when it's compiled > against libmfx 1.32 > > - Your change would only be legitimate if there would have been a change > In the dispatcher code since API 1.28 that would automatically handle the > Plugin loading for drivers < 1.28. > But I have just gone through the recent changes to the dispatcher code > And there doesn't exist such functionality. > > - For those reasons, your commit is not acceptable as it would break > HEVC encoding for all 3,4,5 gen CPUs and later CPUs with downlevel > Drivers > > What remains is the question whether there's any benefit at all in > Avoiding the plugin load call. > > Does it cost time in cases when it would not be needed (driver level >= 1.28)? > > - If yes, then the driver API level can be checked at runtime to avoid the > Plugin load. > - Otherwise, I can't imagine any reason to make any change at all > > What are yours?
Hi softworkz, Thanks for your comment. - I definitely agree with you that it is important to keep backwards compatibility. - I am aware that using QSV_VERSION_ATLEAST at compile time might break backwards compatibility, but considering QSV_VERSION_ATLEAST has been used widely at compile time, I used it in this patch too. - Considering that the plugins are redundant now, it is possible to deprecate or remove these plugins in the future. I am fine not to accept this patch presently, and will pick it back once MFXVideoUSER_Load is deprecated or removed from the SDK in the future Thanks Haihao > > Kind regards, > softworkz > > > > > > _______________________________________________ > 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". _______________________________________________ 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".