[FFmpeg-devel] [PATCH] lavc: propagate hwaccel errors

[wm4]

On Wed, 5 Aug 2015 17:49:01 -0700
Philip Langdale <philipl at overt.org> wrote:

> On Wed,  5 Aug 2015 21:59:19 +0200
> wm4 <nfxjfg at googlemail.com> wrote:
> 
> > At least the new videotoolbox decoder does not actually set a frame if
> > end_frame fails. This causes the API to return success and signals
> > that a picture was decoded, even though AVFrame->data[0] is NULL.
> > 
> > Fix this by propagating end_frame errors.
> > ---
> > Now more correct.
> > 
> > Confirmed to fix a problem when videotoolbox refuses to decode
> > something. Hevc and mpeg are untested.
> 
> Looks reasonable, though I see vc1dec has the same problem.

vc1dec doesn't propagate the error codes, but at least it does
propagate the error itself. So it should work.

Applied the patch.

> > ---
> >  libavcodec/h264.c         | 3 ++-
> >  libavcodec/h264_picture.c | 3 ++-
> >  libavcodec/h264_slice.c   | 8 ++++++--
> >  libavcodec/hevc.c         | 5 ++++-
> >  libavcodec/mpeg12dec.c    | 9 +++++++--
> >  5 files changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> > index 8b575be..c4ab3fa 100644
> > --- a/libavcodec/h264.c
> > +++ b/libavcodec/h264.c
> > @@ -1844,7 +1844,8 @@ static int h264_decode_frame(AVCodecContext
> > *avctx, void *data, if (avctx->flags2 & AV_CODEC_FLAG2_CHUNKS)
> >              decode_postinit(h, 1);
> >  
> > -        ff_h264_field_end(h, &h->slice_ctx[0], 0);
> > +        if ((ret = ff_h264_field_end(h, &h->slice_ctx[0], 0)) < 0)
> > +            return ret;
> >  
> >          /* Wait for second field. */
> >          *got_frame = 0;
> > diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
> > index 81d90d7..04bbf02 100644
> > --- a/libavcodec/h264_picture.c
> > +++ b/libavcodec/h264_picture.c
> > @@ -172,7 +172,8 @@ int ff_h264_field_end(H264Context *h,
> > H264SliceContext *sl, int in_setup) }
> >  
> >      if (avctx->hwaccel) {
> > -        if (avctx->hwaccel->end_frame(avctx) < 0)
> > +        err = avctx->hwaccel->end_frame(avctx);
> > +        if (err < 0)
> >              av_log(avctx, AV_LOG_ERROR,
> >                     "hardware accelerator failed to decode
> > picture\n"); }
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index b088392..48f501b 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1170,15 +1170,19 @@ int ff_h264_decode_slice_header(H264Context
> > *h, H264SliceContext *sl) if (h->current_slice) {
> >              av_assert0(!h->setup_finished);
> >              if (h->cur_pic_ptr && FIELD_PICTURE(h) &&
> > h->first_field) {
> > -                ff_h264_field_end(h, h->slice_ctx, 1);
> > +                ret = ff_h264_field_end(h, h->slice_ctx, 1);
> >                  h->current_slice = 0;
> > +                if (ret < 0)
> > +                    return ret;
> >              } else if (h->cur_pic_ptr && !FIELD_PICTURE(h)
> > && !h->first_field && h->nal_unit_type  == NAL_IDR_SLICE) { av_log(h,
> > AV_LOG_WARNING, "Broken frame packetizing\n");
> > -                ff_h264_field_end(h, h->slice_ctx, 1);
> > +                ret = ff_h264_field_end(h, h->slice_ctx, 1);
> >                  h->current_slice = 0;
> >                  ff_thread_report_progress(&h->cur_pic_ptr->tf,
> > INT_MAX, 0); ff_thread_report_progress(&h->cur_pic_ptr->tf, INT_MAX,
> > 1); h->cur_pic_ptr = NULL;
> > +                if (ret < 0)
> > +                    return ret;
> >              } else
> >                  return AVERROR_INVALIDDATA;
> >          }
> > diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> > index 7f79189..d8da18d 100644
> > --- a/libavcodec/hevc.c
> > +++ b/libavcodec/hevc.c
> > @@ -2896,9 +2896,12 @@ static int hevc_decode_frame(AVCodecContext
> > *avctx, void *data, int *got_output, return ret;
> >  
> >      if (avctx->hwaccel) {
> > -        if (s->ref && avctx->hwaccel->end_frame(avctx) < 0)
> > +        if (s->ref && (ret = avctx->hwaccel->end_frame(avctx)) < 0) {
> >              av_log(avctx, AV_LOG_ERROR,
> >                     "hardware accelerator failed to decode
> > picture\n");
> > +            ff_hevc_unref_frame(s, s->ref, ~0);
> > +            return ret;
> > +        }
> >      } else {
> >          /* verify the SEI checksum */
> >          if (avctx->err_recognition & AV_EF_CRCCHECK && s->is_decoded
> > && diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > index 1250d53..453cd6a 100644
> > --- a/libavcodec/mpeg12dec.c
> > +++ b/libavcodec/mpeg12dec.c
> > @@ -1723,9 +1723,11 @@ static int mpeg_field_start(MpegEncContext *s,
> > const uint8_t *buf, int buf_size) 
> >          if (s->avctx->hwaccel &&
> >              (s->avctx->slice_flags & SLICE_FLAG_ALLOW_FIELD)) {
> > -            if (s->avctx->hwaccel->end_frame(s->avctx) < 0)
> > +            if ((ret = s->avctx->hwaccel->end_frame(s->avctx)) < 0) {
> >                  av_log(avctx, AV_LOG_ERROR,
> >                         "hardware accelerator failed to decode first
> > field\n");
> > +                return ret;
> > +            }
> >          }
> >  
> >          for (i = 0; i < 4; i++) {
> > @@ -2082,9 +2084,12 @@ static int slice_end(AVCodecContext *avctx,
> > AVFrame *pict) return 0;
> >  
> >      if (s->avctx->hwaccel) {
> > -        if (s->avctx->hwaccel->end_frame(s->avctx) < 0)
> > +        int ret = s->avctx->hwaccel->end_frame(s->avctx);
> > +        if (ret < 0) {
> >              av_log(avctx, AV_LOG_ERROR,
> >                     "hardware accelerator failed to decode
> > picture\n");
> > +            return ret;
> > +        }
> >      }
> >  
> >      /* end of slice reached */
> 
> 
> 
> 
> --phil