[FFmpeg-devel] avutil/hwcontext_qsv: fix log level for session initialization error

Submitted by Moritz Barsnick on July 4, 2018, 2:24 p.m.

Details

Message ID 20180704142442.GB5061@sunshine.barsnick.net
State New
Headers show

Commit Message

Moritz Barsnick July 4, 2018, 2:24 p.m.
While the error is not fatal, the message should not be displayed only
in verbose logging, as its consequences are of interest.

Also fix message formatting (missing space).

Signed-off-by: Moritz Barsnick <barsnick@gmx.net>
---

Ideally, the returned mfxStatus would be evaluated and printed, but no
such function is available (yet). %d perhaps?

 libavutil/hwcontext_qsv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Zhong Li July 5, 2018, 10:17 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Moritz Barsnick

> Sent: Wednesday, July 4, 2018 10:25 PM

> To: FFmpeg development discussions and patches

> <ffmpeg-devel@ffmpeg.org>

> Subject: [FFmpeg-devel] [PATCH] avutil/hwcontext_qsv: fix log level for

> session initialization error

> 

> While the error is not fatal, the message should not be displayed only in

> verbose logging, as its consequences are of interest.

> 

> Also fix message formatting (missing space).

> 

> Signed-off-by: Moritz Barsnick <barsnick@gmx.net>

> ---

> 

> Ideally, the returned mfxStatus would be evaluated and printed, but no such

> function is available (yet). %d perhaps?


ff_qsv_print_error() can be used to print detailed error type. Is it helpful?

> 

>  libavutil/hwcontext_qsv.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

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

> 250091c4e8..e7ffb42f37 100644

> --- a/libavutil/hwcontext_qsv.c

> +++ b/libavutil/hwcontext_qsv.c

> @@ -474,7 +474,7 @@ static int

> qsv_init_internal_session(AVHWFramesContext *ctx,

> 

>      err = MFXVideoVPP_Init(*session, &par);

>      if (err != MFX_ERR_NONE) {

> -        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP

> session."

> +        av_log(ctx, AV_LOG_WARNING, "Error opening the internal VPP

> session. "

>                 "Surface upload/download will not be possible\n");

>          MFXClose(*session);

>          *session = NULL;

> --

> 2.14.4
Moritz Barsnick July 5, 2018, 1:20 p.m.
On Thu, Jul 05, 2018 at 10:17:35 +0000, Li, Zhong wrote:
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Moritz Barsnick
[...]
> > Ideally, the returned mfxStatus would be evaluated and printed, but no such
> > function is available (yet). %d perhaps?
> 
> ff_qsv_print_error() can be used to print detailed error type. Is it helpful?

Basically yes, even in other places, but that function's in libavcodec,
and we're in libavutil. We would need to move the code around IIUC, but
I don't have any way of testing new code with this failing scenario
right now (I have this effect on a Windows machine, but can't actually
build on Windows), so that would be a blind modification from my side.
Better if the QSV maintainers (i.e. you ;-)) did this.

This is the issue it would help to understand:
http://ffmpeg.org/pipermail/ffmpeg-user/2018-July/040438.html

Thanks,
Moritz
Maxym Dmytrychenko July 5, 2018, 2:17 p.m.
do you really want to have warning log with the error statement inside?
(root case of such MFXVideoVPP_Init behaviour is different topic and
actually should be re-checked)

just looking at the initial issue on Windows:
>I thought I had the entire pipeline (decode/scale-resize/encode) being
performed by Quicksync
it is not yet clear if exactly so and logs have to be checked,
like CPU utilization can be high due to decode, as an example.

Command line can be easier, if input codec format is supported by qsv,
something like:
ffmpeg.exe -hwaccel qsv  -c:v h264_qsv -i "testvid.mp4" -vf
"scale_qsv=640:360" -b:v 800k -c:v h264_qsv -c:a copy -y "testoutput.mp4"

regards
Maxym

On Thu, Jul 5, 2018 at 3:20 PM Moritz Barsnick <barsnick@gmx.net> wrote:

> On Thu, Jul 05, 2018 at 10:17:35 +0000, Li, Zhong wrote:
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> Of Moritz Barsnick
> [...]
> > > Ideally, the returned mfxStatus would be evaluated and printed, but no
> such
> > > function is available (yet). %d perhaps?
> >
> > ff_qsv_print_error() can be used to print detailed error type. Is it
> helpful?
>
> Basically yes, even in other places, but that function's in libavcodec,
> and we're in libavutil. We would need to move the code around IIUC, but
> I don't have any way of testing new code with this failing scenario
> right now (I have this effect on a Windows machine, but can't actually
> build on Windows), so that would be a blind modification from my side.
> Better if the QSV maintainers (i.e. you ;-)) did this.
>
> This is the issue it would help to understand:
> http://ffmpeg.org/pipermail/ffmpeg-user/2018-July/040438.html
>
> Thanks,
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Moritz Barsnick July 5, 2018, 10:12 p.m.
On Thu, Jul 05, 2018 at 16:17:45 +0200, Maxym Dmytrychenko wrote:
> just looking at the initial issue on Windows:
> >I thought I had the entire pipeline (decode/scale-resize/encode) being
> performed by Quicksync
> it is not yet clear if exactly so and logs have to be checked,
> like CPU utilization can be high due to decode, as an example.

