From patchwork Sun Oct 6 05:01:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 15526 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id C4A7F449BCF for ; Sun, 6 Oct 2019 08:18:52 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 982EE680B0F; Sun, 6 Oct 2019 08:18:52 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3132068075D for ; Sun, 6 Oct 2019 08:18:46 +0300 (EEST) Received: by mail-wm1-f46.google.com with SMTP id y21so9277546wmi.0 for ; Sat, 05 Oct 2019 22:18:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=UF8qzG4SYpE7Nck7Oec8Xyp2L6S87j/PsMy6iXXgprw=; b=VUJeS0an5QfcdvQXOakVDSOxz4nBGlhFKvHfRh9oqKfGeeqo0VbmUv1k8qfImyQkG9 z1WnYKOSbch6govEg0Rv1zRAI0UHTEhZqoNtfQYlj34HqrqMzOjrRzzZ3gRJWqSPQVym EyrIcfOEXAUGfkx8Ftfvfm4rWCXejPYdESikGJIasFppsG34eanEBCiAmlJdCIuS6ON6 2r7g/a5QOuWIxv0qfDCL/CZ+SKDX9NbR2TxeSnlUwJxAlPiFMYjL24zoAJhlNGKLfFFW rozigDpAhfwxcIgSst+hau2UKQxSfwRpdqV8sFyhwOJbJYUSLgrtTgWsNWn0VskzvtOV PczA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=UF8qzG4SYpE7Nck7Oec8Xyp2L6S87j/PsMy6iXXgprw=; b=PPjqtT+rtUtNsRDkBganr5nq7MbtAIFSzwMUzEac/iG0C3vnecng3w6Xvdy4W7knXo NqZ7l/+GaNeRAXWFd9SvtdosPJtb6f4TvbXrdKR70tHc/4Fp2yeh9FHXMOoWRpzwit/F l3sGDf7EdX9DBVmb/ka6lVG+v920r3Ng9y5NPOlpoJIDUwqq5MwptNoCNcq87fsEXGU1 +z/IdaCG61qZwDLJrt2jchgWwZVKBabDxErbolkuSFgaxx9e3lXCQdwpTAvxa+w6BBXB NcRktIcJJDiY8R2MkailOPUCZ/lC6wLJv3MCeaxPFW5212AN45h9D1wA0zIIuqXwGtMp Cu9w== X-Gm-Message-State: APjAAAWI1Bji8WzPlngKal3jYuPgz76HsOy800tp3+fcImigD5O1pSAW I+Q3osh1DzE++4kj0pIDaT0GVtyX5SU= X-Google-Smtp-Source: APXvYqyq7Na5PfhtxpgJEyUA68ivZLk9ZTkhhLt61+I2ArTwz8Z3Dt6j6exlIkXFFyjBVc9fI4mwfg== X-Received: by 2002:a1c:1dd4:: with SMTP id d203mr16407019wmd.45.1570338627512; Sat, 05 Oct 2019 22:10:27 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08937.dynamic.kabel-deutschland.de. [188.192.137.55]) by smtp.gmail.com with ESMTPSA id s10sm18543014wmf.48.2019.10.05.22.10.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 05 Oct 2019 22:10:27 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sun, 6 Oct 2019 07:01:15 +0200 Message-Id: <20191006050120.26807-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191006050120.26807-1-andreas.rheinhardt@gmail.com> References: <20191006050120.26807-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 05/10] avcodec/flac_parser: Fix off-by-one error 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: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The flac parser uses a fifo to buffer its data. Consequently, when searching for sync codes of flac packets, one needs to take care of the possibility of wraparound. This is done by using an optimized start code search that works on each of the continuous buffers separately and by explicitly checking whether the last pre-wrap byte and the first post-wrap byte constitute a valid sync code. Moreover, the last MAX_FRAME_HEADER_SIZE - 1 bytes ought not to be searched for (the start of) a sync code because a header that might be found in this region might not be completely available. These bytes ought to be searched lateron when more data is available or when flushing. Unfortunately there was an off-by-one error in the calculation of the length to search of the post-wrap buffer: It was too large, because the calculation was based on the amount of bytes available in the fifo from the last pre-wrap byte onwards. This meant that a header might be parsed twice (once prematurely and once regularly when more data is available); it could also mean that an invalid header will be treated as valid (namely if the length of said invalid header is MAX_FRAME_HEADER_SIZE and the invalid byte that will be treated as the last byte of this potential header happens to be the right CRC-8). Should a header be parsed twice, the second instance will be the best child of the first instance; the first instance's score will be FLAC_HEADER_BASE_SCORE - FLAC_HEADER_CHANGED_PENALTY ( = 3) higher than the second instance's score. So the frame belonging to the first instance will be output and it will be done as a zero length frame (the difference of the header's offset and the child's offset). This has serious consequences when flushing, as returning a zero length buffer signals to the caller that no more data will be output; consequently the last frames not yet output will be dropped. Furthermore, a "sample/frame number mismatch in adjacent frames" warning got output when returning the zero-length frame belonging to the first header, because the child's sample/frame number of course didn't match the expected sample frame/number given its parent. filter/hdcd-mix.flac from the FATE-suite was affected by this (the last frame was omitted) which is the reason why several FATE-tests needed to be updated. Fixes ticket #5937. Signed-off-by: Andreas Rheinhardt --- libavcodec/flac_parser.c | 4 ++-- tests/fate/filter-audio.mak | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c index 2658d3c4dc..197f234e61 100644 --- a/libavcodec/flac_parser.c +++ b/libavcodec/flac_parser.c @@ -244,9 +244,9 @@ static int find_new_headers(FLACParseContext *fpc, int search_start) uint8_t wrap[2]; wrap[0] = buf[read_len - 1]; - read_len = search_end - search_start + 1; - /* search_start + 1 is the post-wrap offset in the fifo. */ + read_len = search_end - (search_start + 1) + 1; + buf = flac_fifo_read(fpc, search_start + 1, &read_len); wrap[1] = buf[0]; diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak index f1db7b9f02..fed2644ccf 100644 --- a/tests/fate/filter-audio.mak +++ b/tests/fate/filter-audio.mak @@ -312,47 +312,47 @@ FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM fate-filter-hdcd-mix: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-mix: CMD = md5 -i $(SRC) -af hdcd -f s24le fate-filter-hdcd-mix: CMP = oneline -fate-filter-hdcd-mix: REF = e7079913e90c124460cdbc712df5b84c +fate-filter-hdcd-mix: REF = 77443573e0bd3532de52a8bc0e825da7 # output will be different because of the gain mismatch in the second and third parts FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-mix-psoff fate-filter-hdcd-mix-psoff: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-mix-psoff: CMD = md5 -i $(SRC) -af hdcd=process_stereo=false -f s24le fate-filter-hdcd-mix-psoff: CMP = oneline -fate-filter-hdcd-mix-psoff: REF = bd0e81fe17696b825ee3515ab928e6bb +fate-filter-hdcd-mix-psoff: REF = 89e57885917a436b30855db4d478cefb # test the different analyze modes FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-pe fate-filter-hdcd-analyze-pe: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-pe: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=pe -f s24le fate-filter-hdcd-analyze-pe: CMP = oneline -fate-filter-hdcd-analyze-pe: REF = bb83e97bbd0064b9b1c0ef2f2c8f0c77 +fate-filter-hdcd-analyze-pe: REF = 2d839d8a1cf73b10a566ce3d4cfaa79e FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-lle fate-filter-hdcd-analyze-lle: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-lle: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=lle -f s24le fate-filter-hdcd-analyze-lle: CMP = oneline -fate-filter-hdcd-analyze-lle: REF = 121cc4a681aa0caef5c664fece7a3ddc +fate-filter-hdcd-analyze-lle: REF = b4b185332b7025c191062f49a2c015f1 FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-cdt fate-filter-hdcd-analyze-cdt: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-cdt: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=cdt -f s24le fate-filter-hdcd-analyze-cdt: CMP = oneline -fate-filter-hdcd-analyze-cdt: REF = 12136e6a00dd532994f6edcc347af1d4 +fate-filter-hdcd-analyze-cdt: REF = afa6577675c63e87da3edbd442b7b6e2 FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-tgm fate-filter-hdcd-analyze-tgm: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-tgm: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=tgm -f s24le fate-filter-hdcd-analyze-tgm: CMP = oneline -fate-filter-hdcd-analyze-tgm: REF = a3c39f62e9b9b42c9c440d0045d5fb2f +fate-filter-hdcd-analyze-tgm: REF = 285f0fd2249b4903cd5e1ad5ce004219 # the two additional analyze modes from libhdcd FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-ltgm fate-filter-hdcd-analyze-ltgm: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-ltgm: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=lle:process_stereo=false -f s24le fate-filter-hdcd-analyze-ltgm: CMP = oneline -fate-filter-hdcd-analyze-ltgm: REF = 76ffd86b762b5a93332039f27e4c0c0e +fate-filter-hdcd-analyze-ltgm: REF = 404dc2301ea97e9f96c3d6d2ebcfeaa5 FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-analyze-pel fate-filter-hdcd-analyze-pel: SRC = $(TARGET_SAMPLES)/filter/hdcd-mix.flac fate-filter-hdcd-analyze-pel: CMD = md5 -i $(SRC) -af hdcd=analyze_mode=pe:force_pe=true -f s24le fate-filter-hdcd-analyze-pel: CMP = oneline -fate-filter-hdcd-analyze-pel: REF = 8156c5a3658d789ab46447d62151f5e9 +fate-filter-hdcd-analyze-pel: REF = 9342983208ec1a7f2b3e332ac4dcb723 FATE_AFILTER_SAMPLES-$(call FILTERDEMDECENCMUX, HDCD, FLAC, FLAC, PCM_S24LE, PCM_S24LE) += fate-filter-hdcd-false-positive fate-filter-hdcd-false-positive: SRC = $(TARGET_SAMPLES)/filter/hdcd-false-positive.flac