From patchwork Sat Aug 29 17:56:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21995 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 5D6A744AD75 for ; Sat, 29 Aug 2020 20:57:21 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 429AB68AD2F; Sat, 29 Aug 2020 20:57:21 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B8ED268A848 for ; Sat, 29 Aug 2020 20:57:11 +0300 (EEST) Received: by mail-wm1-f65.google.com with SMTP id o21so1891606wmc.0 for ; Sat, 29 Aug 2020 10:57:11 -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=LDJ4xRvQcA36F9cgZvTj78prwOZHu6eAlddz0UJcpbM=; b=vPmjh2eiUIhKGQgK3hk4LW2TDxSga0ebZlgl3z2QIAn3h2/x9yZ7SXxAn7kaNYUume LEtro1sjZ3V7BQaWjbV8Hc0+gCVLzd1WUMVjt4jkp1ms3qLGre9kCBtcyRaTtDo2crf3 YL7zKVUhSs6u5nv7u6p3o5lROb56/Uw1WQselN8HIrCc7pigowz9H4VJDVYfsZffp1Gq nQruWBo4K6h2AVpsddwz9lqzb8OzPmaljjlI8e/mWUNPRwa+13tNEE8gpfRJzLSt7sG9 U2UzScYp/v2AZMqKRA0SvvV1zRGcFyWW5dFBI54lCkCMJb8IWRkdevyLPcLSlwlnQebj 9nCw== 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=LDJ4xRvQcA36F9cgZvTj78prwOZHu6eAlddz0UJcpbM=; b=lW0adHDzNPvO3X+Pi1htiylb4IiuqVHMKr2MEthqH0JOSF197i9EMvKCdO6JoSby4A I2S1pW4vBwd+UPpJGbMeGw+5Ha7Q3j/5eBcZz69IfM4Hhl9dqagEeX4Vp1/5JoP+NHFS PrrceC6BY7cuMTqzYjVTbF4GGFBEWTiwE5URMR3C6Y8P8dtIkFbMnk3qtvJ61BqB98aF eBMDx5e8G/NS5rsPtogCt8k6dE4mNw7aF++pEUpZKhN6YPCGFGznrSZMjCh+h7JVxnDp Xdykzpo3PhCDloaJXsDsAFk7UUL9THLifKC4JV88hWJBQfzyfWWAU+8+WR4HTThC/z4P QUHg== X-Gm-Message-State: AOAM530eifttv9UUStvEKojEopGyUZO8lrjdbiT0qmWYkFPSKdVl+WVY NkSQ9z1FQi9t5vdZvK30ODpwnHakUS0= X-Google-Smtp-Source: ABdhPJzSivu1nvqutMb5usXx7bcjY2vItyVDIrVyOkhaN0PH5czhnAN0YC8nTNg/b0gYXoSC4plUJw== X-Received: by 2002:a7b:c251:: with SMTP id b17mr3933120wmj.148.1598723830766; Sat, 29 Aug 2020 10:57:10 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id z8sm4016734wmf.10.2020.08.29.10.57.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 29 Aug 2020 10:57:10 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 29 Aug 2020 19:56:21 +0200 Message-Id: <20200829175626.11682-8-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200829175626.11682-1-andreas.rheinhardt@gmail.com> References: <20200829175626.11682-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 08/13] avcodec/wnv1: Use LE bitstream reader, avoid copying packet, fix memleak 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 Winnov WNV1 format is designed for a little-endian bitstream reader; yet our decoder reversed every byte bitwise (in a buffer only allocated for this purpose) to use a big-endian bitstream reader. This commit stops this. Two things needed to be done to achieve this: The codes in the table used to initialize a VLC reader needed to be reversed bitwise (when initializing a VLC in LE mode, it is expected that the first bit to be read is in the least significant bit; with BE codes the first bit to be read is the most significant bit of the code) and the following expression needed to be adapted: ff_reverse[get_bits(&w->gb, 8 - w->shift)] But this is easy: When only the bits read are reversed, they coincide with what a little-endian bitstream reader reads that reads the original, not-reversed data. But ff_reverse always reverses the full eight bits and this also performs a shift by (8 - (8 - w->shift)) on top of reversing the bits read. So the above line needs to be changed to get_bits(&w->gb, 8 - w->shift) << w->shift and this also shows why the variable shift is named the way it is. Finally, this also fixes a hypothetical memleak: For gigantic packets, initializing a GetBitContext can fail and in this case, the buffer containing the reversed data would leak. Signed-off-by: Andreas Rheinhardt --- libavcodec/wnv1.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/libavcodec/wnv1.c b/libavcodec/wnv1.c index 915e9c7dc9..857807a951 100644 --- a/libavcodec/wnv1.c +++ b/libavcodec/wnv1.c @@ -24,10 +24,10 @@ * Winnov WNV1 codec. */ +#define BITSTREAM_READER_LE #include "avcodec.h" #include "get_bits.h" #include "internal.h" -#include "mathops.h" typedef struct WNV1Context { @@ -36,9 +36,9 @@ typedef struct WNV1Context { } WNV1Context; static const uint16_t code_tab[16][2] = { - { 0x1FD, 9 }, { 0xFD, 8 }, { 0x7D, 7 }, { 0x3D, 6 }, { 0x1D, 5 }, { 0x0D, 4 }, { 0x005, 3 }, + { 0x17F, 9 }, { 0xBF, 8 }, { 0x5F, 7 }, { 0x2F, 6 }, { 0x17, 5 }, { 0x0B, 4 }, { 0x005, 3 }, { 0x000, 1 }, - { 0x004, 3 }, { 0x0C, 4 }, { 0x1C, 5 }, { 0x3C, 6 }, { 0x7C, 7 }, { 0xFC, 8 }, { 0x1FC, 9 }, { 0xFF, 8 } + { 0x01, 3 }, { 0x03, 4 }, { 0x07, 5 }, { 0x0F, 6 }, { 0x1F, 7 }, { 0x3F, 8 }, { 0x07F, 9 }, { 0xFF, 8 } }; #define CODE_VLC_BITS 9 @@ -50,7 +50,7 @@ static inline int wnv1_get_code(WNV1Context *w, int base_value) int v = get_vlc2(&w->gb, code_vlc.table, CODE_VLC_BITS, 1); if (v == 15) - return ff_reverse[get_bits(&w->gb, 8 - w->shift)]; + return get_bits(&w->gb, 8 - w->shift) << w->shift; else return base_value + ((v - 7U) << w->shift); } @@ -66,30 +66,17 @@ static int decode_frame(AVCodecContext *avctx, unsigned char *Y,*U,*V; int i, j, ret; int prev_y = 0, prev_u = 0, prev_v = 0; - uint8_t *rbuf; if (buf_size < 8 + avctx->height * (avctx->width/2)/8) { av_log(avctx, AV_LOG_ERROR, "Packet size %d is too small\n", buf_size); return AVERROR_INVALIDDATA; } - rbuf = av_malloc(buf_size + AV_INPUT_BUFFER_PADDING_SIZE); - if (!rbuf) { - av_log(avctx, AV_LOG_ERROR, "Cannot allocate temporary buffer\n"); - return AVERROR(ENOMEM); - } - memset(rbuf + buf_size, 0, AV_INPUT_BUFFER_PADDING_SIZE); - - if ((ret = ff_get_buffer(avctx, p, 0)) < 0) { - av_free(rbuf); + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) return ret; - } p->key_frame = 1; - for (i = 8; i < buf_size; i++) - rbuf[i] = ff_reverse[buf[i]]; - - if ((ret = init_get_bits8(&l->gb, rbuf + 8, buf_size - 8)) < 0) + if ((ret = init_get_bits8(&l->gb, buf + 8, buf_size - 8)) < 0) return ret; if (buf[2] >> 4 == 6) @@ -127,7 +114,6 @@ static int decode_frame(AVCodecContext *avctx, *got_frame = 1; - av_free(rbuf); return buf_size; } @@ -142,7 +128,7 @@ static av_cold int decode_init(AVCodecContext *avctx) code_vlc.table_allocated = 1 << CODE_VLC_BITS; init_vlc(&code_vlc, CODE_VLC_BITS, 16, &code_tab[0][1], 4, 2, - &code_tab[0][0], 4, 2, INIT_VLC_USE_NEW_STATIC); + &code_tab[0][0], 4, 2, INIT_VLC_USE_NEW_STATIC | INIT_VLC_LE); return 0; }