From patchwork Tue Feb 21 18:48:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aman Karmani X-Patchwork-Id: 2629 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.65.149 with SMTP id x21csp526514vsf; Tue, 21 Feb 2017 10:55:02 -0800 (PST) X-Received: by 10.223.153.55 with SMTP id x52mr18793741wrb.196.1487703301939; Tue, 21 Feb 2017 10:55:01 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id u15si17449129wmf.119.2017.02.21.10.55.01; Tue, 21 Feb 2017 10:55:01 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@tmm1-net.20150623.gappssmtp.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 31CA16882D0; Tue, 21 Feb 2017 20:54:50 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-pg0-f50.google.com (mail-pg0-f50.google.com [74.125.83.50]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5E9C46882A3 for ; Tue, 21 Feb 2017 20:54:43 +0200 (EET) Received: by mail-pg0-f50.google.com with SMTP id b129so39670661pgc.2 for ; Tue, 21 Feb 2017 10:54:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tmm1-net.20150623.gappssmtp.com; s=20150623; h=sender:from:to:cc:subject:date:message-id; bh=PirMQQaQX62ZnviuBMScVlZY9ocD1FLa41Ze0k0O2VQ=; b=HOw0Ok4NfbOtTIn2vokGCJB/q3XxqdAkTgwrL3rhBuFM8IRD79a0Kx4BkZMKYDOk+B 66aTy0yVfBhAzuWHg0mRR/CyIQeG9pNYrLpuHVFZbC2NGjcbCYxWzONi0SlEAqLIXWh5 v3cRArzvnEFFvDuV0ZKm56neEq26DAyzOcHGaQTrbG3GTEb0tw2GZ85l4wqzEUcII9b9 Q0UwFh3WP2bIVAFX1OEPjcx7/KdEI8Dl2luBjxCCQyS8LFFG3dod4UBY85PFLccISaKn gFn7AUVP5HQPy/jSeHE1PN4C5U4nFb54Tw9GYVpAFU2hwGoXiIDQDR4ynpZGM7zDFOBa PHXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=PirMQQaQX62ZnviuBMScVlZY9ocD1FLa41Ze0k0O2VQ=; b=KkeVublwihYvkNi9/nqygn1LYTyzaLEk9tuf9sbGLllMAnrFayoGC4bXleCzDtY8iO ssy7BslMJf3lImrEre1uH4X2jaUUuIpz6ZxXX1CrjHnsJSOlpgzJireWkDn6AA691o22 RqaelgxENexm4Mdy6Aym6mDAOAVZrqdxio/vR0p5449YSOuIpFr5La0Vej7fMQDexEcK AAQ2H1PGnX4ZoX43k5ADMc7rnFQWmczzBnYEOom79bCtu3LZuF2P7GZ1lteIs2B7N1ZT fwgZYMLKjruH3mY8V6JA/xAnRw0McIFkCEgxKR9TfCVep2YXz4iKIEmjJH9gHMN3nTuA Ka6w== X-Gm-Message-State: AMke39l56MD1pyh+X4qyKQW5+NxlM+J98SPz/tDxzdZWlQmdizZhEQZsfCGlMFZnfhk9OA== X-Received: by 10.98.78.66 with SMTP id c63mr34935747pfb.138.1487702924825; Tue, 21 Feb 2017 10:48:44 -0800 (PST) Received: from tmm1-macbook.local.net (c-73-252-174-83.hsd1.ca.comcast.net. [73.252.174.83]) by smtp.gmail.com with ESMTPSA id w16sm42607460pgc.15.2017.02.21.10.48.42 (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 21 Feb 2017 10:48:43 -0800 (PST) From: Aman Gupta To: ffmpeg-devel@ffmpeg.org Date: Tue, 21 Feb 2017 10:48:37 -0800 Message-Id: <20170221184837.46049-1-ffmpeg@tmm1.net> X-Mailer: git-send-email 2.11.0 (Apple Git-80) Subject: [FFmpeg-devel] [PATCH v2] avcodec/h264, videotoolbox: fix crash after VT decoder fails X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Aman Gupta MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" From: Aman Gupta 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]); } 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; + + 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;