| Message ID | 20180701114600.5442-1-atomnuker@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Jul 01, 2018 at 12:46:00PM +0100, Rostislav Pehlivanov wrote: > Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> > --- > tests/fate/atrac.mak | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak > index acf79a539c..38a5f33150 100644 > --- a/tests/fate/atrac.mak > +++ b/tests/fate/atrac.mak > @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm fails on mips: stddev: 200.81 PSNR: 50.27 MAXDIFF: 2448 bytes: 937984/ 937984 MAXDIFF: |2448 - 0| >= 1 Test atrac9-2 failed. Look at tests/data/fate/atrac9-2.err for details. make: *** [fate-atrac9-2] Error 1 make: *** Waiting for unfinished jobs.... [...]
On 7/1/2018 8:46 AM, Rostislav Pehlivanov wrote: > Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> > --- > tests/fate/atrac.mak | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak > index acf79a539c..38a5f33150 100644 > --- a/tests/fate/atrac.mak > +++ b/tests/fate/atrac.mak > @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm > > FATE_ATRAC3P-$(call DEMDEC, OMA, ATRAC3P) += $(FATE_ATRAC3P) > > -FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) $(FATE_ATRAC3P-yes) > +FATE_ATRAC9 += fate-atrac9-1 > +fate-atrac9-1: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/sy_title01.at9 > +fate-atrac9-1: REF = $(SAMPLES)/atrac9/sy_title01.at9.pcm > + > +FATE_ATRAC9 += fate-atrac9-2 > +fate-atrac9-2: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9 > +fate-atrac9-2: REF = $(SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9.pcm Are these pcm files from a test suit (if any), or at least created by an official binary decoder? I'd rather avoid adding tests using reference files created by this same decoder, seeing you for example resent the patch three times to fix a wrong calculation each time. You never know if you'll find others later, which could result in different output that may just be different enough to fail on some hardware (usually mips/arm/x86_32). But if there is no other way, make sure to create the reference files using cpuflags 0 using an x86_32 build (for x87 float math), and not with asm enabled (imdct and such), or with an x86_64 build. If you can't run a x86_32 build, I or someone else could do it instead. > + > +FATE_ATRAC9-$(call DEMDEC, OMA, ATRAC9) += $(FATE_ATRAC9) > + > +FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) $(FATE_ATRAC3P-yes) $(FATE_ATRAC9-yes) > > $(FATE_ATRAC_ALL): CMP = oneoff > > @@ -42,3 +52,4 @@ FATE_SAMPLES_AVCONV += $(FATE_ATRAC_ALL) > fate-atrac: $(FATE_ATRAC_ALL) > fate-atrac3: $(FATE_ATRAC3-yes) > fate-atrac3p: $(FATE_ATRAC3P-yes) > +fate-atrac9: $(FATE_ATRAC9-yes) >
2018-07-01 14:51 GMT+02:00, James Almer <jamrial@gmail.com>: > On 7/1/2018 8:46 AM, Rostislav Pehlivanov wrote: >> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> >> --- >> tests/fate/atrac.mak | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak >> index acf79a539c..38a5f33150 100644 >> --- a/tests/fate/atrac.mak >> +++ b/tests/fate/atrac.mak >> @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = >> $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm >> >> FATE_ATRAC3P-$(call DEMDEC, OMA, ATRAC3P) += $(FATE_ATRAC3P) >> >> -FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) >> $(FATE_ATRAC3P-yes) >> +FATE_ATRAC9 += fate-atrac9-1 >> +fate-atrac9-1: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/sy_title01.at9 >> +fate-atrac9-1: REF = $(SAMPLES)/atrac9/sy_title01.at9.pcm >> + >> +FATE_ATRAC9 += fate-atrac9-2 >> +fate-atrac9-2: CMD = pcm -i >> $(TARGET_SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9 >> +fate-atrac9-2: REF = >> $(SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9.pcm > > Are these pcm files from a test suit (if any), or at least created > by an official binary decoder? I believe we added support for codecs when we were not able to produce "official" binary output. > I'd rather avoid adding tests using reference files created by > this same decoder Why not? I agree that in an ideal world, we would only have known correct reference files but this is not the case now (we have always been knowingly testing "wrong" output") and, more important, I don't think fate is a test for correctness but much more a regression test to find unintended changes in FFmpeg's behaviour, don't you agree? Finding such unintended changes even makes sense for decoders that are known not to produce correct output. Good to know that this decoder is bit-exact at least on some platform=-) Carl Eugen
On Tue, Jul 03, 2018 at 09:35:53PM +0200, Carl Eugen Hoyos wrote: > 2018-07-01 14:51 GMT+02:00, James Almer <jamrial@gmail.com>: > > On 7/1/2018 8:46 AM, Rostislav Pehlivanov wrote: > >> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> > >> --- > >> tests/fate/atrac.mak | 13 ++++++++++++- > >> 1 file changed, 12 insertions(+), 1 deletion(-) > >> > >> diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak > >> index acf79a539c..38a5f33150 100644 > >> --- a/tests/fate/atrac.mak > >> +++ b/tests/fate/atrac.mak > >> @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = > >> $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm > >> > >> FATE_ATRAC3P-$(call DEMDEC, OMA, ATRAC3P) += $(FATE_ATRAC3P) > >> > >> -FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) > >> $(FATE_ATRAC3P-yes) > >> +FATE_ATRAC9 += fate-atrac9-1 > >> +fate-atrac9-1: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/sy_title01.at9 > >> +fate-atrac9-1: REF = $(SAMPLES)/atrac9/sy_title01.at9.pcm > >> + > >> +FATE_ATRAC9 += fate-atrac9-2 > >> +fate-atrac9-2: CMD = pcm -i > >> $(TARGET_SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9 > >> +fate-atrac9-2: REF = > >> $(SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9.pcm > > > > Are these pcm files from a test suit (if any), or at least created > > by an official binary decoder? > > I believe we added support for codecs when we were not able to > produce "official" binary output. > > > I'd rather avoid adding tests using reference files created by > > this same decoder > > Why not? > I agree that in an ideal world, we would only have known correct > reference files but this is not the case now (we have always been > knowingly testing "wrong" output") and, more important, I don't > think fate is a test for correctness but much more a regression > test to find unintended changes in FFmpeg's behaviour, don't you > agree? Finding such unintended changes even makes sense for > decoders that are known not to produce correct output. The problem with raw (pcm,rgb.yuv) reference files coming from our decoder and not teh official decoder is that they can change (that is for lossy codecs mainly, its harder to miss a relevant bug in a lossless decoder) Each time a bug is fixed in the decoder the reference can change and the pcm/yuv/rgb file in fate samples may then need to be updated. And we cannot change files in fate just add more files. So this can lead to "dead" files in the samples ... [...]
2018-07-04 1:09 GMT+02:00, Michael Niedermayer <michael@niedermayer.cc>: > On Tue, Jul 03, 2018 at 09:35:53PM +0200, Carl Eugen Hoyos wrote: >> 2018-07-01 14:51 GMT+02:00, James Almer <jamrial@gmail.com>: >> > On 7/1/2018 8:46 AM, Rostislav Pehlivanov wrote: >> >> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> >> >> --- >> >> tests/fate/atrac.mak | 13 ++++++++++++- >> >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak >> >> index acf79a539c..38a5f33150 100644 >> >> --- a/tests/fate/atrac.mak >> >> +++ b/tests/fate/atrac.mak >> >> @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = >> >> $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm >> >> >> >> FATE_ATRAC3P-$(call DEMDEC, OMA, ATRAC3P) += $(FATE_ATRAC3P) >> >> >> >> -FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) >> >> $(FATE_ATRAC3P-yes) >> >> +FATE_ATRAC9 += fate-atrac9-1 >> >> +fate-atrac9-1: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/sy_title01.at9 >> >> +fate-atrac9-1: REF = $(SAMPLES)/atrac9/sy_title01.at9.pcm >> >> + >> >> +FATE_ATRAC9 += fate-atrac9-2 >> >> +fate-atrac9-2: CMD = pcm -i >> >> $(TARGET_SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9 >> >> +fate-atrac9-2: REF = >> >> $(SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9.pcm >> > >> > Are these pcm files from a test suit (if any), or at least created >> > by an official binary decoder? >> >> I believe we added support for codecs when we were not able to >> produce "official" binary output. >> >> > I'd rather avoid adding tests using reference files created by >> > this same decoder >> >> Why not? >> I agree that in an ideal world, we would only have known correct >> reference files but this is not the case now (we have always been >> knowingly testing "wrong" output") and, more important, I don't >> think fate is a test for correctness but much more a regression >> test to find unintended changes in FFmpeg's behaviour, don't you >> agree? Finding such unintended changes even makes sense for >> decoders that are known not to produce correct output. > > The problem with raw (pcm,rgb.yuv) reference files coming from > our decoder and not teh official decoder is that they can change > (that is for lossy codecs mainly, its harder to miss a relevant bug in a > lossless decoder) > Each time a bug is fixed in the decoder the reference can change > and the pcm/yuv/rgb file in fate samples may then need to be updated. > And we cannot change files in fate just add more files. So this can > lead to "dead" files in the samples ... Thank you for reminding me, I misunderstood. Carl Eugen
diff --git a/tests/fate/atrac.mak b/tests/fate/atrac.mak index acf79a539c..38a5f33150 100644 --- a/tests/fate/atrac.mak +++ b/tests/fate/atrac.mak @@ -33,7 +33,17 @@ fate-atrac3p-2: REF = $(SAMPLES)/atrac3p/sonateno14op27-2-cut.pcm FATE_ATRAC3P-$(call DEMDEC, OMA, ATRAC3P) += $(FATE_ATRAC3P) -FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) $(FATE_ATRAC3P-yes) +FATE_ATRAC9 += fate-atrac9-1 +fate-atrac9-1: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/sy_title01.at9 +fate-atrac9-1: REF = $(SAMPLES)/atrac9/sy_title01.at9.pcm + +FATE_ATRAC9 += fate-atrac9-2 +fate-atrac9-2: CMD = pcm -i $(TARGET_SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9 +fate-atrac9-2: REF = $(SAMPLES)/atrac9/3_br144_nb10_ib10_g04_bex1_sf1_d1.at9.pcm + +FATE_ATRAC9-$(call DEMDEC, OMA, ATRAC9) += $(FATE_ATRAC9) + +FATE_ATRAC_ALL = $(FATE_ATRAC1-yes) $(FATE_ATRAC3-yes) $(FATE_ATRAC3P-yes) $(FATE_ATRAC9-yes) $(FATE_ATRAC_ALL): CMP = oneoff @@ -42,3 +52,4 @@ FATE_SAMPLES_AVCONV += $(FATE_ATRAC_ALL) fate-atrac: $(FATE_ATRAC_ALL) fate-atrac3: $(FATE_ATRAC3-yes) fate-atrac3p: $(FATE_ATRAC3P-yes) +fate-atrac9: $(FATE_ATRAC9-yes)
Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> --- tests/fate/atrac.mak | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)