[FFmpeg-devel] [PATCH v2 1/2] qsv: needn't load user plugin since libmfx 1.28

[Soft Works]

> -----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?

Kind regards,
softworkz