From patchwork Wed May 22 10:09:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 13246 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 572E6447E2A for ; Wed, 22 May 2019 13:10:27 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2E1D3689D1C; Wed, 22 May 2019 13:10:27 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 958446804B6 for ; Wed, 22 May 2019 13:10:20 +0300 (EEST) Received: by mail-wr1-f66.google.com with SMTP id w8so1601239wrl.6 for ; Wed, 22 May 2019 03:10:20 -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=sh80q7y4RQcckIb8d08A7wUmmDCK8ycoO5QH+fdfJTg=; b=m3wOhxRfuguNAP4EwIQDAap5WO1RzxAh8NSNOvoBnDZFG312VZck4q6YYJCfI6mnku omtTlEL+ldi3AoOYireyYYsbbpZez9DU9d6hsYWHDxm+4R2Cc9uNtB9Xm5di0D7rxJLX 3pC8lzxBUp1RHxGhgIEKRS87x8oAWQ3vMV/Q/4QZLrHiVW29yQk83yjeslreXATJgrAO jg0qtnAvIDZOI4icfxrsfqCg+IJws2JndxK/vOrkO//V/+x0EESCTyyfX8fGyYItr84a iNayHM9Jr+l6KA0zpD0ZwA2UT5vmxr1cc1YovR/kkRYfCC8Whup3fCTNR9g3QSmSu1fn P3Kg== 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=sh80q7y4RQcckIb8d08A7wUmmDCK8ycoO5QH+fdfJTg=; b=nHGB+Rr9oPcz9097wAREqC9oWZvmEeKXtOfk9mQF1yoATpt7tleN1sBCi5aTJAtnOC Dp5Zvh5IlsLc7uJquF3jC8dwiaL/Jom8BOoNreatcmj5UzFmZxYLbkqXBMPyyKfR+ybF Etebkzmb9SBMoepG7Gt7+UjIkVl7fzC2uaFPT38Vsy+Y1kZHeotm6qsZfutMxCedkQKB uXEx5VEJ+bKQnl3a/q5qjEyqJo7RW2balgH6q4JoCy785ZrFA0MJecjl+lO4eIkpp5SX 3uaHEwiHBAT4Kp/in/ijFCKNRPobKnvizSZ7Pwx6FwPmGwGtrQ8Zv7IYlBzozv024Ayf HtcA== X-Gm-Message-State: APjAAAXN9xXFfZ0buKHlLm4xoi7OioO0eOgFMB07hjknf8n3wmY2g9ix qx49hERTOsmMXOGBhYiLF2xWyG/h X-Google-Smtp-Source: APXvYqw2sLoVunEZEPDLHiJA4nf8kHrb3nSInUOd+5NWLwbwsWGKHEIrbFIlkMkCma+XEDztFuXbjA== X-Received: by 2002:adf:d4c4:: with SMTP id w4mr770411wrk.254.1558519819904; Wed, 22 May 2019 03:10:19 -0700 (PDT) Received: from localhost.localdomain (ipbcc18715.dynamic.kabel-deutschland.de. [188.193.135.21]) by smtp.gmail.com with ESMTPSA id z4sm25334514wru.69.2019.05.22.03.10.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 May 2019 03:10:19 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 22 May 2019 12:09:23 +0200 Message-Id: <20190522100923.47212-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190522010441.44257-10-andreas.rheinhardt@gmail.com> References: <20190522010441.44257-10-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] cbs_mpeg2: Fix parsing of slice headers 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" 1. The extra information in slice headers was parsed incorrectly: In the first reading pass to derive the length of the extra information, one should look at bits n, n + 9, n + 18, ... and check whether they equal one (further extra information) or zero (end of extra information), but instead bits n, n + 8, n + 16, ... were inspected. The second pass of reading (where the length is already known and the bytes between the length-determining bits are copied into a buffer) did not record what was in bits n, n + 9, n + 18, ..., presuming they equal one. And during writing, the bytes in the buffer are interleaved with set bits and written. This means that if the detected length of the extra information was greater than the real length, the output was corrupted. Fortunately no sample is known that made use of this mechanism: The extra information in slices is still marked as reserved in the specifications. cbs_mpeg2 is now ready in case this changes. 2. Furthermore, the buffer is now padded and slightly different, but very similar code for reading resp. writing has been replaced by code used for both. This was made possible by a new macro, the equivalent to cbs_h2645's fixed(). 3. These changes also made it possible to remove the extra_bit_slice element from the MPEG2RawSliceHeader structure. Said element was always zero except when the detected length of the extra information was less than the real length. Signed-off-by: Andreas Rheinhardt --- There was no merge conflict for me for this commit during the rebase in reply to James' request despite there being changes in the immediate vicinity to the changes in this patch. But I don't know if the patch would apply cleanly to everyone else's repo as my repo contained the whole history which might have been used to apply the commit. libavcodec/cbs_mpeg2.c | 18 +++++++----------- libavcodec/cbs_mpeg2.h | 2 -- libavcodec/cbs_mpeg2_syntax_template.c | 16 +++++----------- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index 07213437dc..eea0186c97 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -51,6 +51,13 @@ #define sis(width, name, subs, ...) \ xsi(width, name, current->name, MIN_INT_BITS(width), MAX_INT_BITS(width), subs, __VA_ARGS__) +#define marker_bit() \ + bit(marker_bit, 1) +#define bit(name, value) do { \ + av_unused uint32_t bit = value; \ + xui(1, name, bit, value, value, 0); \ + } while (0) + #define READ #define READWRITE read @@ -72,11 +79,6 @@ var = value; \ } while (0) -#define marker_bit() do { \ - av_unused uint32_t one; \ - CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", NULL, &one, 1, 1)); \ - } while (0) - #define nextbits(width, compare, var) \ (get_bits_left(rw) >= width && \ (var = show_bits(rw, width)) == (compare)) @@ -92,7 +94,6 @@ #undef RWContext #undef xui #undef xsi -#undef marker_bit #undef nextbits #undef infer @@ -113,10 +114,6 @@ range_min, range_max)); \ } while (0) -#define marker_bit() do { \ - CHECK(ff_cbs_write_unsigned(ctx, rw, 1, "marker_bit", NULL, 1, 1, 1)); \ - } while (0) - #define nextbits(width, compare, var) (var) #define infer(name, value) do { \ @@ -135,7 +132,6 @@ #undef RWContext #undef xui #undef xsi -#undef marker_bit #undef nextbits #undef infer diff --git a/libavcodec/cbs_mpeg2.h b/libavcodec/cbs_mpeg2.h index 11f93b9df8..db5ebc56ea 100644 --- a/libavcodec/cbs_mpeg2.h +++ b/libavcodec/cbs_mpeg2.h @@ -194,8 +194,6 @@ typedef struct MPEG2RawSliceHeader { uint8_t slice_picture_id_enable; uint8_t slice_picture_id; - uint8_t extra_bit_slice; - size_t extra_information_length; uint8_t *extra_information; AVBufferRef *extra_information_ref; diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c index d9ef480f39..1f2d2497c3 100644 --- a/libavcodec/cbs_mpeg2_syntax_template.c +++ b/libavcodec/cbs_mpeg2_syntax_template.c @@ -377,31 +377,25 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, uint8_t bit; start = *rw; for (k = 0; nextbits(1, 1, bit); k++) - skip_bits(rw, 8); + skip_bits(rw, 1 + 8); current->extra_information_length = k; if (k > 0) { *rw = start; current->extra_information_ref = - av_buffer_alloc(current->extra_information_length); + av_buffer_allocz(k + AV_INPUT_BUFFER_PADDING_SIZE); if (!current->extra_information_ref) return AVERROR(ENOMEM); current->extra_information = current->extra_information_ref->data; - for (k = 0; k < current->extra_information_length; k++) { - xui(1, extra_bit_slice, bit, 1, 1, 0); - xui(8, extra_information_slice[k], - current->extra_information[k], 0, 255, 1, k); - } } -#else +#endif for (k = 0; k < current->extra_information_length; k++) { - xui(1, extra_bit_slice, 1, 1, 1, 0); + bit(extra_bit_slice, 1); xui(8, extra_information_slice[k], current->extra_information[k], 0, 255, 1, k); } -#endif } } - ui(1, extra_bit_slice); + bit(extra_bit_slice, 0); return 0; }