[FFmpeg-devel] avcodec/vaapi_h264: skip decode if pic has no slices

Mark Thompson sw at jkqxz.net
Wed Mar 13 03:00:17 EET 2019


On 09/03/2019 20:30, Peter F wrote:
> Thank you very much for your reply.
> Am Sa., 9. März 2019 um 21:09 Uhr schrieb Jan Ekström <jeebjp at gmail.com>:
> 
>>> From 386c94489a86bb747b6531f727843cf259a24f5d Mon Sep 17 00:00:00 2001
>>> From: xbmc <fernetmenta at online.de>
>> Is this author field meant to not have an actual name in it? Just verifying.
> 
> It can stay as is. The original author sometimes uses fernetmenta /
> xbmc depending on his
> local git configuration. The email is unique though. I just
> transported it upstream and fixed
> the minors.
> 
>>
>>> Date: Sat, 26 Jan 2019 19:48:35 +0100
>>> Subject: [PATCH] avcodec/vaapi_h264: skip decode if pic has no slices
>>
>> Something along the lines of "avcodec/vaapi_h264: skip decoding if no
>> slices were provided"?
>>
>> Also I would prefer if the reasoning for skipping decode on our layer
>> would be explained in further lines of the commit message, like you
>> have nicely explained it in the initial e-mail (to work-around a mesa
>> vaapi driver bug).
>> I don't remember the specifics of AVC, but are there valid VCL NAL
>> units without slices (and would such end up in this code path to begin
>> with)? I would guess that there would be no such valid VCL NAL units
>> (or if there were, they wouldn't get to this point in the decode
>> chain) - mostly just noting that this might be something we would like
>> to check to verify if this should be an error or a "normal" state.
>>
>> The general idea I'm OK with since if we already know that there's no
>> slices to decode, we might as well skip decoding as long as that
>> doesn't cause issues with the state of the underlying
>> libraries/drivers. Thus, I would mostly just wait for a reply from one
>> of the VAAPI wrapper maintainers regarding if this skip should happen
>> here or earlier in the decode process (where the buffers are being
>> allocated).

I suspect it would make sense for this to happen earlier in the decoder if it knows it doesn't have any slices (therefore not calling any hwaccel code at all), but I'm not really sure - maybe in this case it doesn't know early enough so you always have the start_frame call.  The current setup is going to allocate a reference frame but then with this change never do anything with it, which seems bad from an uninitialised-data perspective.  On the other hand, I agree that calling into the hardware decoder with no slices seems rather unhelpful, and it probably doesn't write anything useful to the frame either.

Still, all of the other hwaccel APIs (VDPAU, DXVA2, NVDEC) do currently do the same thing too, though, so it would seem nicer if this could be kept consistent across all of them.  If Mesa with VAAPI is the only case where this fails (checking Mesa with VDPAU on the same hardware might be interesting?) then fixing it in the driver would seem the best answer.  I might have a look at that later.

> From 3c4885579e86f6c002e614c4082a3bdb02d8426e Mon Sep 17 00:00:00 2001
> From: xbmc <fernetmenta at online.de>
> Date: Sat, 26 Jan 2019 19:48:35 +0100
> Subject: [PATCH] avcodec/vaapi_h264: skip decode if pic has no slices
> 
> This fixes / workarounds https://bugs.freedesktop.org/show_bug.cgi?id=105368.
> It was hit frequently when watching h264 channels received via DVB-X.
> Corresponding kodi bug: https://github.com/xbmc/xbmc/issues/15704
> ---
>  libavcodec/vaapi_h264.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
> index 5854587a25..f12fdc457a 100644
> --- a/libavcodec/vaapi_h264.c
> +++ b/libavcodec/vaapi_h264.c
> @@ -317,6 +317,11 @@ static int vaapi_h264_end_frame(AVCodecContext *avctx)
>      H264SliceContext *sl = &h->slice_ctx[0];
>      int ret;
>  
> +    if (pic->nb_slices == 0) {
> +        ret = AVERROR_INVALIDDATA;
> +        goto finish;
> +    }
> +
>      ret = ff_vaapi_decode_issue(avctx, pic);
>      if (ret < 0)
>          goto finish;
> -- 
> 2.20.1

This pastes the parameter buffers you have already sent on to the front of the next frame, which is going to do something pretty weird - I think you want to call ff_vaapi_decode_cancel() before returning.

Have you tried this on the i965 and iHD drivers and made sure it doesn't do anything nasty in the same case there?  (Or if you can provide a sample file I'll have a go myself.)

Thanks,

- Mark


More information about the ffmpeg-devel mailing list