Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols
@ 2022-10-20  1:29 Peter Ross
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 2/4] avcodec/svq1enc: do not use ambiguous interframe mean symbols Peter Ross
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Peter Ross @ 2022-10-20  1:29 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 6285 bytes --]

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

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
-- 
2.35.1

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCHv2 2/4] avcodec/svq1enc: do not use ambiguous interframe mean symbols
  2022-10-20  1:29 [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
@ 2022-10-20  1:30 ` Peter Ross
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field Peter Ross
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Peter Ross @ 2022-10-20  1:30 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 8326 bytes --]

Don't emit interframe mean symbols -128 and 128 to maintain compatibility with
older versions of FFmpeg that incorrectly interpret these symbols.
---
 libavcodec/svq1enc.c              |  5 +++++
 tests/ref/seek/vsynth_lena-svq1   | 28 ++++++++++++++--------------
 tests/ref/vsynth/vsynth1-svq1     |  8 ++++----
 tests/ref/vsynth/vsynth2-svq1     |  6 +++---
 tests/ref/vsynth/vsynth3-svq1     |  6 +++---
 tests/ref/vsynth/vsynth_lena-svq1 |  6 +++---
 6 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 75adbe7ea0..9bd5a04368 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -234,6 +234,11 @@ static int encode_block(SVQ1EncContext *s, uint8_t *src, uint8_t *ref,
         }
     }
 
+    if (best_mean == -128)
+        best_mean = -127;
+    else if (best_mean == 128)
+        best_mean = 127;
+
     split = 0;
     if (best_score > threshold && level) {
         int score  = 0;
diff --git a/tests/ref/seek/vsynth_lena-svq1 b/tests/ref/seek/vsynth_lena-svq1
index 33fe33e916..36c0fb7f4e 100644
--- a/tests/ref/seek/vsynth_lena-svq1
+++ b/tests/ref/seek/vsynth_lena-svq1
@@ -2,49 +2,49 @@ ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st:-1 flags:0  ts:-1.000000
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517568 size: 25636
+ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517672 size: 25636
 ret: 0         st: 0 flags:0  ts: 0.788359
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326556 size: 23552
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326616 size: 23552
 ret: 0         st: 0 flags:1  ts:-0.317500
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret:-1         st:-1 flags:0  ts: 2.576668
 ret: 0         st:-1 flags:1  ts: 1.470835
-ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517568 size: 25636
+ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517672 size: 25636
 ret: 0         st: 0 flags:0  ts: 0.365000
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157040 size: 21896
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157232 size: 21896
 ret: 0         st: 0 flags:1  ts:-0.740859
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret:-1         st:-1 flags:0  ts: 2.153336
 ret: 0         st:-1 flags:1  ts: 1.047503
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326556 size: 23552
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326616 size: 23552
 ret: 0         st: 0 flags:0  ts:-0.058359
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st: 0 flags:1  ts: 2.835859
-ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722804 size: 25888
+ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722884 size: 25888
 ret: 0         st:-1 flags:0  ts: 1.730004
-ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722804 size: 25888
+ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722884 size: 25888
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157040 size: 21896
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157232 size: 21896
 ret: 0         st: 0 flags:0  ts:-0.481641
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st: 0 flags:1  ts: 2.412500
-ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722804 size: 25888
+ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722884 size: 25888
 ret: 0         st:-1 flags:0  ts: 1.306672
-ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517568 size: 25636
+ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517672 size: 25636
 ret: 0         st:-1 flags:1  ts: 0.200839
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st: 0 flags:0  ts:-0.905000
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret: 0         st: 0 flags:1  ts: 1.989141
-ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722804 size: 25888
+ret: 0         st: 0 flags:1 dts: 1.920000 pts: 1.920000 pos: 722884 size: 25888
 ret: 0         st:-1 flags:0  ts: 0.883340
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326556 size: 23552
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 326616 size: 23552
 ret: 0         st:-1 flags:1  ts:-0.222493
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
 ret:-1         st: 0 flags:0  ts: 2.671641
 ret: 0         st: 0 flags:1  ts: 1.565859
-ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517568 size: 25636
+ret: 0         st: 0 flags:1 dts: 1.440000 pts: 1.440000 pos: 517672 size: 25636
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157040 size: 21896
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 157232 size: 21896
 ret: 0         st:-1 flags:1  ts:-0.645825
 ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     36 size: 22300
diff --git a/tests/ref/vsynth/vsynth1-svq1 b/tests/ref/vsynth/vsynth1-svq1
index 1970f779fc..e91ef46f17 100644
--- a/tests/ref/vsynth/vsynth1-svq1
+++ b/tests/ref/vsynth/vsynth1-svq1
@@ -1,4 +1,4 @@
-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
+78cdca850b19faf3aac0b0682207451e *tests/data/fate/vsynth1-svq1.mov
+1333541 tests/data/fate/vsynth1-svq1.mov
+0b9ee47ee4bf735fe3697daad64fc409 *tests/data/fate/vsynth1-svq1.out.rawvideo
+stddev:    9.57 PSNR: 28.50 MAXDIFF:  210 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-svq1 b/tests/ref/vsynth/vsynth2-svq1
index 32a0b80df2..b50ba45e20 100644
--- a/tests/ref/vsynth/vsynth2-svq1
+++ b/tests/ref/vsynth/vsynth2-svq1
@@ -1,4 +1,4 @@
-6ba5415d077304dc73ba8d8aaccb15eb *tests/data/fate/vsynth2-svq1.mov
-940285 tests/data/fate/vsynth2-svq1.mov
-a8cd3b833cd7f570ddbf1e6b3eb125b6 *tests/data/fate/vsynth2-svq1.out.rawvideo
+42578021105a2f526179c5601e635312 *tests/data/fate/vsynth2-svq1.mov
+940337 tests/data/fate/vsynth2-svq1.mov
+ba8f6b721a8e19fe8a6ef92a8cff7479 *tests/data/fate/vsynth2-svq1.out.rawvideo
 stddev:    3.71 PSNR: 36.72 MAXDIFF:  210 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth3-svq1 b/tests/ref/vsynth/vsynth3-svq1
index e760abbdb4..ba1d3d5082 100644
--- a/tests/ref/vsynth/vsynth3-svq1
+++ b/tests/ref/vsynth/vsynth3-svq1
@@ -1,4 +1,4 @@
-1972e0df8be667443992e405cceec291 *tests/data/fate/vsynth3-svq1.mov
-40773 tests/data/fate/vsynth3-svq1.mov
-a1e5334cf67649bf8c7d95dc4d1bf148 *tests/data/fate/vsynth3-svq1.out.rawvideo
+03805cb764c00c2162b2bed24b7f34bd *tests/data/fate/vsynth3-svq1.mov
+40757 tests/data/fate/vsynth3-svq1.mov
+a99efde992a2e3efcc085ecc6920a1e3 *tests/data/fate/vsynth3-svq1.out.rawvideo
 stddev:   14.49 PSNR: 24.91 MAXDIFF:  183 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth_lena-svq1 b/tests/ref/vsynth/vsynth_lena-svq1
index e0614df3f9..94f260865a 100644
--- a/tests/ref/vsynth/vsynth_lena-svq1
+++ b/tests/ref/vsynth/vsynth_lena-svq1
@@ -1,4 +1,4 @@
-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
+7534b2c6b7fc7201f193e9b4514cdb90 *tests/data/fate/vsynth_lena-svq1.mov
+766817 tests/data/fate/vsynth_lena-svq1.mov
+85261558fa744ef468fe77dbe4d91d8d *tests/data/fate/vsynth_lena-svq1.out.rawvideo
 stddev:    3.23 PSNR: 37.93 MAXDIFF:   61 bytes:  7603200/  7603200
-- 
2.35.1

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field
  2022-10-20  1:29 [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 2/4] avcodec/svq1enc: do not use ambiguous interframe mean symbols Peter Ross
@ 2022-10-20  1:30 ` Peter Ross
  2022-10-20  1:42   ` James Almer
  2022-10-20 19:26   ` Michael Niedermayer
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 4/4] avcodec/svq1dec: detect buggy FFmpeg encoder and apply correction to interframe mean symbols Peter Ross
  2022-10-20 19:33 ` [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Michael Niedermayer
  3 siblings, 2 replies; 8+ messages in thread
From: Peter Ross @ 2022-10-20  1:30 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 1118 bytes --]

This will enable the acurate identification of FFmpeg produced
SVQ1 streams, should there be new bugs found in the encoder.
---
 libavcodec/svq1enc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index 9bd5a04368..6aacaef88d 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -41,6 +41,7 @@
 #include "svq1.h"
 #include "svq1encdsp.h"
 #include "svq1enc_cb.h"
+#include "version.h"
 
 #include "libavutil/avassert.h"
 #include "libavutil/frame.h"
@@ -628,6 +629,14 @@ static av_cold int svq1_encode_init(AVCodecContext *avctx)
 
     ff_h263_encode_init(&s->m); // mv_penalty
 
+    if (!(s->avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
+        avctx->extradata = av_malloc(sizeof(LIBAVCODEC_IDENT));
+        if (!avctx->extradata)
+            return AVERROR(ENOMEM);
+        memcpy(avctx->extradata, LIBAVCODEC_IDENT, sizeof(LIBAVCODEC_IDENT));
+        avctx->extradata_size = sizeof(LIBAVCODEC_IDENT);
+    }
+
     return 0;
 }
 
-- 
2.35.1

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCHv2 4/4] avcodec/svq1dec: detect buggy FFmpeg encoder and apply correction to interframe mean symbols
  2022-10-20  1:29 [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 2/4] avcodec/svq1enc: do not use ambiguous interframe mean symbols Peter Ross
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field Peter Ross
@ 2022-10-20  1:30 ` Peter Ross
  2022-10-20 19:33 ` [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Michael Niedermayer
  3 siblings, 0 replies; 8+ messages in thread
From: Peter Ross @ 2022-10-20  1:30 UTC (permalink / raw)
  To: ffmpeg-devel


[-- Attachment #1.1: Type: text/plain, Size: 5102 bytes --]

---
 libavcodec/svq1dec.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
index 828b38b93d..c7269456e2 100644
--- a/libavcodec/svq1dec.c
+++ b/libavcodec/svq1dec.c
@@ -73,6 +73,8 @@ typedef struct SVQ1Context {
     int height;
     int frame_code;
     int nonref;         // 1 if the current frame won't be referenced
+
+    int last_tempref;
 } SVQ1Context;
 
 static const uint8_t string_table[256] = {
@@ -229,7 +231,7 @@ static int svq1_decode_block_intra(GetBitContext *bitbuf, uint8_t *pixels,
 }
 
 static int svq1_decode_block_non_intra(GetBitContext *bitbuf, uint8_t *pixels,
-                                       ptrdiff_t pitch)
+                                       ptrdiff_t pitch, int buggy)
 {
     uint32_t bit_cache;
     uint8_t *list[63];
@@ -270,6 +272,13 @@ static int svq1_decode_block_non_intra(GetBitContext *bitbuf, uint8_t *pixels,
 
         mean = get_vlc2(bitbuf, svq1_inter_mean.table, 9, 3) - 256;
 
+        if (buggy) {
+            if (mean == -128)
+                mean = 128;
+            else if (mean == 128)
+                mean = -128;
+        }
+
         SVQ1_CALC_CODEBOOK_ENTRIES(ff_svq1_inter_codebooks);
 
         for (y = 0; y < height; y++) {
@@ -455,7 +464,7 @@ static int svq1_decode_delta_block(AVCodecContext *avctx, HpelDSPContext *hdsp,
                                    GetBitContext *bitbuf,
                                    uint8_t *current, uint8_t *previous,
                                    ptrdiff_t pitch, svq1_pmv *motion, int x, int y,
-                                   int width, int height)
+                                   int width, int height, int buggy)
 {
     uint32_t block_type;
     int result = 0;
@@ -487,7 +496,7 @@ static int svq1_decode_delta_block(AVCodecContext *avctx, HpelDSPContext *hdsp,
             ff_dlog(avctx, "Error in svq1_motion_inter_block %i\n", result);
             break;
         }
-        result = svq1_decode_block_non_intra(bitbuf, current, pitch);
+        result = svq1_decode_block_non_intra(bitbuf, current, pitch, buggy);
         break;
 
     case SVQ1_BLOCK_INTER_4V:
@@ -498,7 +507,7 @@ static int svq1_decode_delta_block(AVCodecContext *avctx, HpelDSPContext *hdsp,
             ff_dlog(avctx, "Error in svq1_motion_inter_4v_block %i\n", result);
             break;
         }
-        result = svq1_decode_block_non_intra(bitbuf, current, pitch);
+        result = svq1_decode_block_non_intra(bitbuf, current, pitch, buggy);
         break;
 
     case SVQ1_BLOCK_INTRA:
@@ -524,15 +533,18 @@ static void svq1_parse_string(GetBitContext *bitbuf, uint8_t out[257])
     out[i] = 0;
 }
 
-static int svq1_decode_frame_header(AVCodecContext *avctx, AVFrame *frame)
+static int svq1_decode_frame_header(AVCodecContext *avctx, AVFrame *frame, int * buggy)
 {
     SVQ1Context *s = avctx->priv_data;
     GetBitContext *bitbuf = &s->gb;
     int frame_size_code;
     int width  = s->width;
     int height = s->height;
+    int tempref;
 
-    skip_bits(bitbuf, 8); /* temporal_reference */
+    tempref = get_bits(bitbuf, 8); /* temporal_reference */
+    *buggy = tempref == 0 && s->last_tempref == 0 && avctx->extradata_size == 0;
+    s->last_tempref = tempref;
 
     /* frame type */
     s->nonref = 0;
@@ -624,7 +636,7 @@ static int svq1_decode_frame(AVCodecContext *avctx, AVFrame *cur,
     int buf_size       = avpkt->size;
     SVQ1Context     *s = avctx->priv_data;
     uint8_t *current;
-    int result, i, x, y, width, height;
+    int result, i, x, y, width, height, buggy;
     int ret;
 
     /* initialize bit buffer */
@@ -664,7 +676,7 @@ static int svq1_decode_frame(AVCodecContext *avctx, AVFrame *cur,
             src[i] = ((src[i] << 16) | (src[i] >> 16)) ^ src[7 - i];
     }
 
-    result = svq1_decode_frame_header(avctx, cur);
+    result = svq1_decode_frame_header(avctx, cur, &buggy);
     if (result != 0) {
         ff_dlog(avctx, "Error in svq1_decode_frame_header %i\n", result);
         return result;
@@ -734,7 +746,7 @@ static int svq1_decode_frame(AVCodecContext *avctx, AVFrame *cur,
                     result = svq1_decode_delta_block(avctx, &s->hdsp,
                                                      &s->gb, &current[x],
                                                      previous, linesize,
-                                                     s->pmv, x, y, width, height);
+                                                     s->pmv, x, y, width, height, buggy);
                     if (result != 0) {
                         ff_dlog(avctx,
                                 "Error in svq1_decode_delta_block %i\n",
@@ -820,6 +832,8 @@ static av_cold int svq1_decode_init(AVCodecContext *avctx)
 
     ff_thread_once(&init_static_once, svq1_static_init);
 
+    s->last_tempref = 0xFF;
+
     return 0;
 }
 
-- 
2.35.1

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field Peter Ross
@ 2022-10-20  1:42   ` James Almer
  2022-10-20  7:46     ` Peter Ross
  2022-10-20 19:26   ` Michael Niedermayer
  1 sibling, 1 reply; 8+ messages in thread
