| Message ID | 20180704142442.GB5061@sunshine.barsnick.net |
|---|---|
| State | New |
| Headers | show |
> -----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
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
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 >
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
> -----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
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;
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(-)