On Tue, 21 Feb 2017 10:48:37 -0800 Aman Gupta <ffm...@tmm1.net> wrote:
> From: Aman Gupta <a...@tmm1.net> > > The way videotoolbox hooks in as a hwaccel is pretty hacky. The VT decode > API is not invoked until end_frame(), so alloc_frame() returns a dummy > frame with a 1-byte buffer. When end_frame() is eventually called, the > dummy buffer is replaced with the actual decoded data from > VTDecompressionSessionDecodeFrame(). > > When the VT decoder fails, the frame returned to the h264 decoder from > alloc_frame() remains invalid and should not be used. Before > 9747219958060d8c4f697df62e7f172c2a77e6c7, it was accidentally being > returned all the way up to the API user. After that commit, the dummy > frame was unref'd so the user received an error. > > However, since that commit, VT hwaccel failures started causing random > segfaults in the h264 decoder. This happened more often on iOS where the > VT implementation is more likely to throw errors on bitstream anomolies. > A recent report of this issue can be see in > http://ffmpeg.org/pipermail/libav-user/2016-November/009831.html > > The issue here is that the dummy frame is still referenced internally by the > h264 decoder, as part of the reflist and cur_pic_ptr. Deallocating the > frame causes assertions like this one to trip later on during decoding: > > Assertion h->cur_pic_ptr->f->buf[0] failed at > src/libavcodec/h264_slice.c:1340 > > With this commit, we leave the dummy 1-byte frame intact, but avoid returning > it > to the user. > > This reverts commit 9747219958060d8c4f697df62e7f172c2a77e6c7. > --- > libavcodec/h264_refs.c | 3 +-- > libavcodec/h264dec.c | 7 ++++++- > libavcodec/videotoolbox.c | 2 -- > 3 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c > index 97bf588b51..ad296753c3 100644 > --- a/libavcodec/h264_refs.c > +++ b/libavcodec/h264_refs.c > @@ -571,8 +571,7 @@ void ff_h264_remove_all_refs(H264Context *h) > > if (h->short_ref_count && !h->last_pic_for_ec.f->data[0]) { > ff_h264_unref_picture(h, &h->last_pic_for_ec); > - if (h->short_ref[0]->f->buf[0]) > - ff_h264_ref_picture(h, &h->last_pic_for_ec, h->short_ref[0]); > + ff_h264_ref_picture(h, &h->last_pic_for_ec, h->short_ref[0]); In case others don't realize, this just undoes an earlier VT-specific hack, and should not affect software decoding in any way. > } > > for (i = 0; i < h->short_ref_count; i++) { > diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c > index 41c0964392..a0ae632fed 100644 > --- a/libavcodec/h264dec.c > +++ b/libavcodec/h264dec.c > @@ -850,7 +850,12 @@ static int output_frame(H264Context *h, AVFrame *dst, > H264Picture *srcp) > AVFrame *src = srcp->f; > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(src->format); > int i; > - int ret = av_frame_ref(dst, src); > + int ret; > + > + if (src->format == AV_PIX_FMT_VIDEOTOOLBOX && src->buf[0]->size == 1) > + return AVERROR_INVALIDDATA; Kind of un-nice, but I guess we can live with it for now? And it's probably much better than trying to make the h264 code deal with an unset buffer (which was what I tried earlier, but which is imperfect). I'd probably return AVERROR_EXTERNAL (or maybe we should introduce an error code that signals that a hw decoder failed). > + > + ret = av_frame_ref(dst, src); > if (ret < 0) > return ret; > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > index 1288aa5087..d931dbd5f9 100644 > --- a/libavcodec/videotoolbox.c > +++ b/libavcodec/videotoolbox.c > @@ -351,8 +351,6 @@ static int videotoolbox_common_end_frame(AVCodecContext > *avctx, AVFrame *frame) > AVVideotoolboxContext *videotoolbox = avctx->hwaccel_context; > VTContext *vtctx = avctx->internal->hwaccel_priv_data; > > - av_buffer_unref(&frame->buf[0]); > - > if (!videotoolbox->session || !vtctx->bitstream) > return AVERROR_INVALIDDATA; > In theory, ff_videotoolbox_buffer_create() could still leave buf[0] unset if av_buffer_create() fails. (Which is rare, but theoretically possible.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
