diff mbox series

[FFmpeg-devel,PATCHv2,1/4] avcodec/svq1: fix interframe mean VLC symbols

Message ID 406d03fabf0bcf44fbfc894cf9960e3ad3947caf.1666229366.git.pross@xvid.org
State New
Headers show
Series [FFmpeg-devel,PATCHv2,1/4] avcodec/svq1: fix interframe mean VLC symbols | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Peter Ross Oct. 20, 2022, 1:29 a.m. UTC 
Fixes ticket #128.

The SVQ1 interframe mean VLC symbols -128 and 128 are incorrectly swapped
in our SVQ1 implementation, resulting in visible artifacts for some videos.
This patch unswaps the order of these two symbols.

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).

As a result of this patch, the reference data for the fate-svq1 test
($SAMPLES/svq1/marymary-shackles.mov) must be modified. For this file, our
decoder output is now bitwise identical to the reference decoder. I have
tested patch with various other samples and they are all now bitwise identical.

The SVQ1 encoder also produces different output because of this change, so
the the vsynth test reference data has also been updated.
---
 libavcodec/svq1_vlc.h             |  4 ++--
 tests/ref/fate/svq1               | 22 +++++++++++-----------
 tests/ref/vsynth/vsynth1-svq1     |  2 +-
 tests/ref/vsynth/vsynth2-svq1     |  4 ++--
 tests/ref/vsynth/vsynth_lena-svq1 |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

Comments

Michael Niedermayer Oct. 20, 2022, 7:33 p.m. UTC | #1 
On Thu, Oct 20, 2022 at 12:29:53PM +1100, Peter Ross wrote:
> Fixes ticket #128.
> 
> The SVQ1 interframe mean VLC symbols -128 and 128 are incorrectly swapped
> in our SVQ1 implementation, resulting in visible artifacts for some videos.
> This patch unswaps the order of these two symbols.
> 
> 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).
> 
> As a result of this patch, the reference data for the fate-svq1 test
> ($SAMPLES/svq1/marymary-shackles.mov) must be modified. For this file, our
> decoder output is now bitwise identical to the reference decoder. I have
> tested patch with various other samples and they are all now bitwise identical.
> 
> The SVQ1 encoder also produces different output because of this change, so
> the the vsynth test reference data has also been updated.

The order of the patches should be different because with this first
the encoder and decoder behavior changes and no detection / id is 
updated or used yet