From: James Almer @ 2022-10-20  1:42 UTC (permalink / raw)
  To: ffmpeg-devel

On 10/19/2022 10:30 PM, Peter Ross wrote:
> This will enable the acurate identification of FFmpeg produced
> SVQ1 streams, should there be new bugs found in the encoder.
> ---
>   libavcodec/svq1enc.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> index 9bd5a04368..6aacaef88d 100644
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -41,6 +41,7 @@
>   #include "svq1.h"
>   #include "svq1encdsp.h"
>   #include "svq1enc_cb.h"
> +#include "version.h"
>   
>   #include "libavutil/avassert.h"
>   #include "libavutil/frame.h"
> @@ -628,6 +629,14 @@ static av_cold int svq1_encode_init(AVCodecContext *avctx)
>   
>       ff_h263_encode_init(&s->m); // mv_penalty
>   
> +    if (!(s->avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
> +        avctx->extradata = av_malloc(sizeof(LIBAVCODEC_IDENT));
> +        if (!avctx->extradata)
> +            return AVERROR(ENOMEM);
> +        memcpy(avctx->extradata, LIBAVCODEC_IDENT, sizeof(LIBAVCODEC_IDENT));
> +        avctx->extradata_size = sizeof(LIBAVCODEC_IDENT);

Can you explain what effect is this meant to have and where? Do or 
should muxers like mp4 and matroska expect extradata from svq1 streams 
that they are meant to write to the output file?

> +    }
> +
>       return 0;
>   }
>   
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field
  2022-10-20  1:42   ` James Almer
@ 2022-10-20  7:46     ` Peter Ross
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Ross @ 2022-10-20  7:46 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 3048 bytes --]

