On 23/05/18 11:29, Jun Zhao wrote:
> dump more decoding error details when sync surface fail.
> 
> Signed-off-by: Jun Zhao <mypopy...@gmail.com>
> ---
>  libavutil/hwcontext_vaapi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index e55bb8d..5bdb02f 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -742,6 +742,23 @@ static int vaapi_map_frame(AVHWFramesContext *hwfc,
>          av_log(hwfc, AV_LOG_ERROR, "Failed to sync surface "
>                 "%#x: %d (%s).\n", surface_id, vas, vaErrorStr(vas));
>          err = AVERROR(EIO);
> +        /* query decode detail error */
> +        if (vas == VA_STATUS_ERROR_DECODING_ERROR) {
> +            VASurfaceDecodeMBErrors *dec_err = NULL;
> +            int i;
> +            vas = vaQuerySurfaceError(hwctx->display, surface_id, 
> VA_STATUS_ERROR_DECODING_ERROR,
> +                                      (void **)&dec_err);
> +            if (VA_STATUS_SUCCESS == vas) {
> +                if (NULL != dec_err) {

Why this check?

> +                    for (i = 0; dec_err[i].status != -1; i++) {

How many items can be in this list?  We probably don't want to print more than 
one in the error log if this can happen on every frame in a broken stream.

> +                          av_log(hwfc, AV_LOG_ERROR, "Decoding deatils 
> error, "

"Decoding error details:" maybe?

> +                                 "type: %d, start mb: %d, end md: %d, num 
> mbs: %d.\n",
> +                                 dec_err[i].decode_error_type, 
> dec_err[i].start_mb,
> +                                 dec_err[i].end_mb, dec_err[i].num_mb);
> +                    }
> +                }
> +            }
> +        }
>          goto fail;
>      }
>  
> 

What is the lifetime of the returned pointer here?  This can certainly be 
called on any thread asynchronously.

It's a bit nasty to have this in the generic code rather than the decoder: very 
few actual uses are going to call SyncSurface directly on a decoder output - 
rather they will pass it to some other component (another VAAPI one, or 
export).  Could we instead put this inside the decoder, enabled by some debug 
option?  (Some sort of "catch decoder errors earlier at a cost of (possibly 
greatly) decreased performance" flag.)

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

Reply via email to