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 C6D2F41A36 for ; Tue, 18 Oct 2022 01:44:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6439D68BCCC; Tue, 18 Oct 2022 04:44:41 +0300 (EEST) Received: from mx.sdf.org (mx.sdf.org [205.166.94.24]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C415668BBFE for ; Tue, 18 Oct 2022 04:44:34 +0300 (EEST) Received: from 7d19aeb8cdbb54b522328c0f343a8807 ([1.145.213.234]) (authenticated (0 bits)) by mx.sdf.org (8.15.2/8.14.5) with ESMTPSA id 29I1iQbQ007954 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits) verified NO) for ; Tue, 18 Oct 2022 01:44:30 GMT Date: Tue, 18 Oct 2022 12:44:20 +1100 From: Peter Ross To: FFmpeg development discussions and patches Message-ID: Mail-Followup-To: FFmpeg development discussions and patches References: <20221017203853.GA907335@pb2> MIME-Version: 1.0 In-Reply-To: <20221017203853.GA907335@pb2> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols 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: multipart/mixed; boundary="===============7954141761237702794==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7954141761237702794== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="jfdHCmfOBFi7kkCf" Content-Disposition: inline --jfdHCmfOBFi7kkCf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 17, 2022 at 10:38:53PM +0200, Michael Niedermayer wrote: > On Mon, Oct 17, 2022 at 08:33:28PM +1100, Peter Ross wrote: > > On Mon, Oct 17, 2022 at 05:04:29AM +0200, Andreas Rheinhardt wrote: > > > Peter Ross: > > > > Fixes ticket #128. > > > >=20 > > > > The SVQ1 interframe mean VLC symbols -128 and 128 are incorrectly s= wapped > > > > in our SVQ1 implementation, resulting in visible artifacts for some= videos. > > > > This patch unswaps the order of these two symbols. > > > >=20 > > > > The most noticable example of the artiacts caused by this error can= be observed in > > > > https://trac.ffmpeg.org/attachment/ticket/128/svq1_set.7z '352_288_= k_50.mov'. > > > > The artifacts are not observed when using the reference decoder > > > > (QuickTime 7.7.9 x86 binary). > > > >=20 > > > > As a result of this patch, the reference data for the fate-svq1 test > > > > ($SAMPLES/svq1/marymary-shackles.mov) must be modified. For this fi= le, our > > > > decoder output is now bitwise identical to the reference decoder. I= have > > > > tested patch with various other samples and they are all now bitwis= e identical. > > >=20 > > > Seems like this is not the only test whose reference needs to be > > > updated. There are also the fate-vsynth%-svq1 tests. > >=20 > > Right, the encoder. This change will cause previous videos created by t= he FFmpeg > > encoder to playback *with* artifacts in newer builds of FFmpeg with thi= s patch > > applied. There is not much we can do about this, since there isn't plac= e for a > > version string in the SVQ3 bitstream (unlike MPEG-4 etc). >=20 > it is possible i think to avoid this >=20 > we just need anything that differs reasonably consistently between the bi= nary > encoder and the ffmpeg encoder > the first thing that comes to mind is the temporal reference values, we s= eem > to always store 0 but there may be other things we do that differ for the temporal reference field, the official binary encoder emits an incr= easing sequence of numbers for each frame, and they wrap at 8-bits. i don't think = we can use a simple '=3D=3D 0 case is FFmpeg', as there's a good chance that a leg= itimate interframe will occur whe the field is 0. there's not much else to detect whether its a FFmpeg file, just the checksu= m and unknown extradata bits, both which the buggy FFmpeg encoder set to zero. Pr= oblem is, some legitimate samples also have both these fields set to zero. i am open to what other ideas you have! > This fix will need some caution so everything keeps working=20 > 1. we need the decoder to detect the official and our old buggy encoder a= nd=20 > handle each appropriately > 2. when we fix our encoder there are at least 3 ways > 2a. avoid the ambigous codes, these will decode fine in all cases i like this option. encoder performance is already abysmal, so it won't hur= t too much. > 2b. whatever we used to detect the official encoder we can mimic that too > while fixing this. but that may be a one shot opertunity > 2c. find a cleaner way to store our version first, implment that in the > decoder and then use it in the encoder when we fix this. This would > need some testing to make sure it works with the official decoder > I see a few things that might work for the version. the decoder decod= es > some "embedded message", theres the temporal reference which seems not > mattering > there are various encoder choices that allow embeding some message if= one > really wanted >=20 > thx going forward i propose we set the mov container extradata field to LIBAVCO= DEC_IDENT. "just" in case there is another error in the encoder that needs addressing = in future. my analysis of the binary decoder suggests it not inspect the container ext= radata field. -- Peter (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B) --jfdHCmfOBFi7kkCf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABECAB0WIQSpB+AvpuUM0jTNINJnYHnFrEDdawUCY04E8AAKCRBnYHnFrEDd a8IsAJ463LL58UJYeToD6HP2P5HboYSg4ACfYNasSDoZdBFYlh+sL05TGlAJyFg= =RiFe -----END PGP SIGNATURE----- --jfdHCmfOBFi7kkCf-- --===============7954141761237702794== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============7954141761237702794==--