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 74ADD480D6 for ; Thu, 9 Nov 2023 20:36:27 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5997D68CB6E; Thu, 9 Nov 2023 22:36:25 +0200 (EET) Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A957568C902 for ; Thu, 9 Nov 2023 22:36:19 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=s31663417; t=1699562179; x=1700166979; i=eclipse7@gmx.net; bh=fIF614kMWDKxMjFHuIEWLA08rDgrCUv0jgUK1vbn8p8=; h=X-UI-Sender-Class:Date:From:To:Subject:References:In-Reply-To; b=kJpiAGs/dTO9Ya7GYoyyeYuGgq43n5JuSJo0Um/VYIZeFwEJ5LWFfigC0aWJ4Ui7 YtDfzTsVFFJQ1P1MvKEoaXvFmHAB+18ysZOiXsdymLbEjH10UT3PuvDFbrscd8lI+ kW+r6mh7k2eFfRCNIQ7pVEfWErnN9nSGNg06vAYdyS/9JM57ZXShk1HOMxMjrfOOm XSoAzDFXyUJ+jBW7GBJ4oISZFz2mllmxMcSLpvcnOkr1HYbir7BxVtzNk8YV/oGg3 c74V1M3D7qLv65pWIoWbhhDpLFxfEJZkxu4lFtyjV4pWntO+Hr4Waqu5Sq6a/edpX o8TcN+PLpssiWXETlQ== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from metallschleimette ([91.12.124.168]) by mail.gmx.net (mrgmx004 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MFbRs-1rBhUq3tQk-00H7Ge for ; Thu, 09 Nov 2023 21:36:18 +0100 Date: Thu, 9 Nov 2023 21:45:35 +0100 From: Alexander Strasser To: FFmpeg development discussions and patches Message-ID: References: <20231031084044.GQ3543730@pb2> <169944363034.11195.830944857438126327@lain.khirnov.net> <169952480359.11195.9525129954974840844@lain.khirnov.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <169952480359.11195.9525129954974840844@lain.khirnov.net> X-Provags-ID: V03:K1:4+f45/Dd9b8ApMl4Hdn+lobneFWSu6HYi0zntH0AZPtdNld6BO6 V28pYpjYquLA1yQPHA/MBkibfjqRn9yUlNuV/reJmbo4h/djzovp5DAM8d2hfrXoJ/SHEuE 3Pl996dChUDFaqO9MbgcPZesnRty/Fm+K3J4Qlpjg/GBXiBpo1XoRiL0Sn8WJBmc3VSo/sy xEfpwPkyrZNzwwuebJBBQ== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:lvTQG9fBzdY=;OJhm4c6h1JupBdIuxBjwSw6lnQP nnXlp7UCGt9EbxofIM5FuS5FrXr8Y5mqVN9H+wSE+wilv6GlFDdzBssG33bmbcqbVwyMOf0BB CC2kdzEZXvgVX8yhuii5Jj3MCbeuLRjMf9XLbr5O9/VasjCqfbyRkJthr5R6ZlJA6XBkS2Tgo xRgVkgbIIfYFiivGqUW5gr53pw3CIlQ0Iz/ippkV7aFo88xj0czyFvXGBssKVJh74f6yax6RA CGalz82B6sOvpEG8lTbjPo7usZZepnwcyaxE3GoeVteMCfy0mEcu6I807U+5YylDX5HKGOKa3 h/hCMlEnOGkQBuIysvaLYudWpKNA+hlelwlOMtDYdB9oI2Hvj/BPUDxKvlTXf9UpM5jCR2TD9 zCrOmH47WuBVzC7q9H5MQ516EYeyFa2D0eDiH5DPAFmf1koAVy3A5iIknM7GAyncrKmK/jYRZ f1tInI5ZGZRvcJHfFIrqXn7uzJ8AbTAXGh3uixw7C6QedzowobZ9LA/mLhYCXJXP94gf+88ag V+WwykWM/Bn8W5fpnVmBc9/LnfjJp5wNh9yFNH3pBqzUBtPEE0TIePV60LXX0D8P8jBnfPsJ5 9MYrslXpH00JuMcclghoDom9QqHbb2vG/T61BujjCARfGtGb2meLNutJ14YzT9VEcyPIOAWuR VyhwyKEXg7dUOHTEGV9GkjCHmLmIUQpU3psrW4DMAvRihpFP1kDBW24TcEBCXrcqotwHBB7YF 2nw2y6iGTwFcNPULHyPlWqaGmkvpUVTP7YzNCFM8IlaqTmywKtd+WAdGl4caQTyci9svhdbQ+ q3U1xSxzFa5S7Jav9a2HyqUT4tQSS1rL+dsX6STj8oABrweNGoldE1SQ5QYbGnhpemeNd+Bcr K8EWd7W66V0Ovgetuxkmzco1edLerCEZnzCXFswMOYezgVDmjhsUu/r9rWNhTIHtNbfyGa+55 jbn8/hpEBBaknCLL60Z0Beg0bPs= Subject: Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo: Remove spec-incompliant inverse quantisation 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: On 2023-11-09 11:13 +0100, Anton Khirnov wrote: > Quoting Alexander Strasser (2023-11-08 21:55:10) > > On 2023-11-08 12:40 +0100, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2023-10-31 09:40:44) > > > > On Mon, Oct 30, 2023 at 02:11:27PM +0100, Andreas Rheinhardt wrote: > > > > > Section 7.4.4 of the MPEG-2 specifications requires that the > > > > > last bit of the last coefficient be toggled so that the sum > > > > > of all coefficients is odd; both our decoder and encoder > > > > > did this only if the bitexact flag has been set (although > > > > > stuff like this should be behind AV_CODEC_FLAG2_FAST). > > > > > This patch changes this by removing the spec-incompliant > > > > > functions. > > > > > > > > This commit message should include benchamarks documenting the speed loss > > > > (of the unquantize, the IDCT and overall) > > > > It is expected that the speed of some IDCTs will be impacted negativly > > > > as the non zero terms will prevent the skiping of some significant code > > > > > > > > as well as information about how much PSNR improves (to the encoder input) > > > > > > > > Also the change is a +-1 in one spot before the IDCT, the IDCT is not bitexactly > > > > specified in MPEG-2 so one could think of this as a > > > > correct implementation followed by a IDCT that was sometimes +-1 off > > > > instead of spec non compliance > > > > > > > > Only after the benchmarks and PSNR is presented should we decide if this > > > > is a change we want > > > > > > I disagree that the burden of proof should be on Andreas here. It should > > > be up to whoever wants to keep this code to show that it is useful. > > > > There was an argument presented. > > I see no argument for why the code in question is useful, can you point > to the exact text? First this: > > > > It is expected that the speed of some IDCTs will be impacted negativly > > > > as the non zero terms will prevent the skiping of some significant code Second there was an argument for compliance: > > > > Also the change is a +-1 in one spot before the IDCT, the IDCT is not bitexactly > > > > specified in MPEG-2 so one could think of this as a > > > > correct implementation followed by a IDCT that was sometimes +-1 off > > > > instead of spec non compliance Third there was no rejection of the change, but a request for measurement of the effect. I would expect an approval of the patch if the measurement leads to insignificant enough results. > > That argument could be challenged or otherwise explained why it more > > important to have this always behave like with bitexact. > > > > This could lead to "OK, I think removal is better" or if not benchmarks > > could lead to one or the other decision. > > > > Saying the burden is on whoever wants to keep the code sounds like a way > > for arbitrary code removal. While I agree getting rid of code can be a good > > thing, this would definitely take it too far. > > All code is a maintenance burden, therefore all code should have a > reason for its presence in the codebase, otherwise it should be removed. I can't see how the reason for the presence of code can be ultimately defined objectively and non-arbitrary. Alexander _______________________________________________ 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".