From patchwork Thu Jul 20 20:54:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Thompson X-Patchwork-Id: 4403 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.1.76 with SMTP id 73csp90430vsb; Thu, 20 Jul 2017 14:01:17 -0700 (PDT) X-Received: by 10.28.58.80 with SMTP id h77mr3322951wma.13.1500584477487; Thu, 20 Jul 2017 14:01:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1500584477; cv=none; d=google.com; s=arc-20160816; b=vqF2nsmedGL9EEa0f7x17COvQQ++S208reUY11lfj+O662DB+pF3sdqFS3KjSua6CF pOVcPYMIvziBrx57VvIS/YRwCmBApyqYFynExJiAmJOVgp1zb6kjmlwfuMCi8z6jyf+3 dZlPxQQWWYnbMfPvpBXBiL9gv/catff/ptZizNHEwcPVR5o7TIrYBUE1SuP/1uiKloe3 smBlEjVN82aY+eybDFEwz12khPPAz9hJfrYtImOC+jozomVKT8ImqnqYl6S8L0L6jqja 1lJaEs4C5nznTbkfroyTdO28vsG+aFBns7Xc+wGnXOeorNef3Edq3r2eZCX6UICAacr4 UA0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:reply-to:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:dkim-signature :delivered-to:arc-authentication-results; bh=8MoYZWiclqLDqLFXCeRc1HLCoIB9AaSYK6x7kQHUdg0=; b=R8e17FWmxLwpJonF8A9AnOJjqzg1kJ3yzEpCX3TI45HJJ2OdJqxIbrH27FaN59KIRq hJ6iV4fVqTBTz4uczhtbwPbGg425BwaGCXVdBgtfstJWnwrxn+vlfh3+ZEMJBCjHzAwX 5vUu1kp2JRucFKb+rxw8YeI/OOdSX4E+AItkaUECXZyPy2ub57es6wwO9FTAihGw7Zqe dR3LUbqcOOb2d0OeiblbyEl+yd6qki4NWMJqVNQQN++BKEvz539hJTos2gwyie6d+EHz GfZ9e60WLsz7csB5BehjOYyCotqyO/FHdApa3XGmKtqAee9uh7001bczy9qKvaWg/4OS WpmA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@jkqxz-net.20150623.gappssmtp.com header.b=i776UUmL; 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 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id r11si5957175wrr.120.2017.07.20.14.01.16; Thu, 20 Jul 2017 14:01:17 -0700 (PDT) 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=@jkqxz-net.20150623.gappssmtp.com header.b=i776UUmL; 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 C06A3689B23; Fri, 21 Jul 2017 00:01:05 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr0-f181.google.com (mail-wr0-f181.google.com [209.85.128.181]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 59CF0689AEA for ; Fri, 21 Jul 2017 00:00:59 +0300 (EEST) Received: by mail-wr0-f181.google.com with SMTP id v105so45164930wrb.0 for ; Thu, 20 Jul 2017 14:01:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=gb/g5Uly0ByWqJ1i/JnjuHPoRjmWt4XMfNOq3XGPEtU=; b=i776UUmLRpZSODI6qkTzRF2q8wG7efA9+DUK5zERfhqun9D2iKfWTJWqyzG0QuGLZN xGsjRNZmlHpFoNO8jv2HfjzhDKjqrsGQVMqBtP/DZ5IkQSxekIG7IER49M6yD9Lh0d0X Nw22SaPxrWRBgm5wBm8pqi8FNTnAppuQxBnRz32egniXS9bZuiYyIxGXGI0KeY+5I+DA Qd5aN+fb1CtNMLLht1L/ldfUWg8fw+n1LNAKHO8wpJMo6i4Yt3RY3Z1ZnOuAko1gSUPJ S3kAdXh471oFbJtXbQMgvWiF5mk/RYx5xnJflH2yWvYVPW8u7z7d9wzCMxtUC32oC5ZS i2jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gb/g5Uly0ByWqJ1i/JnjuHPoRjmWt4XMfNOq3XGPEtU=; b=UkF/mGI4U+xbYcaaDUS3rD9rq4inK6RKCtE0dcDoIFIfIN9FDsr+q/k2t6sgosKMcT om9OzZIrIElPWoOPLZQwcw38fp0K/nE/cH4RRDIDWiU/4HMnpD5ZD7pGIa07imknHcnP UnQ+srPTSTxaOlQx3TzPhaNryavEQmKeyrAk9C+iLS5rWg+Tk9p23Vew0uX56/WlKmEB XJHZIzv9qiiqg7SUlGaIFM+QcqpiMSTHQJWkYR8ssIYowlMjuq9hidxYD/Og77mCDFb+ 9fYSlVW2YP/f9DmKSRRAPF+91FNha3uzAgya/lsLXLQ+uwvwR6YXHlhooimdZM5rsTG1 YQsQ== X-Gm-Message-State: AIVw110Nj5LB2zBIt82g9dfyIxRI5fZ/Z4lcLx9g+Nn3PJouBQvF7O85 LnxlO+cCICGYIM2Yzh8= X-Received: by 10.223.155.205 with SMTP id e13mr6969129wrc.71.1500584046528; Thu, 20 Jul 2017 13:54:06 -0700 (PDT) Received: from [192.168.0.8] (cpc91242-cmbg18-2-0-cust650.5-4.cable.virginm.net. [82.8.130.139]) by smtp.gmail.com with ESMTPSA id y100sm3312083wmh.7.2017.07.20.13.54.05 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jul 2017 13:54:05 -0700 (PDT) To: FFmpeg development discussions and patches References: <20170718230105.5764A174178@albiero.videolan.org> <20170720003348.GB3693@nb4> From: Mark Thompson Message-ID: Date: Thu, 20 Jul 2017 21:54:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170720003348.GB3693@nb4> Content-Language: en-US Subject: [FFmpeg-devel] [PATCH] pixdesc: Explicitly handle invalid arguments to av_find_best_pix_fmt_of_2() 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" --- On 20/07/17 01:33, Michael Niedermayer wrote: > Hi > > On Tue, Jul 18, 2017 at 11:01:01PM +0000, Mark Thompson wrote: >> ffmpeg | branch: master | Mark Thompson | Thu Jul 6 22:50:35 2017 +0100| [8a442d7a8a687a469ca502a18a0c68f5302b15e0] | committer: Mark Thompson >> >> pixdesc: Improve scoring for opaque/unknown pixel formats >> >> Hardware pixel formats do not tell you anything about their actual >> contents, but should still score higher than formats with completely >> unknown properties, which in turn should score higher than invalid >> formats. >> >> Do not return an AVERROR code as a score. >> >> Fixes a hang in libavfilter where format negotiation gets stuck in a >> loop because AV_PIX_FMT_NONE scores more highly than all other >> possibilities. >> >>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8a442d7a8a687a469ca502a18a0c68f5302b15e0 >> --- >> >> libavutil/pixdesc.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) > > This still breaks > valgrind ./ffmpeg_g -i ~/videos/matrixbench_mpeg2.mpg -y -t 4 -acodec libmp3lame http://127.0.0.1:8192/feed1.ffm > [mpeg @ 0x24823f20] start time for stream 0 is not set in estimate_timings_from_pts > Input #0, mpeg, from '/home/michael/videos/matrixbench_mpeg2.mpg': > Duration: 00:03:07.66, start: 0.220000, bitrate: 5633 kb/s > Stream #0:0[0x1bf]: Data: dvd_nav_packet > Stream #0:1[0x1e0]: Video: mpeg2video (Main), yuv420p(tv, bt470bg/bt470m/bt470m, bottom first), 720x576 [SAR 16:15 DAR 4:3], 25 fps, 25 tbr, 90k tbn, 50 tbc > Stream #0:2[0x1c0]: Audio: mp2, 48000 Hz, stereo, s16p, 384 kb/s > ==17852== Invalid read of size 1 > ==17852== at 0x104871B: av_find_best_pix_fmt_of_2 (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BD23A: choose_pixel_fmt (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BBB2C: open_output_file (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BC696: ffmpeg_parse_options (in ffmpeg/ffmpeg_g) > ==17852== by 0x4A8A7D: main (in ffmpeg/ffmpeg_g) > ==17852== Address 0x9 is not stack'd, malloc'd or (recently) free'd > ==17852== > ==17852== > ==17852== Process terminating with default action of signal 11 (SIGSEGV) > ==17852== Access not within mapped region at address 0x9 > ==17852== at 0x104871B: av_find_best_pix_fmt_of_2 (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BD23A: choose_pixel_fmt (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BBB2C: open_output_file (in ffmpeg/ffmpeg_g) > ==17852== by 0x4BC696: ffmpeg_parse_options (in ffmpeg/ffmpeg_g) > ==17852== by 0x4A8A7D: main (in ffmpeg/ffmpeg_g) > ==17852== If you believe this happened as a result of a stack > ==17852== overflow in your program's main thread (unlikely but > ==17852== possible), you can try to increase the size of the > ==17852== main thread stack using the --main-stacksize= flag. > ==17852== The main thread stack size used in this run was 8388608. > > > the receiver side of the connection can be setup with > valgrind ./ffserver_g -f ~/videos/ffserver.conf > > ffserver.conf attached > > [...] > Right - so someone does call find_best() with an invalid source format, which will give the same score to all possibilities and then barf if one of them is invalid. This change makes the invalid format handling more explicit, and fixes your case. Thanks, - Mark libavutil/pixdesc.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index 1983ce9ef5..a9dd11a498 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -2633,21 +2633,27 @@ enum AVPixelFormat av_find_best_pix_fmt_of_2(enum AVPixelFormat dst_pix_fmt1, en const AVPixFmtDescriptor *desc2 = av_pix_fmt_desc_get(dst_pix_fmt2); int score1, score2; - loss_mask= loss_ptr?~*loss_ptr:~0; /* use loss mask if provided */ - if(!has_alpha) - loss_mask &= ~FF_LOSS_ALPHA; - - score1 = get_pix_fmt_score(dst_pix_fmt1, src_pix_fmt, &loss1, loss_mask); - score2 = get_pix_fmt_score(dst_pix_fmt2, src_pix_fmt, &loss2, loss_mask); - - if (score1 == score2) { - if(av_get_padded_bits_per_pixel(desc2) != av_get_padded_bits_per_pixel(desc1)) { - dst_pix_fmt = av_get_padded_bits_per_pixel(desc2) < av_get_padded_bits_per_pixel(desc1) ? dst_pix_fmt2 : dst_pix_fmt1; + if (!desc1) + dst_pix_fmt = dst_pix_fmt2; + else if (!desc2) + dst_pix_fmt = dst_pix_fmt1; + else { + loss_mask= loss_ptr?~*loss_ptr:~0; /* use loss mask if provided */ + if(!has_alpha) + loss_mask &= ~FF_LOSS_ALPHA; + + score1 = get_pix_fmt_score(dst_pix_fmt1, src_pix_fmt, &loss1, loss_mask); + score2 = get_pix_fmt_score(dst_pix_fmt2, src_pix_fmt, &loss2, loss_mask); + + if (score1 == score2) { + if(av_get_padded_bits_per_pixel(desc2) != av_get_padded_bits_per_pixel(desc1)) { + dst_pix_fmt = av_get_padded_bits_per_pixel(desc2) < av_get_padded_bits_per_pixel(desc1) ? dst_pix_fmt2 : dst_pix_fmt1; + } else { + dst_pix_fmt = desc2->nb_components < desc1->nb_components ? dst_pix_fmt2 : dst_pix_fmt1; + } } else { - dst_pix_fmt = desc2->nb_components < desc1->nb_components ? dst_pix_fmt2 : dst_pix_fmt1; + dst_pix_fmt = score1 < score2 ? dst_pix_fmt2 : dst_pix_fmt1; } - } else { - dst_pix_fmt = score1 < score2 ? dst_pix_fmt2 : dst_pix_fmt1; } if (loss_ptr)