From patchwork Fri Oct 1 21:08:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 30866 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a6b:6506:0:0:0:0:0 with SMTP id z6csp2088212iob; Fri, 1 Oct 2021 14:09:39 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzCg7ZTQUug5bG3gSyAtq0JiUuT28uICpTSqVjJc7Wn2yXSenSTUkCNqu1AOILp2jtfhKJ9 X-Received: by 2002:a50:da04:: with SMTP id z4mr340472edj.52.1633122579274; Fri, 01 Oct 2021 14:09:39 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id f20si8456450ejk.645.2021.10.01.14.09.39; Fri, 01 Oct 2021 14:09:39 -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=@outlook.com header.s=selector1 header.b=c4HlPumO; arc=fail (body hash mismatch); 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; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=outlook.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5B29168A905; Sat, 2 Oct 2021 00:09:17 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-oln040092068100.outbound.protection.outlook.com [40.92.68.100]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1A17768A8DC for ; Sat, 2 Oct 2021 00:09:15 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OSKHet/I0aHdw5VwJ5dKP+AMFjS/rqGL6Tw1WiVFvqSeuHYj3xbIR9MLPiY6oQpLlHKaICnBHUZuywxCDbHmrqVdHrudhMfr1IzOP7WoUX4MrfRtNlcnU1R8ZCQRIO5j7R1wplZs/8jXAYvHKK/XbMXnfHkWh2OfuuynGZrtLhQ+LNZm2dPdNEYbojw14tOBG7QFH+yo+PtFxFclR1eYui1Mv9uCKwoGj/h5zJoKBD7RwifFqXbQ3DiHdN/HCec78cSlgxfp+nHDjQkh/PiZbqcRnU6Ftw7pn/e9WP0Vh09AEj+bGu2NY3W+gsQ7Tz/cCeDwvJqE98NTaZvXndDFlA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RHvcMj2+jOYcMOG3R9AaH6/cWlFk+SzCNMa1HxOA+gs=; b=J7OTiF8qPxmPw7nhDIcn6zJBjs2KupRJ1P6DLe6bybVOH90zXZHPSfoSoiIkpiBUBOKW54BHu/jQzfIviqCn9BWTIFE7dzjMdNYmqu/IgaV6xjsqVuA1sjI6+0wReGsqBxmKad5LnwvO6eOpUC0ov+LkxWdFJDHhurtuo3zlpMGTJpORZ1uaDSJzRsb9cxSHRDsV+z2uRyebix9fmyIrcnVKYOttf3e6GxgK9GeFKHkC8Lakk0qKr+9BeBOgyXYwaT5kDt+m9e5FPB2vycWsECHPyjOWsGAyqw6zGqDrGqELg4GL6ybOz4QKIzle1Kav6ooMD/FSGzrdaBNspDYW+Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=RHvcMj2+jOYcMOG3R9AaH6/cWlFk+SzCNMa1HxOA+gs=; b=c4HlPumOe9SYRmgHSJQ+j9X7SiJP8lbJ5CE5hyP4N9joqysrhgclVzc0K4Kef7XRKRgMH7++uWkczSsNLgPM6vATIPJS0LBLZCwTIOcIApTAEqCkNTf6mGQz1pm/2cfB6iPxlA8bZOe3I18TP/1xFF8r4PkZOaeMt7RUZKRVn6fyYmHrTqehZpHGDY04jY1muCWC3HZJeMzqsiS5rtaTesFtJFYs2Thd/908/wIUduzESkoGbLrz60+AfIdIDUbJsPzdrMr5kCkzX+Ot22gG+6vVSrzp53oUk3cshoHqfRqsSnG92AJ3a/QK4gDG2Jp7vIE82OvdeHTtJE1188GgNg== Received: from AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) by AS8PR03MB6952.eurprd03.prod.outlook.com (2603:10a6:20b:29a::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.17; Fri, 1 Oct 2021 21:09:14 +0000 Received: from AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::700f:d70b:3bb8:4d51]) by AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::700f:d70b:3bb8:4d51%7]) with mapi id 15.20.4566.019; Fri, 1 Oct 2021 21:09:14 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 1 Oct 2021 23:08:23 +0200 Message-ID: X-Mailer: git-send-email 2.30.2 In-Reply-To: References: X-TMN: [TiZG+sTCbZwI0QrIHD5PJstZRgsVM8zr] X-ClientProxiedBy: FR3P281CA0071.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:4b::7) To AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) X-Microsoft-Original-Message-ID: <20211001210837.2879205-5-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from sblaptop.fritz.box (188.192.142.38) by FR3P281CA0071.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:4b::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4587.8 via Frontend Transport; Fri, 1 Oct 2021 21:09:14 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 109a31c0-ffcd-48f9-c24a-08d9851fb927 X-MS-Exchange-SLBlob-MailProps: S/btQ8cKWiTgZdhvd8QKnLjC/Dfe6SG91ARVHwq3PICswtUPbyB7GzYsg4grZguuQDgo0UNdJesCauIZ0FQrRyAeb0MjEaOy4J4yf/soqlShbLGBhxngWfTZzkugQhigzW1auQ3fE5LoXCCdzaVRK3BHYlq8GSpFFmexxvL53A5Fj08l/2kKpR+th+j4OUG18ZmaxprwOV79zlb+6003rrQ2IRM3r80tXqJ6QIxaD06zD/6OstlHW0eUh9bXkIbGJyW4V7hWhgQv+8JZvwP+UmMFc/kN5idUC2bEbXoboFA7lHoVZHL2pKCBmzU5tpfS4gNg+aIHPb2XE6klRoCguEZnHYTuQICRQ7CvhmhRYvJpGq+GG5v38+fftMBN4x2Os2+kHaIDxEhFanCX/tPDQ528a8e9AUDOYAFNFteQJu8VAxwtKaaM1wgyPfMKvK5ewhxvHdva88Ji7bq9mmzQmC4RT+ZyBiXsvweQaCdp2OPko8UqmOpnOd0wQQW0FV3+yTpmuTu4b1ojSGPPGJ6sVdWag3/qXP6So7M9nkmdalJ+WXRkNaXfEh9MtEDERjCaPZ3kIzveoPU+3C4GTRH58jrVSzUH4ofno+6rIbYhL9nvQ1Bg5tdAfd6FxRaDqTAd9emTZeaB+13MHBsanPsJXyinHbUT7qI6Fdlv4IUHCQeb8CvXrcPjf503GG6d2efJTHdQTp2E3iWRKeCPxV4zvWSywxfDhxgkHQnbn/JOlot9yxnG0NWaIirJIBCWjMoRBU4i5ahdEZA= X-MS-TrafficTypeDiagnostic: AS8PR03MB6952: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 41lxYdIotfywqXN2Ty9pxBNAokwkPBkwVxsZxa1PKPT5P9yM9irgx64q19kj8OTy8DXPBF6k1ouY9HCojorwWq+xl3QuRxd/q28F1VP1UAGR9VU1pbBRCL/mElITA1t7InLKszzU3+qR/njMldtFKLBg/RI+IbVlXyW6ONzhTv9h7uvNNx+ODJlO7MsQEWXJRBsKCPX5CSpKWMLuo5/y7OXaTknXe8T5eIIfpGluw7YoTF4Pjch3rqcSYQPZq6fQWDtFInukZnPBS8JTRJQl/dgxlUQxMcFfx0YkSU90FRjGrUq+4b0FApeLUu9nU6HpBqs8mELHHqiUYcLaIjGBQ14Jt5rwysDt+sQCNb2ENphFnc2Rr5hBlY4/bC7Y8yq4AMCDyt4Qo1fMgYM2cM8voURniIWtsNmnP8gVXbQjO9gW0bLTbM+lRTWbb9sAt6cQ X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: 7o7aNfJILcRmjZVPfAx2fFQP1JTYNyyHKujCmY02B8pKNAGDg3UT1BQ+ULARRFN2lkdMeBSwvC3Nn0GJ9lO2Yfei96rzYxXv5HUOXrNTImB7vmiyn5wy7dh+0GTGqkvZ3asPSS5GV0LsZ8Osn6EyLg== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 109a31c0-ffcd-48f9-c24a-08d9851fb927 X-MS-Exchange-CrossTenant-AuthSource: AM7PR03MB6660.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Oct 2021 21:09:14.8338 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR03MB6952 Subject: [FFmpeg-devel] [PATCH 06/20] avformat/sccdec: Don't use uninitialized data, fix crash, simplify logic X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: mAqXpdA4XVHQ Up until now, the scc demuxer not only read the line that it intends to process, but also the next line, in order to be able to calculate the duration of the current line. This approach leads to unnecessary complexity and also to bugs: For the last line, the timing of the next subtitle is not only logically indeterminate, but also uninitialized and the same applies to the duration of the last packet derived from it.* Worse yet, in case of e.g. an empty file, it is not only the duration that is uninitialized, but the whole timing as well as the line buffer itself.** The latter is used in av_strtok(), which could lead to crashes. Furthermore, the current code always outputs at least one packet, even for empty files. This commit fixes all of this: It stops using two lines at a time; instead only the current line is dealt with and in case there is a packet after that, the duration of the last packet is fixed up after having already parsed it; consequently the duration of the last packet is left in its default state (meaning "unknown/up until the next subtitle"). If no further line could be read, processing is stopped; in particular, no packet is output for an empty file. *: Due to stack reuse it seems to be zero quite often; for the same reason Valgrind does not report any errors for a normal input file. **: While ff_subtitles_read_line() claims to always zero-terminate the buffer like snprintf(), it doesn't do so if it didn't read anything. And even if it did, it would not necessarily help here: The current code jumps over 12 bytes that it deems to have read even when it hasn't. Signed-off-by: Andreas Rheinhardt --- libavformat/sccdec.c | 54 ++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 39 deletions(-) diff --git a/libavformat/sccdec.c b/libavformat/sccdec.c index 592609eb88..1861a99aad 100644 --- a/libavformat/sccdec.c +++ b/libavformat/sccdec.c @@ -63,8 +63,7 @@ static int scc_read_header(AVFormatContext *s) { SCCContext *scc = s->priv_data; AVStream *st = avformat_new_stream(s, NULL); - char line2[4096], line[4096]; - int64_t pos, ts, next_ts = AV_NOPTS_VALUE; + AVPacket *sub = NULL; ptrdiff_t len; uint8_t out[4096]; FFTextReader tr; @@ -77,47 +76,26 @@ static int scc_read_header(AVFormatContext *s) st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE; st->codecpar->codec_id = AV_CODEC_ID_EIA_608; - while (!ff_text_eof(&tr) || next_ts == AV_NOPTS_VALUE || line2[0]) { + while (1) { char *saveptr = NULL, *lline; int hh, mm, ss, fs, i; - AVPacket *sub; + char line[4096]; + int64_t pos, ts; - if (next_ts == AV_NOPTS_VALUE) { - while (!ff_text_eof(&tr)) { - len = ff_subtitles_read_line(&tr, line, sizeof(line)); - if (len <= 13) - continue; + len = ff_subtitles_read_line(&tr, line, sizeof(line)); + if (len <= 13) { + if (ff_text_eof(&tr)) + break; + continue; + } if (!strncmp(line, "Scenarist_SCC V1.0", 18)) continue; - if (av_sscanf(line, "%d:%d:%d%*[:;]%d", &hh, &mm, &ss, &fs) == 4) - break; - } - - ts = (hh * 3600LL + mm * 60LL + ss) * 1000LL + fs * 33LL; - - while (!ff_text_eof(&tr)) { - len = ff_subtitles_read_line(&tr, line2, sizeof(line2)); - if (len <= 13) - continue; - - if (av_sscanf(line2, "%d:%d:%d%*[:;]%d", &hh, &mm, &ss, &fs) == 4) - break; - } - } else { - memmove(line, line2, sizeof(line)); - line2[0] = 0; - - while (!ff_text_eof(&tr)) { - len = ff_subtitles_read_line(&tr, line2, sizeof(line2)); - if (len <= 13) - continue; - - if (av_sscanf(line2, "%d:%d:%d%*[:;]%d", &hh, &mm, &ss, &fs) == 4) - break; - } - } + if (av_sscanf(line, "%d:%d:%d%*[:;]%d", &hh, &mm, &ss, &fs) != 4) + continue; - next_ts = (hh * 3600LL + mm * 60LL + ss) * 1000LL + fs * 33LL; + ts = (hh * 3600LL + mm * 60LL + ss) * 1000LL + fs * 33LL; + if (sub) + sub->duration = ts - sub->pts; pos = ff_text_pos(&tr); lline = line; @@ -168,8 +146,6 @@ static int scc_read_header(AVFormatContext *s) sub->pos = pos; sub->pts = ts; - sub->duration = next_ts - ts; - ts = next_ts; } ff_subtitles_queue_finalize(s, &scc->q);