From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 988764023A for ; Mon, 20 Jun 2022 11:35:43 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DE75D68B5EE; Mon, 20 Jun 2022 14:35:40 +0300 (EEST) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05olkn2055.outbound.protection.outlook.com [40.92.91.55]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9EB9168B325 for ; Mon, 20 Jun 2022 14:35:34 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QNkLsQ9Ardz6uXj4MVCrPYppKSYDMUXoPMmESmGMvpHLLrceWMPctL6UceR6MBhZr1ZOgsyYMcnc3ZREoRKmfbAYIULoPDGSXWLjJqqkf4zwManeq8hmvZgRV8PcVLmSl0DFokOe0+5m6IuXT4KF6UEDnxg/A7Da/aByzvmIV+dkypl6bskleq/ZF5CIL9QDhh3arnwcuhjL/txZ+42R5+A7fl3sy08ViP319J/OMjNRRoHNNJKVgMx0GBbcpDtt8bXzfS0UxDdK7eYBmhQ6GBQhms2hddc/KbT6aDbkuWYKPp6qNCYJtgPn/YUHp467uZo6Xt+AfponSk3s8uq0Mg== 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=eStKVN0se8+e35kZaENdVg8yYnHNg9PMIff69x06yPs=; b=oJQ7c3ZSUdXDuY0w8G/pEy03AYj1kBe1flOjFjjScCCgL8ztEqpCBkFwHu1TyLdOOI7D8CYrNYcv5ca0a6RquLXYG6/MEUGWV5iPL4i9AfXwsgz8RQRh/j4P14u2CfqTT/jTEsDEUqkVFYbXWH6ZtVdkxH7IlHMn9cVOJdJqmV6kmkOQROLz0ghyah7ninQwn89Z2wU/q200FuJUOiitrTD2I10/dtImHjECYOoQbxd58E/R476LT9tWL7XS2vWICLfNA5FR21XNGdDZ5/oTToSzpRm4r4YJ0ghstwBp99h3m0zZT3WnUdUDi3C3aEx/LkxMQ7ReAG4voWexQOqttg== 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=eStKVN0se8+e35kZaENdVg8yYnHNg9PMIff69x06yPs=; b=kyO4KV8Bv7rQlIZF9+YmNzZc3y75iIBantSRwiuD8cTo5nXWdx/4B5THN6fsofbThM1H04QqRhoG/Y4RvwYANAe94AtNB4ksFFYhX2wkKEXj2ebJubgtpL2hu4blQoC8AY2ZcMivzJj1howpoOZnKVX5F14muUmiVcZudOx1NhCf1iAPiWUgFnqnTHMV1H7nRh/5Tew6kizoKQLflwNooGZF2S4+L92MVmF6813d9Yd+b85Pprz9qlUNGxZdAxb0z5xkuB83UWe4r+jhgxdE/aTRmqLQfvH4CviTN94DIG12QxOldJubSuJ1MtJldfX+nAafK8uFSBIV+T6PS9Zn9w== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by DB8PR01MB6472.eurprd01.prod.exchangelabs.com (2603:10a6:10:154::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5353.14; Mon, 20 Jun 2022 11:35:33 +0000 Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::60b9:9f29:40cc:f01c]) by DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::60b9:9f29:40cc:f01c%10]) with mapi id 15.20.5353.021; Mon, 20 Jun 2022 11:35:33 +0000 From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Mon, 20 Jun 2022 13:35:23 +0200 Message-ID: X-Mailer: git-send-email 2.34.1 X-TMN: [K3rG9v46Dc54B7TqVJ494VQ6RPI/EEhJ] X-ClientProxiedBy: ZR0P278CA0196.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:44::22) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: <20220620113523.2676818-1-andreas.rheinhardt@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: cdf7bdcb-9c8e-4569-80dc-08da52b0fc7a X-MS-Exchange-SLBlob-MailProps: S/btQ8cKWiStnrXejT7dVH9fulBxg7H9aHrg1LCpZJC9LFSZNa3eVOLf6iqct5UzKdKDb+Vl/VhUrOpelafvYmmMzhcnHihK+wDSKk3I8CPAFlhcRAYYI94s/CYAiO0dTZzM9JApIipCutXXfvnmhGyuAbApMk1zktRBwV+te+XZ5wxcE/ywSjeGhjUSLA3sB94bGQBQHltVsN3xF7OVJBfGDbMRtVw3DoN8z0+oiycgCBUQYdhC9MFB6gSOzrm6Zbolyl5jQmRjFjsF3fBIVBDvaS1TLYWxasZ2lw0fKtNnslLnpXylM7yIH+4Gg+k7Ix5ey0n9y1H5hzn4WrwhzMkZrRMcaWMLLq5/7etWyEWYpngupR0RV8KhIWKItCVCEHHQ9PdfJYLTB40O3ZPMA3tzY6KlLnxUlky3uJz/XozZL9SYXkY+VMqS8Mc/rsxrISiEMpLbdFQErHSR9pwnby9Y9LV58eFwMDyAtefvBbKOKWkRQyOgzF4nWR61mr+TO22sQG8woU4fvBEG/EYGjqSDVYhrHPpNZ3AOw3cGzY7pWPLltltsjJpJRoPoULVmjD043kWN+FDN+KlTb2jXvWCiJaj3+15F7MKVo2ZuZ1WtryBfjDfQg8NLJo9ax8m0OrhZtojy1jcKO3YthFnCS1vJ0w0xs3ytm0FN9Yc1ex2y1jhqybNBLDGGBi/x19tqfTDC7dOjpyLDk6xV3WQX32TzlfPmAKoMLWIFRuVDJ/LQqKoWh/Hrw66nIZFMe8AQl8WxFYpnRjQ= X-MS-TrafficTypeDiagnostic: DB8PR01MB6472:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: KLGa1bkslxw/P8T4VifxsDSWKakNPX+4c/FHQmKApV4bTBPgOXLN7wPPduxumHK3EFFYx3rMDfGJXKePrK5Ygwp6uBM5oE549ClAgjyPt4DLxx4wbicPORK7U6gG/qMc8SLP5qY2M6VSUwlkpii5tJmGYZDEZTQVlkFwZ7qnxs8GrZDWP2jCjQnIKyLAs0TSH5cyd1br/vCaMcuU4uW3r2Mtsm9SDGhMy/fMacrOT8vJzXIYE5714Zo/xuXZoMUQlgMAJz52HwJc6WAhY/YpjIK/DJsV6bFb6Uy6PAeI0GPvAhmLZzAYiha83ULM8LiMuQp5oAC8J68LpzAtKKIznEioPsXaZeavvBRZ2yVYR9thSSFBZaldzykT/af1jCX3Ly2Kdn3HJC0vElHR8W2ecDF/7mJZm7gwGCxaGTJnF87j714ceTR9ygtqKZ2kDu/LmqVf1eW8q0Vr58OKaiOe0NUp3GM1ahli17WNLByqw1qE8VeYkeJsyDxjppy4cZq1gg+++84ExTZiVq+wzB4WCm11wloE1L+uAcgUh8PwCzaH9wpqGQqaACl6Blq3L1LrPA3bhA+75ij7I1nHSXOrYQ== X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tpsF+0W32smrIrwlMGGFmiJbiNuYTOogVPSRkz0t7rnbN83EXrgcw2s23Ogv?= =?us-ascii?Q?5Un0x5/X6sPEN6fi+gDZQPbSrpRbmyIypA1WPo/+MGKafAzKPKXxAPb3QFYu?= =?us-ascii?Q?jcpwdoE0G+lIF4UwMxJ9AoLNxhfGWiVto25vToNxwuRNl9iviNLqsdCeBvGL?= =?us-ascii?Q?9ZT/1gN2gy9Pp5Q/97Dm7hp/vORnbuTEXjNYtp0oP3tULOEd6od46dFVzOBc?= =?us-ascii?Q?szr4k+EHvuTNbYl/1xoEQk4fmiS9zyESUXcZB1ehsiVQM8l7nSUvhlnWORta?= =?us-ascii?Q?7RiLMF6Aemk6eHHgfuTvf1jckJ/dzQyRbAlPex1LQuSNgSwdrwpXF/dSBjLg?= =?us-ascii?Q?hn2gXTE1ltoguV5XLQbSC1/hAp5Y/zXWUOBHHHB/wks642L+Bkokbcyf5CqL?= =?us-ascii?Q?Na91PQjj1l2RG+6dePlZZiEtALP+RLLf43wHRNrL5xI/qxW3uPAFLIB4q7TI?= =?us-ascii?Q?oUsCAMOigRcRj/L2RieX5RtQc+TlUwplLT8YphvTdV1sbiWNDz4P3eiKem6A?= =?us-ascii?Q?tC72EPE09McD3PxGdTssVw/+E4Yh9noZ7qzSICCUeMMI8PVAOoqSK2ZyFqph?= =?us-ascii?Q?og1cJOySGUeHP4j8uR8PApVjpRtczT0+J75xnXbRzgRQRuhVAzW5XNmqsIem?= =?us-ascii?Q?C2c6i2K2XX02ctkM6qWBd5Z67b9Wd4mkP2bGpJ+VnTAqVJIY6V0x8lkU26fC?= =?us-ascii?Q?V8vqB9vTXDfKkbUAggE3QTwXrsyUuiemq51NQUG0usnOTnEGNrSSKk2T+Y6C?= =?us-ascii?Q?eU77+BaptWExvN/BubcgsPXJc0NGVsKtl51Ax88J9vBb7z1yVYUmHbOXCJHQ?= =?us-ascii?Q?IViiQ3vYPdHOuz/4skNTVkWQzEAMv5JflQA7OhZxRtWau17BclkPxUGmeKkM?= =?us-ascii?Q?rbrDllVyQh2h6QGlG+eL/dytu37RjGd+wN1MGf+e2PKCakwaDV2+bQZc2Hhw?= =?us-ascii?Q?lnugD2pZkaNZZuiz5hmb5naT3Ub0UstduC7ZjkuIdll6H71UIt0NkQy/1KAm?= =?us-ascii?Q?uNnqtrzoNfwF+gdiLabvncTE3cAiSlLcuMOwQF/8Nsg3q61iSVoKLjf22wre?= =?us-ascii?Q?FRqK7VS0RljY1VR2LkCWs7Q6vPbqfgTC+XlC/ZkYhpHbUUe5L956/nqR3a6I?= =?us-ascii?Q?S7DLu61ubXRFF9S88nrBI23lb/IyzGJr6E7tB8q/2oDRY9LsaWmFBHqOvfod?= =?us-ascii?Q?u/POiBCDMjg1MqNsJMRPD6WQAnYFMHzcEHYYA6oMMgel2cKOq0RvYlxn9jhl?= =?us-ascii?Q?eKZ7r7luLBnZn+Zf3gLuCdURltYRtyUX61mWQhM1ooyfV/pGj0KNednyqx53?= =?us-ascii?Q?UjNEhEOlvO67k3FVKeJcTlkYnw68Ccz++GNHsspHqhbODP+fbuQMCsWaVJtg?= =?us-ascii?Q?YF2v2Z7cfukojEuJ6aKAprXcLhrqeymrqpJpcKERosMTlo1PokcLVz5hDQq9?= =?us-ascii?Q?WCAoVcY6ZGEx5mp/x5KRiSukPZkAcsflXOrv53QhUUh/4S7Cc4pK7w=3D=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: cdf7bdcb-9c8e-4569-80dc-08da52b0fc7a X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jun 2022 11:35:33.3236 (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: DB8PR01MB6472 Subject: [FFmpeg-devel] [PATCH] avcodec/get_bits: Don't let number of bits_left become invalid 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: When using the checked cached bitstream reader, it can currently happen that the GetBitContext's number of bits_left becomes wrong, namely off by UINT_MAX due to wraparound (if it were an int, it would become negative). The reason for this is that reading new input is performed through show_bits() which does not update get_vlc2() if one is already past the end of input*, so that the following skip_remaining() may remove more bits than are actually present (it seems to have been designed as if it were only to be called if there are enough bits left). (The issue could be reproduced with fraps-v5-bouncing-balls-partial.avi from the FATE suite if fraps were modified to use a checked bitstream reader.) At the moment, this does not seem to cause any issues: The only place where bits_left is used as in the right operand of a shift is when refilling and this doesn't happen if one is already at the end. Furthermore, get_bits_count() and get_bits_left() return proper values, because they return an int and so being off by UINT_MAX doesn't matter. Some functions that check whether the cache has enough bits left will directly read from the cache. This commit nevertheless intends to fix this by adding a variant of refill_32() that always adds 32 bits to bits_left, but reports if it has added fake bits (not read from the bitstream), so that they can be compensated for. To do so, the first thing get_vlc2() does is ensuring that at least 32 bits are in the cache (even if fake bits), so that all the following operations can be performed on the cache (the earlier code potentially refilled the cache at every reading stage). This is also beneficial for code size. In the following, Clang means Clang 14 and GCC GCC 11.2: | Clang old | Clang new | GCC old | GCC new _________________________________________________________ fraps | f84 | f04 | c9e | b7e magicyuv | 1f2f | 1ecf | 1b1b | 1a49 mvha | c8a | bfa | 11f4 | 1168 photocd | 12d5 | 12f9 | 15ca | 15aa sheervideo | 14f2a | 11778 | 103d3 | 10339 utvideodec | 35a1 | 34d1 | 3037 | 2ea7 (None of the above files (which are all files that use the cached bitstream reader) call get_vlc2() with a max depth of one; in this case, saving refill_32() wins size-wise. This is unfortunately not true in case of max_depth == 1, where the added check leads to a slight codesize increase.) It is unfortunately not beneficial for speed in the majority of cases, probably because of the additional branch that is added to the common case in which the result is obtained after only one lookup. *: The unchecked bitstream reader does not have this check and is therefore unaffected by this issue. Signed-off-by: Andreas Rheinhardt --- get_xbits() also suffers from the problem that it skips bits that need not be present (no user of get_xbits() uses the cached bitstream reader). I think that there is another potential issue with the cached bitstream reader: Checking for whether an overread already happened may not work, namely if the buffer length is a multiple of four and != 4. In this case get_bits_left() will return 0 even after an overread. This could be fixed by modifying the overread check s->index >> 3 >= s->buffer_end - s->buffer to ">" (this is similar to using size_in_bits_plus8 in the ordinary GetBitContext). libavcodec/get_bits.h | 77 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h index 16f8af5107..d48ff111a4 100644 --- a/libavcodec/get_bits.h +++ b/libavcodec/get_bits.h @@ -227,13 +227,8 @@ static inline int get_bits_count(const GetBitContext *s) } #if CACHED_BITSTREAM_READER -static inline void refill_32(GetBitContext *s, int is_le) +static av_always_inline void refill_32_internal(GetBitContext *s, int is_le) { -#if !UNCHECKED_BITSTREAM_READER - if (s->index >> 3 >= s->buffer_end - s->buffer) - return; -#endif - if (is_le) s->cache = (uint64_t)AV_RL32(s->buffer + (s->index >> 3)) << s->bits_left | s->cache; else @@ -242,6 +237,52 @@ static inline void refill_32(GetBitContext *s, int is_le) s->bits_left += 32; } +/** + * If the end of input has not been reached yet, refill the GetBitContext + * with 32 bits read from the bitstream. + * Otherwise just pretend that it read 32 zeroes. One can then use + * show_val()/get_val()/skip_remaining() (which require enough cache bits + * to be available) as if there were at least 32 bits available; + * all other functions not solely based upon these are forbidden. + * Lateron one has to use fixup_fake with the return value of this function + * to restore the GetBitContext. + * + * This function is internal to the GetBit-API; access from users is forbidden. + * + * @return The amount of fake bits added. + */ +static inline unsigned refill_32_fake(GetBitContext *s, int is_le) +{ +#if !UNCHECKED_BITSTREAM_READER + if (s->index >> 3 >= s->buffer_end - s->buffer) { + s->bits_left += 32; + return 32; + } +#endif + refill_32_internal(s, is_le); + return 0; +} + +static inline void fixup_fake(GetBitContext *s, unsigned fake_bits) +{ +#if !UNCHECKED_BITSTREAM_READER + if (s->bits_left <= fake_bits) + s->bits_left = 0; + else + s->bits_left -= fake_bits; +#endif +} + +static inline void refill_32(GetBitContext *s, int is_le) +{ +#if !UNCHECKED_BITSTREAM_READER + if (s->index >> 3 >= s->buffer_end - s->buffer) + return; +#endif + refill_32_internal(s, is_le); + return; +} + static inline void refill_64(GetBitContext *s, int is_le) { #if !UNCHECKED_BITSTREAM_READER @@ -773,6 +814,7 @@ static inline const uint8_t *align_get_bits(GetBitContext *s) SKIP_BITS(name, gb, n); \ } while (0) +#if CACHED_BITSTREAM_READER /* Return the LUT element for the given bitstream configuration. */ static inline int set_idx(GetBitContext *s, int code, int *n, int *nb_bits, const VLCElem *table) @@ -780,11 +822,12 @@ static inline int set_idx(GetBitContext *s, int code, int *n, int *nb_bits, unsigned idx; *nb_bits = -*n; - idx = show_bits(s, *nb_bits) + code; + idx = show_val(s, *nb_bits) + code; *n = table[idx].len; return table[idx].sym; } +#endif /** * Parse a vlc code. @@ -800,9 +843,21 @@ static av_always_inline int get_vlc2(GetBitContext *s, const VLCElem *table, { #if CACHED_BITSTREAM_READER int nb_bits; - unsigned idx = show_bits(s, bits); - int code = table[idx].sym; - int n = table[idx].len; + unsigned fake_bits = 0; + unsigned idx; + int n, code; + + /* We only support VLC tables whose codelengths are bounded by 32. */ + if (s->bits_left < 32) +#ifdef BITSTREAM_READER_LE + fake_bits = refill_32_fake(s, 1); +#else + fake_bits = refill_32_fake(s, 0); +#endif + + idx = show_val(s, bits); + code = table[idx].sym; + n = table[idx].len; if (max_depth > 1 && n < 0) { skip_remaining(s, bits); @@ -814,6 +869,8 @@ static av_always_inline int get_vlc2(GetBitContext *s, const VLCElem *table, } skip_remaining(s, n); + fixup_fake(s, fake_bits); + return code; #else int code; -- 2.34.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".