* [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
@ 2022-10-17 2:36 Peter Ross
2022-10-17 3:04 ` Andreas Rheinhardt
0 siblings, 1 reply; 6+ messages in thread
From: Peter Ross @ 2022-10-17 2:36 UTC (permalink / raw)
To: ffmpeg-devel
[-- Attachment #1.1: Type: text/plain, Size: 4319 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.
---
libavcodec/svq1_vlc.h | 4 ++--
tests/ref/fate/svq1 | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 13 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
--
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] 6+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
2022-10-17 2:36 [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
@ 2022-10-17 3:04 ` Andreas Rheinhardt
2022-10-17 9:33 ` Peter Ross
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Rheinhardt @ 2022-10-17 3:04 UTC (permalink / raw)
To: ffmpeg-devel
Peter Ross:
> 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.
Seems like this is not the only test whose reference needs to be
updated. There are also the fate-vsynth%-svq1 tests.
> ---
> libavcodec/svq1_vlc.h | 4 ++--
> tests/ref/fate/svq1 | 22 +++++++++++-----------
> 2 files changed, 13 insertions(+), 13 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
>
>
_______________________________________________
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] 6+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
2022-10-17 3:04 ` Andreas Rheinhardt
@ 2022-10-17 9:33 ` Peter Ross
2022-10-17 20:38 ` Michael Niedermayer
0 siblings, 1 reply; 6+ messages in thread
From: Peter Ross @ 2022-10-17 9:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1430 bytes --]
On Mon, Oct 17, 2022 at 05:04:29AM +0200, Andreas Rheinhardt wrote:
> Peter Ross:
> > 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.
>
> Seems like this is not the only test whose reference needs to be
> updated. There are also the fate-vsynth%-svq1 tests.
Right, the encoder. This change will cause previous videos created by the FFmpeg
encoder to playback *with* artifacts in newer builds of FFmpeg with this patch
applied. There is not much we can do about this, since there isn't place for a
version string in the SVQ3 bitstream (unlike MPEG-4 etc).
-- 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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
2022-10-17 9:33 ` Peter Ross
@ 2022-10-17 20:38 ` Michael Niedermayer
2022-10-18 1:44 ` Peter Ross
0 siblings, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2022-10-17 20:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2871 bytes --]
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.
> > >
> > > 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.
> >
> > Seems like this is not the only test whose reference needs to be
> > updated. There are also the fate-vsynth%-svq1 tests.
>
> Right, the encoder. This change will cause previous videos created by the FFmpeg
> encoder to playback *with* artifacts in newer builds of FFmpeg with this patch
> applied. There is not much we can do about this, since there isn't place for a
> version string in the SVQ3 bitstream (unlike MPEG-4 etc).
it is possible i think to avoid this
we just need anything that differs reasonably consistently between the binary
encoder and the ffmpeg encoder
the first thing that comes to mind is the temporal reference values, we seem
to always store 0 but there may be other things we do that differ
This fix will need some caution so everything keeps working
1. we need the decoder to detect the official and our old buggy encoder and
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
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 decodes
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
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
[-- 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] 6+ messages in thread* Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
2022-10-17 20:38 ` Michael Niedermayer
@ 2022-10-18 1:44 ` Peter Ross
2022-10-18 20:05 ` Michael Niedermayer
0 siblings, 1 reply; 6+ messages in thread
From: Peter Ross @ 2022-10-18 1:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 3857 bytes --]
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.
> > > >
> > > > 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.
> > >
> > > Seems like this is not the only test whose reference needs to be
> > > updated. There are also the fate-vsynth%-svq1 tests.
> >
> > Right, the encoder. This change will cause previous videos created by the FFmpeg
> > encoder to playback *with* artifacts in newer builds of FFmpeg with this patch
> > applied. There is not much we can do about this, since there isn't place for a
> > version string in the SVQ3 bitstream (unlike MPEG-4 etc).
>
> it is possible i think to avoid this
>
> we just need anything that differs reasonably consistently between the binary
> encoder and the ffmpeg encoder
> the first thing that comes to mind is the temporal reference values, we seem
> 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 increasing
sequence of numbers for each frame, and they wrap at 8-bits. i don't think we can
use a simple '== 0 case is FFmpeg', as there's a good chance that a legitimate
interframe will occur whe the field is 0.
there's not much else to detect whether its a FFmpeg file, just the checksum and
unknown extradata bits, both which the buggy FFmpeg encoder set to zero. Problem
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
> 1. we need the decoder to detect the official and our old buggy encoder and
> 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 hurt 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 decodes
> 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
>
> thx
going forward i propose we set the mov container extradata field to LIBAVCODEC_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 extradata field.
-- 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] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols
2022-10-18 1:44 ` Peter Ross
@ 2022-10-18 20:05 ` Michael Niedermayer
0 siblings, 0 replies; 6+ messages in thread
From: Michael Niedermayer @ 2022-10-18 20:05 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 5005 bytes --]
On Tue, Oct 18, 2022 at 12:44:20PM +1100, Peter Ross wrote:
> 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.
> > > > >
> > > > > 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.
> > > >
> > > > Seems like this is not the only test whose reference needs to be
> > > > updated. There are also the fate-vsynth%-svq1 tests.
> > >
> > > Right, the encoder. This change will cause previous videos created by the FFmpeg
> > > encoder to playback *with* artifacts in newer builds of FFmpeg with this patch
> > > applied. There is not much we can do about this, since there isn't place for a
> > > version string in the SVQ3 bitstream (unlike MPEG-4 etc).
> >
> > it is possible i think to avoid this
> >
> > we just need anything that differs reasonably consistently between the binary
> > encoder and the ffmpeg encoder
> > the first thing that comes to mind is the temporal reference values, we seem
> > 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 increasing
> sequence of numbers for each frame, and they wrap at 8-bits. i don't think we can
> use a simple '== 0 case is FFmpeg', as there's a good chance that a legitimate
> interframe will occur whe the field is 0.
With a single frame a value of 1 to 255 has a 0% chance to be ffmpeg and a
100% chance to be the official encoder (this is only true if we asume
no damage/corruption)
a 0 value has a chance that matches how often you assume these 2 encoded
files are encountered.
with 2 frames a 0, 0 will very likely be FFmpeg while all others are 100%
not FFmpeg. You can get a 0,0 by droping 255 frames that does not seem
like a probably thing also if 255 frames are lost artifacts already exist
likely and being wrong on the detection should not matter
i belive the TR is worth a try for detection. If it fails iam curious
of the case in which it does so. Maybe its not that easy and there are
unexpected cases
>
> there's not much else to detect whether its a FFmpeg file, just the checksum and
> unknown extradata bits, both which the buggy FFmpeg encoder set to zero. Problem
> 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
> > 1. we need the decoder to detect the official and our old buggy encoder and
> > 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 hurt 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 decodes
> > 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
> >
> > thx
>
> going forward i propose we set the mov container extradata field to LIBAVCODEC_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 extradata field.
good idea!
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
[-- 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] 6+ messages in thread
end of thread, other threads:[~2022-10-18 20:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17 2:36 [FFmpeg-devel] [PATCH] avcodec/svq1: fix interframe mean VLC symbols Peter Ross
2022-10-17 3:04 ` Andreas Rheinhardt
2022-10-17 9:33 ` Peter Ross
2022-10-17 20:38 ` Michael Niedermayer
2022-10-18 1:44 ` Peter Ross
2022-10-18 20:05 ` 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