On Wed, Oct 19, 2022 at 10:42:39PM -0300, James Almer wrote:
> On 10/19/2022 10:30 PM, Peter Ross wrote:
> > This will enable the acurate identification of FFmpeg produced
> > SVQ1 streams, should there be new bugs found in the encoder.
> > ---
> >   libavcodec/svq1enc.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> > index 9bd5a04368..6aacaef88d 100644
> > --- a/libavcodec/svq1enc.c
> > +++ b/libavcodec/svq1enc.c
> > @@ -41,6 +41,7 @@
> >   #include "svq1.h"
> >   #include "svq1encdsp.h"
> >   #include "svq1enc_cb.h"
> > +#include "version.h"
> >   #include "libavutil/avassert.h"
> >   #include "libavutil/frame.h"
> > @@ -628,6 +629,14 @@ static av_cold int svq1_encode_init(AVCodecContext *avctx)
> >       ff_h263_encode_init(&s->m); // mv_penalty
> > +    if (!(s->avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
> > +        avctx->extradata = av_malloc(sizeof(LIBAVCODEC_IDENT));
> > +        if (!avctx->extradata)
> > +            return AVERROR(ENOMEM);
> > +        memcpy(avctx->extradata, LIBAVCODEC_IDENT, sizeof(LIBAVCODEC_IDENT));
> > +        avctx->extradata_size = sizeof(LIBAVCODEC_IDENT);
> 
> Can you explain what effect is this meant to have and where? Do or should
> muxers like mp4 and matroska expect extradata from svq1 streams that they
> are meant to write to the output file?

the intention is to populate MOV stsd atom with a unique version number, such
that if another encoder bug is discovered, we can acurately detect the version
of the encoder and apply workarounds. the mov stsd atom is mapped to avctx->extradata.

we already use this approach for other encoders. the MPEG4 ASP video encoder
inserts the codec ident in a free text part of the header. there is no similar
free text space in the SVQ1 bitstream. fortunately the official/binary decoder
ignores the stsd atom, so i propose to use that. the SVQ1 official decoder has
not received updates for a long time and i dont expect it to change. the windows
binaries are marked end of life, and have not recieved update since 2009.

for other file formats, i expect them just to carry the extradata if they can.
this already works fine for avi. in the case of matroska, FFmpeg already inserts
the extradata in a V_QUICKTIME block per the specification
(https://www.matroska.org/technical/codec_specs.html ), so nothing to do there.

however now that i look closely, FFmpeg also adds a custom atom around the extradata
(commit in 8456bd2c0f3b08756f353646fe3b40a6772e665e), and doesn't strip this away
on demux. this is wrong, imho. 

it may be worth improving this patch set the following two ways:

 1) modify the SVQ1 encoder ident extradata to look like an stsd atom, by
     inserting length and tag fields at the beginning.

 2) modify the matroska muxer *not* to emit this custom atom for codecs who's
    extradta is also in atom format (SVQ1 and SVQ3).

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field Peter Ross
  2022-10-20  1:42   ` James Almer
@ 2022-10-20 19:26   ` Michael Niedermayer
  1 sibling, 0 replies; 8+ messages in thread
From: Michael Niedermayer @ 2022-10-20 19:26 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1616 bytes --]

