From b6eaa3928e198554a3934dd5ad6eac4d16f27df2 Mon Sep 17 00:00:00 2001 From: Aman Gupta <aman@tmm1.net> Date: Tue, 21 Feb 2017 10:48:37 -0800 Subject: [PATCH] avcodec/h264, videotoolbox: fix crash after VT decoder fails 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. Signed-off-by: wm4 <nfxjfg@googlemail.com> --- libavcodec/h264_refs.c | 3 +-- libavcodec/h264dec.c | 7 ++++++- libavcodec/version.h | 2 +- libavcodec/videotoolbox.c | 2 -- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c index 97bf588b512..ad296753c3b 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]); } for (i = 0; i < h->short_ref_count; i++) { diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 41c0964392a..35598ea7cd7 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_EXTERNAL; + + ret = av_frame_ref(dst, src); if (ret < 0) return ret; diff --git a/libavcodec/version.h b/libavcodec/version.h index 7647ad2e189..cd9ed87225f 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 57 #define LIBAVCODEC_VERSION_MINOR 82 -#define LIBAVCODEC_VERSION_MICRO 100 +#define LIBAVCODEC_VERSION_MICRO 101 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index 9199b020556..67adad53ed8 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -352,8 +352,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; -- GitLab