[FFmpeg-devel,V2] lavu/qsv: remove the redundant libmfx init code

Submitted by Zhong Li on Sept. 5, 2019, 5:24 a.m.

Details

Message ID 1567661059-19626-1-git-send-email-zhong.li@intel.com
State New
Headers show

Commit Message

Zhong Li Sept. 5, 2019, 5:24 a.m.
Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 libavutil/hwcontext_qsv.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

Comments

Mark Thompson Sept. 10, 2019, 10:33 p.m.
On 05/09/2019 06:24, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  libavutil/hwcontext_qsv.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index 0329a81..1c0e4ff 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -1145,27 +1145,6 @@ static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
>  
>      err = MFXInit(implementation, &ver, &hwctx->session);
>      if (err != MFX_ERR_NONE) {
> -        av_log(ctx, AV_LOG_ERROR, "Error initializing an MFX session: "
> -               "%d.\n", err);
> -        ret = AVERROR_UNKNOWN;
> -        goto fail;
> -    }
> -
> -    err = MFXQueryVersion(hwctx->session, &ver);
> -    if (err != MFX_ERR_NONE) {
> -        av_log(ctx, AV_LOG_ERROR, "Error querying an MFX session: %d.\n", err);
> -        ret = AVERROR_UNKNOWN;
> -        goto fail;
> -    }
> -
> -    av_log(ctx, AV_LOG_VERBOSE,
> -           "Initialize MFX session: API version is %d.%d, implementation version is %d.%d\n",
> -           MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);
> -
> -    MFXClose(hwctx->session);
> -
> -    err = MFXInit(implementation, &ver, &hwctx->session);
> -    if (err != MFX_ERR_NONE) {
>          av_log(ctx, AV_LOG_ERROR,
>                 "Error initializing an MFX session: %d.\n", err);
>          ret = AVERROR_UNKNOWN;
> @@ -1182,7 +1161,8 @@ static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
>  
>      ret = MFXQueryVersion(hwctx->session,&ver);
>      if (ret == MFX_ERR_NONE) {
> -        av_log(ctx, AV_LOG_VERBOSE, "MFX compile/runtime API: %d.%d/%d.%d\n",
> +        av_log(ctx, AV_LOG_VERBOSE,
> +               "Initialize MFX session: API version is %d.%d, implementation version is %d.%d\n",
>                 MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);
>      }
>      return 0;
> 

Can you add some more explanation?  The extra session checking the version is not obviously redundant, and the commit log does indicate that the setup was deliberate:

commit ccbb31c14b766ef666ef2daa8c467e478183a957
Author: Luca Barbato <lu_zero@gentoo.org>
Date:   Mon Sep 25 09:57:30 2017 +0000

    qsv: Make sure the session is set with the latest version
    
    It is needed to have the calls to MFXJoinSession succeed.


- Mark
Zhong Li Sept. 13, 2019, 3:08 p.m.
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Mark

> Thompson

> Sent: Wednesday, September 11, 2019 6:34 AM

> To: ffmpeg-devel@ffmpeg.org

> Subject: Re: [FFmpeg-devel] [PATCH V2] lavu/qsv: remove the redundant libmfx

> init code

> 

> On 05/09/2019 06:24, Zhong Li wrote:

> > Signed-off-by: Zhong Li <zhong.li@intel.com>

> > ---

> >  libavutil/hwcontext_qsv.c | 24 ++----------------------

> >  1 file changed, 2 insertions(+), 22 deletions(-)

> >

> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c

> > index 0329a81..1c0e4ff 100644

> > --- a/libavutil/hwcontext_qsv.c

> > +++ b/libavutil/hwcontext_qsv.c

> > @@ -1145,27 +1145,6 @@ static int

> > qsv_device_derive_from_child(AVHWDeviceContext *ctx,

> >

> >      err = MFXInit(implementation, &ver, &hwctx->session);

> >      if (err != MFX_ERR_NONE) {

> > -        av_log(ctx, AV_LOG_ERROR, "Error initializing an MFX session: "

> > -               "%d.\n", err);

> > -        ret = AVERROR_UNKNOWN;

> > -        goto fail;

> > -    }

> > -

> > -    err = MFXQueryVersion(hwctx->session, &ver);

> > -    if (err != MFX_ERR_NONE) {

> > -        av_log(ctx, AV_LOG_ERROR, "Error querying an MFX session: %d.\n", err);

> > -        ret = AVERROR_UNKNOWN;

> > -        goto fail;

> > -    }

> > -

> > -    av_log(ctx, AV_LOG_VERBOSE,

> > -           "Initialize MFX session: API version is %d.%d, implementation version

> is %d.%d\n",

> > -           MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);

> > -

> > -    MFXClose(hwctx->session);

> > -

> > -    err = MFXInit(implementation, &ver, &hwctx->session);

> > -    if (err != MFX_ERR_NONE) {

> >          av_log(ctx, AV_LOG_ERROR,

> >                 "Error initializing an MFX session: %d.\n", err);

> >          ret = AVERROR_UNKNOWN;

> > @@ -1182,7 +1161,8 @@ static int

> > qsv_device_derive_from_child(AVHWDeviceContext *ctx,

> >

> >      ret = MFXQueryVersion(hwctx->session,&ver);

> >      if (ret == MFX_ERR_NONE) {

> > -        av_log(ctx, AV_LOG_VERBOSE, "MFX compile/runtime

> API: %d.%d/%d.%d\n",

> > +        av_log(ctx, AV_LOG_VERBOSE,

> > +               "Initialize MFX session: API version is %d.%d,

> > + implementation version is %d.%d\n",

> >                 MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);

> >      }

> >      return 0;

> >

> 

> Can you add some more explanation?  The extra session checking the version is

> not obviously redundant, and the commit log does indicate that the setup was

> deliberate:

> 

> commit ccbb31c14b766ef666ef2daa8c467e478183a957

> Author: Luca Barbato <lu_zero@gentoo.org>

> Date:   Mon Sep 25 09:57:30 2017 +0000

> 

>     qsv: Make sure the session is set with the latest version

> 

>     It is needed to have the calls to MFXJoinSession succeed.

> 

> 

> - Mark


Thanks to point out this. 
So probably just need to remove the redundant MFXQueryVersion()? 
Will update the patch.

Patch hide | download patch | download mbox

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 0329a81..1c0e4ff 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -1145,27 +1145,6 @@  static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
 
     err = MFXInit(implementation, &ver, &hwctx->session);
     if (err != MFX_ERR_NONE) {
-        av_log(ctx, AV_LOG_ERROR, "Error initializing an MFX session: "
-               "%d.\n", err);
-        ret = AVERROR_UNKNOWN;
-        goto fail;
-    }
-
-    err = MFXQueryVersion(hwctx->session, &ver);
-    if (err != MFX_ERR_NONE) {
-        av_log(ctx, AV_LOG_ERROR, "Error querying an MFX session: %d.\n", err);
-        ret = AVERROR_UNKNOWN;
-        goto fail;
-    }
-
-    av_log(ctx, AV_LOG_VERBOSE,
-           "Initialize MFX session: API version is %d.%d, implementation version is %d.%d\n",
-           MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);
-
-    MFXClose(hwctx->session);
-
-    err = MFXInit(implementation, &ver, &hwctx->session);
-    if (err != MFX_ERR_NONE) {
         av_log(ctx, AV_LOG_ERROR,
                "Error initializing an MFX session: %d.\n", err);
         ret = AVERROR_UNKNOWN;
@@ -1182,7 +1161,8 @@  static int qsv_device_derive_from_child(AVHWDeviceContext *ctx,
 
     ret = MFXQueryVersion(hwctx->session,&ver);
     if (ret == MFX_ERR_NONE) {
-        av_log(ctx, AV_LOG_VERBOSE, "MFX compile/runtime API: %d.%d/%d.%d\n",
+        av_log(ctx, AV_LOG_VERBOSE,
+               "Initialize MFX session: API version is %d.%d, implementation version is %d.%d\n",
                MFX_VERSION_MAJOR, MFX_VERSION_MINOR, ver.Major, ver.Minor);
     }
     return 0;