On Mon, 13 Feb 2017 18:04:10 -0800 Aman Gupta <ffm...@tmm1.net> wrote:
> From: Aman Gupta <a...@tmm1.net> > > The videotoolbox hwaccel returns a dummy frame with a 1-byte buffer from > alloc_frame(). In end_frame(), this buffer is replaced with the actual > decoded data from VTDecompressionSessionDecodeFrame(). This is hacky, > but works well enough, as long as the VT decoder returns a valid frame on > every end_frame() call. > > In the case of errors, things get more interesting. Before > 9747219958060d8c4f697df62e7f172c2a77e6c7, the dummy 1-byte frame would > accidentally be returned all the way up to the user. After that commit, > the dummy frame was properly 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 root cause here is that between the calls to alloc_frame() and > end_frame(), the h264 decoder will reference the dummy 1-byte frame in > its ref_list. When the end_frame() call fails, the dummy frame is > unref'd but still referenced in the h264 decoder. This eventually causes > a NULL deference and segmentation fault. > > This patch fixes the issue by properly discarding references to the > dummy frame in the H264Context after the frame has been unref'd. > --- > libavcodec/h264_picture.c | 3 +++ > libavcodec/h264_refs.c | 20 ++++++++++++++++++-- > libavcodec/h264dec.h | 1 + > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c > index f634d2a..702ca12 100644 > --- a/libavcodec/h264_picture.c > +++ b/libavcodec/h264_picture.c > @@ -177,6 +177,9 @@ int ff_h264_field_end(H264Context *h, H264SliceContext > *sl, int in_setup) > if (err < 0) > av_log(avctx, AV_LOG_ERROR, > "hardware accelerator failed to decode picture\n"); > + > + if (err < 0 && avctx->hwaccel->pix_fmt == AV_PIX_FMT_VIDEOTOOLBOX) > + ff_h264_remove_cur_pic_ref(h); > } > > #if FF_API_CAP_VDPAU > diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c > index 97bf588..9c77bc7 100644 > --- a/libavcodec/h264_refs.c > +++ b/libavcodec/h264_refs.c > @@ -560,6 +560,23 @@ static H264Picture *remove_long(H264Context *h, int i, > int ref_mask) > return pic; > } > > +void ff_h264_remove_cur_pic_ref(H264Context *h) > +{ > + int j; > + > + if (h->short_ref[0] == h->cur_pic_ptr) { > + remove_short_at_index(h, 0); > + } > + > + if (h->cur_pic_ptr->long_ref) { > + for (j = 0; j < FF_ARRAY_ELEMS(h->long_ref); j++) { > + if (h->long_ref[j] == h->cur_pic_ptr) { > + remove_long(h, j, 0); > + } > + } > + } > +} > + > void ff_h264_remove_all_refs(H264Context *h) > { > int i; > @@ -571,8 +588,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.h b/libavcodec/h264dec.h > index 5f868b7..063afed 100644 > --- a/libavcodec/h264dec.h > +++ b/libavcodec/h264dec.h > @@ -569,6 +569,7 @@ int ff_h264_alloc_tables(H264Context *h); > int ff_h264_decode_ref_pic_list_reordering(H264SliceContext *sl, void > *logctx); > int ff_h264_build_ref_list(H264Context *h, H264SliceContext *sl); > void ff_h264_remove_all_refs(H264Context *h); > +void ff_h264_remove_cur_pic_ref(H264Context *h); > > /** > * Execute the reference picture marking (memory management control > operations). Still a bit hacky, but if it improves the crash situation, why not. I'm wondering if it might be better to just check for a dummy buffer before returning a frame to the user, though. (Instead of strictly removing the dummy buffer on the end_frame hwaccel callback.) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel