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

2018-07-10 Thread Li, Zhong
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Moritz Barsnick
> Sent: Thursday, July 5, 2018 9:20 PM
> To: FFmpeg development discussions and patches
> 
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_qsv: fix log level for
> session initialization error
> 
> On Thu, Jul 05, 2018 at 10:17:35 +, Li, Zhong wrote:
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-07-05 Thread Moritz Barsnick
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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-07-05 Thread Maxym Dmytrychenko
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  wrote:

> On Thu, Jul 05, 2018 at 10:17:35 +, Li, Zhong wrote:
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@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
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-07-05 Thread Moritz Barsnick
On Thu, Jul 05, 2018 at 10:17:35 +, Li, Zhong wrote:
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@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


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

2018-07-05 Thread Li, Zhong
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Moritz Barsnick
> Sent: Wednesday, July 4, 2018 10:25 PM
> To: FFmpeg development discussions and patches
> 
> 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 
> ---
> 
> 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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel