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 8DBE441138 for ; Mon, 14 Feb 2022 17:58:57 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C3E5368B158; Mon, 14 Feb 2022 19:58:53 +0200 (EET) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-oln040092072015.outbound.protection.outlook.com [40.92.72.15]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2BFAC68B0EA for ; Mon, 14 Feb 2022 19:58:48 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZQcjX3YDTF2Osw1M4+0bceULZnmeNOdlViMFqbdN7NVTVEuN6thP/9pSyXZ6O9AWTAzMdqDqhbTpNiOgl3wU9/U9uQnizuvfOpd4rF67mYDP/h3slb/cbhn22gM/uNtaBQJEn5ptnJCDwhan9mfKd5SQaZkb7cfJvF+2xdHb6M18I3APDg67l7zQAifSJ9ASZU8S53igLsmtm82JKOLlUIFzibZHG6eYouWUfyJisAGI3fY/VPFkIVX+GRy0XwiO4WVf0seUdkbxp9t591Tc8TCQYaizGpWC1Nm3S3UFFGoRsT6mhA4WOsF4Us5YVfA4XuEipTrTSsAuPSvPLhA2Tg== 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=X3IRbjETPfu9Ly6uQfyocdDsyACZbdxV2WxIux0GpQo=; b=i/z5eTgXmuaatP8vl+88Gv6ayAGodNIBet41iYFV5h5fPv33UE8kWLF0u4hYqe6X0qDN9cUvE9a6FD1QyQb8+Mw8tvvSsJhzcDLM9OPZQZcuD/xCEGmPDgv7FXzjFcqOP3Tl197R95XxRVLc2jgI9U6VvM8WLIuO8EAmoTPoDNEP3ME3f4BXfeRy5+HjgNz1cpJy6dvPTSalzl7itErdNDfBPnbbpLpqSw0soABIomblcupzJyNi+w3FyG7NMS7pHll8QSTA1oNtEdfc/UwD+MRN1CxQNng86d3nzx3jT0vyGyD70A4UiNqsxi1CvkOj9huh1CSl3zaUXezXw7Llfw== 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=X3IRbjETPfu9Ly6uQfyocdDsyACZbdxV2WxIux0GpQo=; b=NkZsuYm7wehaF2yqobRIB21ZpW2eQE0YleffqZ+TFjRzwcHMflddEJ+yoYDaxaGSlP0RoR86f1DvfKBnUisT7emVbomL7yue1EFPEnYArv3IjW6kD0xQ3zlSqpErQq5dXWJ7F8vO6cCqL0eNZWemRrXZp/dglTXZe0Z1AYbGCHs+crjIUXajTmiRVHPzD3JUPGW1+31EuPnIV2G+rFbO9Xdb4CxQCZz0GIDsBOj3jV7nzKuTa//FJYXF3jSBpRo9LieuRWTNac4fMesHCEoz9ahJGsdNWPgxcpI6oeRaBDClkDBOMOxwSQ5B70Hmle7sRb12vqt9qj5ejX30Z7OFPw== Received: from AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) by AM4PR0302MB2627.eurprd03.prod.outlook.com (2603:10a6:200:90::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4975.18; Mon, 14 Feb 2022 17:58:46 +0000 Received: from AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::ac56:2ff4:d304:ab22]) by AM7PR03MB6660.eurprd03.prod.outlook.com ([fe80::ac56:2ff4:d304:ab22%5]) with mapi id 15.20.4975.018; Mon, 14 Feb 2022 17:58:46 +0000 Message-ID: Date: Mon, 14 Feb 2022 18:58:44 +0100 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220213195131.1563462-1-onemda@gmail.com> <20220213195131.1563462-2-onemda@gmail.com> From: Andreas Rheinhardt In-Reply-To: <20220213195131.1563462-2-onemda@gmail.com> X-TMN: [EA5om76K3yE6g22D5FJizkAqf98uCXSM] X-ClientProxiedBy: AS9PR06CA0378.eurprd06.prod.outlook.com (2603:10a6:20b:460::18) To AM7PR03MB6660.eurprd03.prod.outlook.com (2603:10a6:20b:1c1::22) X-Microsoft-Original-Message-ID: <926cbec2-a26a-a0aa-2368-7c95936058a9@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: ca5df816-6400-44cd-e981-08d9efe3a516 X-MS-TrafficTypeDiagnostic: AM4PR0302MB2627:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: DDPwI//aPlFPXUXJSLcwL+AZb+fWiLONtvaHlk22q2l4/CPczhostyr0pc15cuADi03hFwQUkYUtXRhNgMtC4+K61YJZ1eHwYDDjbO1rgh51cvXT298Ly/HRV35E6wGz6aWApOYuyudwKohQ62WfKJU5gruwKiuvqAQj/x3fQXEqUwLvPPdNaWVRO58rOBRJxNEEX8nImHCS7y/0YDrSJXAiypnCjO8cy26fJcWejzxBwHrO9dLtD5RCK3tFjM7+Yq2GYQf3OmWMeiqfgkO/IjhmTTzxDsOl2B6UpeHZWBSwVerzurj2SBGZRmU3r6zoFRK+2ovAKDYdn+HPRVokVo1fa0fA/0LGHqlCuELTaWENxUdyQM4l6fNDH1pzFKxGVFcIZAZE8hOgbsh3OsymUa1duVJD6jL3L7HBayoTu1ZYyEifWPvXsX0Z9314FNiTCi7MlEkVaGRGG17UFaCRSKnVsD3JlSH9hZMiwqedbUnl9tZSPmqO2FHpdA028fiaa2lSTswHqBw+skbHq4LvaIROyiRXMXpUFN660oK8ZfdfAi0D63dXAYH6q8+CHmdDGVSK2phg0iC+NhT8si9neA== X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SmhtUVdvS0V3OWF5Qi9uZXEwQmJqZkJHbW9pQk4wQVE0MjMya1RpajlGbXJ5?= =?utf-8?B?dmJtQ1IwM0diNGVEWVA2ZTd1VStFcE01ZHFmL25obkhFSjV6WGdYZ0tUSEVZ?= =?utf-8?B?Q3JoR1o1Y29mNWhDbFF6bmJKaHdXTnNRUk9SZHovZ0hKWHVyVklkMFphUWJ5?= =?utf-8?B?Ukc0ajhnVVZEL2hCenpuVURoZ3ZmVlI3SDNBRlZqWDhnaklROWpyTDhZWVdJ?= =?utf-8?B?N3kzL3JxdWYrWkhERldHWHNsd0N5Z3JsYXRpTE96SWRIY0kzQTR0aW40R2Q2?= =?utf-8?B?NjZwVndwRnRJWktkd0VXUUV2ODRTTm9Ob25NNXN3ZzJPZGR3dlV0WmFzMnI3?= =?utf-8?B?bVhFQ3RCNjhHUS9uczlua0pqZ2NqOVdISjZqaVl0ZXVyeTBhNVdaSWVGenNI?= =?utf-8?B?cm8yR0REaVJmT09XbVlMSDM3OGpCcE1EcWU2SDFDRENSSzF1TkdEdXJrcE92?= =?utf-8?B?WjlRWmxzWE5OeEtkWjFCY2ZFZE1lWmZJcEcrUWV6MmtVNDJkQ0J4RVBWdWg3?= =?utf-8?B?N1dCMDYyK1lQMVNNa2RxdXVlQm5kQ1dWenhWZk1ibTVXV3ZtZ08vRjRBZkNS?= =?utf-8?B?N3I3MVArN3JuUWllOE1VTXFlRmZISXFCbmtsa0o1enF2emd2OUNBRGF5WWVO?= =?utf-8?B?dnJzdUVLU0I0cjNsRllZS2lPa1lxcUlpMkRlNHZBZmhMNmFkNUhHNjBBSFVE?= =?utf-8?B?UHhhM252dEFxOWhsZ3Zrc2Frcm5sSjBza3IxMXRzZ3U2ajVlMk1OYkxzZG1U?= =?utf-8?B?Nkt1ajZGM2FKY3NTY1hWck5Nc2pPSkw5OWxiMW9JeTc4K3ozcm9hZE42SnJu?= =?utf-8?B?OHZHN3NMNDZ3aWdsNmpOL1lGcDJSeFpRLzczbVdJZnFUazBNSmJBQzROaTNF?= =?utf-8?B?YndWbTdYR2tHVjIrVUt4NTMrSSt2Wk5SWWFMRVNLaGlDcExIWnVJMEJoZ2hO?= =?utf-8?B?VHRBZmt0ckNGT0ZyMnRmN3pudjZNbUpUQXRNc04rNWxTeWgvbkVlTVdPd2cz?= =?utf-8?B?Mm05S3hMN0l4eGlyN3RGNzd3aWh6YWhxNjZOSUVJaUp0T1pUVUo1Mjh2anho?= =?utf-8?B?UmlpSGlyNlRacjZpUnJ5TjNmUS9pKzlUTEIyNzVuSjliQkU4NStMRURQMXNS?= =?utf-8?B?QmRhcG5DUkN2Ni8vd1hqV3JmQWpMeXFKUkxDQVRQSjNmQ051ektwbXRaUndW?= =?utf-8?B?VmFXU3MvaUc5NlY4QVlQcWxmZTM4QnlUcFZycGh1cDV4VDNpT1czRnVpdWhO?= =?utf-8?B?MzN5NHRaa3JMRldZUDdaSmdUZlFPSDRublFnbmJ5NVVNWGRGdz09?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: ca5df816-6400-44cd-e981-08d9efe3a516 X-MS-Exchange-CrossTenant-AuthSource: AM7PR03MB6660.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2022 17:58:45.9887 (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: AM4PR0302MB2627 Subject: Re: [FFmpeg-devel] [PATCH 2/3] [WIP] avcodec: add HVQM4 Video decoder 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 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: Paul B Mahol: > Signed-off-by: Paul B Mahol > --- > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/codec_desc.c | 7 + > libavcodec/codec_id.h | 1 + > libavcodec/hvqm4.c | 1719 +++++++++++++++++++++++++++++++++++++++ > 5 files changed, 1729 insertions(+) > create mode 100644 libavcodec/hvqm4.c > > +static int read_trees(int index, > + int length, > + uint16_t code, > + uint8_t *bits, > + uint16_t *codes, > + uint16_t *symbols, > + GetBitContext *gb, > + const uint32_t tree_signed, > + const uint32_t tree_scale) > +{ > + if (get_bits1(gb) == 0) { // leaf node > + uint8_t byte = get_bits(gb, 8); > + int16_t symbol = byte; > + > + if (tree_signed && byte > 0x7F) Is the check "byte > 0x7F" actually is an improvement? > + symbol = (int8_t)byte; > + > + symbol *= 1 << tree_scale; > + bits[index] = length; > + codes[index] = code; > + symbols[index] = symbol; > + index++; > + return index; > + } else { // recurse > + index = read_trees(index, length + 1, code << 1, bits, codes, symbols, gb, tree_signed, tree_scale); > + index = read_trees(index, length + 1, (code << 1) + 1, bits, codes, symbols, gb, tree_signed, tree_scale); > + return index; > + } > +} > + > +static int build_huff(GBCWithVLC *buf, uint32_t is_signed, uint32_t scale) > +{ > + const uint32_t tree_signed = is_signed; > + const uint32_t tree_scale = scale; > + uint8_t bits[256] = { 0 }; > + uint16_t codes[256] = { 0 }; > + uint16_t symbols[256] = { 0 }; > + VLC *vlc = buf->vlc; > + int nb_codes; > + > + ff_free_vlc(vlc); > + nb_codes = read_trees(0, 0, 0, bits, codes, symbols, &buf->gb, tree_signed, tree_scale); > + > + return ff_init_vlc_sparse(vlc, ff_log2(nb_codes) + 3, nb_codes, bits, 1, 1, > + codes, 2, 2, symbols, 2, 2, 0); > +} > + 1. The above code does not check against stack overflow or buffer overflow; you are also not checking that your codes fit into 16bits. 2. I also don't get where your ff_log2(nb_codes) + 3 comes from. 3. The nodes are stored from left to right in the tree, so using ff_init_vlc_sparse() is unnecessary; ff_init_vlc_from_lengths() can be used like this (untested): struct HuffTree { int nb_codes; uint8_t bits[256]; uint16_t symbols[256]; } HuffTree; static int read_trees(int length, HuffTree *huff, GetBitContext *gb, const uint32_t tree_signed, const uint32_t tree_scale) { int ret; if (get_bits1(gb) == 0) { // leaf node uint8_t byte = get_bits(gb, 8); int16_t symbol = tree_signed ? (int8_t)byte : byte; symbol *= 1 << tree_scale; if (huff->nb_codes >= FF_ARRAY_ELEMS(huff->bits)) return AVERROR_INVALIDDATA; huff->bits[huff->nb_codes] = length; huff->symbols[huff->nb_codes] = symbol; huff->nb_codes++; return 0; } else { // recurse if (length == 16) return AVERROR_INVALIDDATA; ret = read_trees(length + 1, huff, gb, tree_signed, tree_scale); if (ret < 0) return ret; return read_trees(length + 1, huff, gb, tree_signed, tree_scale); } } static int build_huff(GBCWithVLC *buf, uint32_t is_signed, uint32_t scale) { const uint32_t tree_signed = is_signed; const uint32_t tree_scale = scale; HuffTree huff; VLC *vlc = buf->vlc; int ret; huff.nb_codes = 0; ff_free_vlc(vlc); ret = read_trees(0, &huff, &buf->gb, tree_signed, tree_scale); if (ret < 0) return ret; return ff_init_vlc_from_lengths(vlc, ff_log2(huff.nb_codes) + 3, huff.nb_codes, huff.bits, 1, huff.symbols, 2, 2, 0, 0, NULL); } 4. Is it legal for the tree to consists of only the root (which then has length zero)? Our code for generating VLCs can't handle this at the moment; I always pondered adding support for it (and somewhere I have a stash entry for it), but never did it. - Andreas _______________________________________________ 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".