[FFmpeg-devel] [PATCH v2 1/2] qsv: needn't load user plugin since libmfx 1.28
Soft Works
softworkz at hotmail.com
Mon Aug 24 20:54:53 EEST 2020
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Xiang, Haihao
> Sent: Monday, August 24, 2020 8:15 AM
> To: ffmpeg-devel at 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 20:11 +0000, Soft Works wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > Soft Works
> > > Sent: Friday, August 21, 2020 9:45 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel at 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-bounces at ffmpeg.org> On Behalf
> Of
> > > > Xiang, Haihao
> > > > Sent: Friday, August 21, 2020 9:29 AM
> > > > To: ffmpeg-devel at 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-bounces at ffmpeg.org> On
> Behalf
> > >
> > > Of
> > > > > > Haihao Xiang
> > > > > > Sent: Friday, August 21, 2020 7:23 AM
> > > > > > To: ffmpeg-devel at ffmpeg.org
> > > > > > Cc: Haihao Xiang <haihao.xiang at 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
Hi Haihao,
> - 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.
At the other places where this macro is being used, it's about checking whether
certain new features are available at the SDK level and in many cases this
is accompanied by runtime checks as well.
> - Considering that the plugins are redundant now, it is possible to deprecate
> or remove these plugins in the future
The only thing that is deprecated are the plugins that are delivered as part
of the MSDK. Two HEVC software plugins and one HEVC partial-hw plugin.
The real hardware plugins have always been part of the driver and with the
latest versions, it just no longer required to load them explicitly.
> once MFXVideoUSER_Load is deprecated or
> removed from the SDK in the future
Even the latest drivers (1.31) are still using the plugin mechanism internally
(see exports of mfxplugin64_hw.dll for example) and the SDK docs do not
indicate an imminent deprecation of the plugin api either.
> I am fine not to accept this patch presently
Thank you very much for your understanding, this would have affected many
users.
Kind regards,
softworkz
More information about the ffmpeg-devel
mailing list