2018-05-24 8:05 GMT+08:00 Mark Thompson <s...@jkqxz.net>:
> 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?
maybe give a assert0?
>
>> + 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.
Oh, from the API define, "array is terminated if "status==-1" is
detected", so I don't know
the error entry number from VA-API level (but I checked the iHD source code,
I think iHD always return 2 error entries in this case). In current
FFmpeg VA-API
HWaccel decoding logic, I believe if vaSyncSurface fail, FFmpeg will return the
error and exit, so we don't have a change to print the error message
every frame.
>> + av_log(hwfc, AV_LOG_ERROR, "Decoding deatils
>> error, "
>
> "Decoding error details:" maybe?
Ok, will change it
>
>> + "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.
Yes, I have the same thought about this API, basically, I think
vaQuerySurfaceError()
need to return the pointer and ask the caller to free the error
entries like getaddressinfo()/freeaddrinfo(),
and I think need to submit a patch to VA-API/iHD driver for this part.
>
> 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.)
I can give a case, when I debug the ticket #7200, I try to dump the
YUV but fail,
so I want to get more decoding error from iHD driver.
use the command like this: ./ffmpeg_g -hwaccel vaapi -vaapi_device
/dev/dri/renderD128 -hwaccel_flags allow_profile_mismatch -i test.ts
-f null /dev/null
I think give a decoder option in this case is a good suggestion, I
will check this way. Thanks.
>
> - Mark
> _______________________________________________
> 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