On Thu, Oct 20, 2022 at 12:30:33PM +1100, Peter Ross wrote:
> This will enable the acurate identification of FFmpeg produced
> SVQ1 streams, should there be new bugs found in the encoder.
> ---
>  libavcodec/svq1enc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> index 9bd5a04368..6aacaef88d 100644
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -41,6 +41,7 @@
>  #include "svq1.h"
>  #include "svq1encdsp.h"
>  #include "svq1enc_cb.h"
> +#include "version.h"
>  
>  #include "libavutil/avassert.h"
>  #include "libavutil/frame.h"
> @@ -628,6 +629,14 @@ static av_cold int svq1_encode_init(AVCodecContext *avctx)
>  
>      ff_h263_encode_init(&s->m); // mv_penalty
>  
> +    if (!(s->avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
> +        avctx->extradata = av_malloc(sizeof(LIBAVCODEC_IDENT));
> +        if (!avctx->extradata)
> +            return AVERROR(ENOMEM);
> +        memcpy(avctx->extradata, LIBAVCODEC_IDENT, sizeof(LIBAVCODEC_IDENT));
> +        avctx->extradata_size = sizeof(LIBAVCODEC_IDENT);

This could still store the "Lavc" without the version #
I thought we did that more consistently but it seems only some
encoders do it like aac:
libavcodec/aacenc.c-    const int bitexact = avctx->flags & AV_CODEC_FLAG_BITEXACT;
libavcodec/aacenc.c:    const char *aux_data = bitexact ? "Lavc" : LIBAVCODEC_IDENT;

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols
  2022-10-20  1:29 [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
                   ` (2 preceding siblings ...)
  2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 4/4] avcodec/svq1dec: detect buggy FFmpeg encoder and apply correction to interframe mean symbols Peter Ross
@ 2022-10-20 19:33 ` Michael Niedermayer
  3 siblings, 0 replies; 8+ messages in thread
From: Michael Niedermayer @ 2022-10-20 19:33 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1681 bytes --]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-10-20 19:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20  1:29 [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 2/4] avcodec/svq1enc: do not use ambiguous interframe mean symbols Peter Ross
2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 3/4] avcodec/svq1enc: output ident string in extradata field Peter Ross
2022-10-20  1:42   ` James Almer
2022-10-20  7:46     ` Peter Ross
2022-10-20 19:26   ` Michael Niedermayer
2022-10-20  1:30 ` [FFmpeg-devel] [PATCHv2 4/4] avcodec/svq1dec: detect buggy FFmpeg encoder and apply correction to interframe mean symbols Peter Ross
2022-10-20 19:33 ` [FFmpeg-devel] [PATCHv2 1/4] avcodec/svq1: fix interframe mean VLC symbols Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git