That may be the case, I don't know, you know much better. It's the
general question:

> do you really want to have warning log with the error statement inside?
> (root case of such MFXVideoVPP_Init behaviour is different topic and
> actually should be re-checked)

It's just that I was under the impression that a non-fatal "error"
which changes the behavior was not being clearly reported. If it
doesn't change behavior, or this change is of no importance to the
user, then it can stay as is. If a message helps to resolve issues, it
should be as informative as possible.

Moritz
Zhong Li July 10, 2018, 8:43 a.m.
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Moritz Barsnick

> Sent: Thursday, July 5, 2018 9:20 PM

> To: FFmpeg development discussions and patches

> <ffmpeg-devel@ffmpeg.org>

> Subject: Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_qsv: fix log level for

> session initialization error

> 

> On Thu, Jul 05, 2018 at 10:17:35 +0000, Li, Zhong wrote:

> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > Behalf Of Moritz Barsnick

> [...]

> > > Ideally, the returned mfxStatus would be evaluated and printed, but

> > > no such function is available (yet). %d perhaps?

> >

> > ff_qsv_print_error() can be used to print detailed error type. Is it helpful?

> 

> Basically yes, even in other places, but that function's in libavcodec, and

> we're in libavutil. We would need to move the code around IIUC,but I don't


Yes, it is a general problem in libavfilter an libavutil. Currently for mostly all cases "status != MFX_ERR_NONE", AVERROR_UNKNOWN is returned. 
This is confusing just like you mentioned mfx status haven't been printed. And also MFX_WRN_XX is not fatal, giving a warning message should be enough.
So making ff_qsv_print_error() can be called by libavfilter/libavutils should be a good idea.

> have any way of testing new code with this failing scenario right now (I have

> this effect on a Windows machine, but can't actually build on Windows), so

> that would be a blind modification from my side.

> Better if the QSV maintainers (i.e. you ;-)) did this.

> 

> This is the issue it would help to understand:

> http://ffmpeg.org/pipermail/ffmpeg-user/2018-July/040438.html

> 

> Thanks,

> Moritz

Patch hide | download patch | download mbox

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 250091c4e8..e7ffb42f37 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -474,7 +474,7 @@  static int qsv_init_internal_session(AVHWFramesContext *ctx,
 
     err = MFXVideoVPP_Init(*session, &par);
     if (err != MFX_ERR_NONE) {
-        av_log(ctx, AV_LOG_VERBOSE, "Error opening the internal VPP session."
+        av_log(ctx, AV_LOG_WARNING, "Error opening the internal VPP session. "
                "Surface upload/download will not be possible\n");
         MFXClose(*session);
         *session = NULL;