ideally one should be able to checkout any revission on git master and
not generate files that do not decode correctly. 
And files generated from this will get misdetected as old FFmpeg later
if its first

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/svq1_vlc.h b/libavcodec/svq1_vlc.h
index 06e3509e4d..5c27928c2a 100644
--- a/libavcodec/svq1_vlc.h
+++ b/libavcodec/svq1_vlc.h
@@ -167,7 +167,7 @@  const uint16_t ff_svq1_inter_mean_vlc[512][2] = {
     { 0xA0, 22 },  { 0xA1, 22 },  { 0xA2, 22 },  { 0xA3, 22 },
     { 0xA4, 22 },  { 0xA5, 22 },  { 0xA6, 22 },  { 0xA7, 22 },
     { 0xA8, 22 },  { 0xA9, 22 },  { 0xAA, 22 },  { 0xAB, 22 },
-    { 0x7F, 22 },  { 0x8F, 21 },  { 0xAC, 22 },  { 0xAD, 22 },
+    { 0x8E, 21 },  { 0x8F, 21 },  { 0xAC, 22 },  { 0xAD, 22 },
     { 0xAE, 22 },  { 0xAF, 22 },  { 0xB0, 22 },  { 0xB1, 22 },
     { 0x53, 20 },  { 0x90, 21 },  { 0xB2, 22 },  { 0x91, 21 },
     { 0xB3, 22 },  { 0xB4, 22 },  { 0x54, 20 },  { 0xB5, 22 },
@@ -231,7 +231,7 @@  const uint16_t ff_svq1_inter_mean_vlc[512][2] = {
     { 0x87, 21 },  { 0x4F, 20 },  { 0x35, 19 },  { 0x4E, 20 },
     { 0x33, 19 },  { 0x32, 19 },  { 0x4D, 20 },  { 0x4C, 20 },
     { 0x83, 22 },  { 0x4B, 20 },  { 0x81, 22 },  { 0x80, 22 },
-    { 0x8E, 21 },  { 0x7E, 22 },  { 0x7D, 22 },  { 0x84, 21 },
+    { 0x7F, 22 },  { 0x7E, 22 },  { 0x7D, 22 },  { 0x84, 21 },
     { 0x8D, 21 },  { 0x7A, 22 },  { 0x79, 22 },  { 0x4A, 20 },
     { 0x77, 22 },  { 0x76, 22 },  { 0x89, 21 },  { 0x74, 22 },
     { 0x73, 22 },  { 0x72, 22 },  { 0x49, 20 },  { 0x70, 22 },
diff --git a/tests/ref/fate/svq1 b/tests/ref/fate/svq1
index d53e2952e4..0b0948cce6 100644
--- a/tests/ref/fate/svq1
+++ b/tests/ref/fate/svq1
@@ -24,19 +24,19 @@ 
 0,         18,         18,        1,    21600, 0x8d5b2ad0
 0,         19,         19,        1,    21600, 0xe67128e6
 0,         20,         20,        1,    21600, 0xb7bf613e
-0,         21,         21,        1,    21600, 0xefd0f51b
-0,         22,         22,        1,    21600, 0x31b7da59
+0,         21,         21,        1,    21600, 0xf697fa3e
+0,         22,         22,        1,    21600, 0x5b6ede88
 0,         23,         23,        1,    21600, 0x7a84a8f7
 0,         24,         24,        1,    21600, 0x0351ad27
-0,         25,         25,        1,    21600, 0xed6f434d
-0,         26,         26,        1,    21600, 0x0e771127
-0,         27,         27,        1,    21600, 0x37bf0b95
-0,         28,         28,        1,    21600, 0x30e10a77
-0,         29,         29,        1,    21600, 0x1a48288a
-0,         30,         30,        1,    21600, 0xf43c6770
-0,         31,         31,        1,    21600, 0x3c43ae68
-0,         32,         32,        1,    21600, 0x04dc0949
-0,         33,         33,        1,    21600, 0x7920758d
+0,         25,         25,        1,    21600, 0x57b547c2
+0,         26,         26,        1,    21600, 0xbb9e1558
+0,         27,         27,        1,    21600, 0xcb470f6b
+0,         28,         28,        1,    21600, 0xeb100de0
+0,         29,         29,        1,    21600, 0x089c2bf0
+0,         30,         30,        1,    21600, 0xe27b6a42
+0,         31,         31,        1,    21600, 0xbfe2b11b
+0,         32,         32,        1,    21600, 0xd9ca0bb5
+0,         33,         33,        1,    21600, 0x12fe783c
 0,         34,         34,        1,    21600, 0x6c12bab5
 0,         35,         35,        1,    21600, 0x1ac23706
 0,         36,         36,        1,    21600, 0x7a95cb5f
diff --git a/tests/ref/vsynth/vsynth1-svq1 b/tests/ref/vsynth/vsynth1-svq1
index cb89915d22..1970f779fc 100644
--- a/tests/ref/vsynth/vsynth1-svq1
+++ b/tests/ref/vsynth/vsynth1-svq1
@@ -1,4 +1,4 @@ 
-39ec74da265e3ef27756618108641181 *tests/data/fate/vsynth1-svq1.mov
+66933a2b34e123a9fc612cbc4dbbe353 *tests/data/fate/vsynth1-svq1.mov
 1334233 tests/data/fate/vsynth1-svq1.mov
 9cc35c54b2c77d36bd7e308b393c1f81 *tests/data/fate/vsynth1-svq1.out.rawvideo
 stddev:    9.58 PSNR: 28.50 MAXDIFF:  210 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-svq1 b/tests/ref/vsynth/vsynth2-svq1
index 4a50775501..32a0b80df2 100644
--- a/tests/ref/vsynth/vsynth2-svq1
+++ b/tests/ref/vsynth/vsynth2-svq1
@@ -1,4 +1,4 @@ 
-1c12440c323bc8ace5464587b5369c4a *tests/data/fate/vsynth2-svq1.mov
-940289 tests/data/fate/vsynth2-svq1.mov
+6ba5415d077304dc73ba8d8aaccb15eb *tests/data/fate/vsynth2-svq1.mov
+940285 tests/data/fate/vsynth2-svq1.mov
 a8cd3b833cd7f570ddbf1e6b3eb125b6 *tests/data/fate/vsynth2-svq1.out.rawvideo
 stddev:    3.71 PSNR: 36.72 MAXDIFF:  210 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth_lena-svq1 b/tests/ref/vsynth/vsynth_lena-svq1
index 01c1b06c74..e0614df3f9 100644
--- a/tests/ref/vsynth/vsynth_lena-svq1
+++ b/tests/ref/vsynth/vsynth_lena-svq1
@@ -1,4 +1,4 @@ 
-a6398d8fd306cfe96dc41060335e67e8 *tests/data/fate/vsynth_lena-svq1.mov
+bbe2b28fcef16aa088d29984931eea6a *tests/data/fate/vsynth_lena-svq1.mov
 766701 tests/data/fate/vsynth_lena-svq1.mov
 aa03471dac3f49455a33a2b19fda1098 *tests/data/fate/vsynth_lena-svq1.out.rawvideo
 stddev:    3.23 PSNR: 37.93 MAXDIFF:   61 bytes:  7603200/  7603200