diff mbox

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

Message ID CAMQ1+vy1FGGJ8uSymxe+saS-O2uyHnJq6ivZNBP5EuV7EtuuEA@mail.gmail.com
State Superseded
Headers show

Commit Message

Peter F March 9, 2019, 3:49 p.m. UTC
Resent - layer 8 issue with using a mail client, sorry for the noise:

From 386c94489a86bb747b6531f727843cf259a24f5d Mon Sep 17 00:00:00 2001
From: xbmc <fernetmenta@online.de>
Date: Sat, 26 Jan 2019 19:48:35 +0100
Subject: [PATCH] avcodec/vaapi_h264: skip decode if pic has no slices

---
 libavcodec/vaapi_h264.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Carl Eugen Hoyos March 9, 2019, 4:18 p.m. UTC | #1
2019-03-09 16:49 GMT+01:00, Peter F <peter.fruehberger@gmail.com>:
> Resent - layer 8 issue with using a mail client

Attachments are fine...

Carl Eugen
Jan Ekström March 9, 2019, 8:02 p.m. UTC | #2
On Sat, Mar 9, 2019 at 5:49 PM Peter F <peter.fruehberger@gmail.com> wrote:
>
> Resent - layer 8 issue with using a mail client, sorry for the noise:
>
> From 386c94489a86bb747b6531f727843cf259a24f5d Mon Sep 17 00:00:00 2001
> From: xbmc <fernetmenta@online.de>

Is this author field meant to not have an actual name in it? Just verifying.

> 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).

>
> ---
>  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;

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).

> +        goto finish;
> +    }
> +
>      ret = ff_vaapi_decode_issue(avctx, pic);
>      if (ret < 0)
>          goto finish;
> --
> 2.20.1

Thanks for the contribution,
Jan
diff mbox

Patch

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;