| Message ID | 20170720204433.GH3740@nb4 |
|---|---|
| State | Not Applicable |
| Headers | show |
Le duodi 2 thermidor, an CCXXV, Michael Niedermayer a écrit :
> breaks on mips (probably big endian issue)
Breaks on MIPS, or detects that something is already broken in MIPS and
nobody noticed until now?
Could you perhaps re-run the test with V=1, then re-run the command line
with -f png, to see how the output is broken?
If the output of the filter is currently broken, then I think we should
apply this patch, because it would raise the incentive for fixing it.
Regards,
On Fri, Jul 21, 2017 at 01:04:03AM +0200, Nicolas George wrote: > Le duodi 2 thermidor, an CCXXV, Michael Niedermayer a écrit : > > breaks on mips (probably big endian issue) > > Breaks on MIPS, or detects that something is already broken in MIPS and > nobody noticed until now? this patch breaks the selftests on mips or maybe more precissely it adds a test which fails > > Could you perhaps re-run the test with V=1, then re-run the command line > with -f png, to see how the output is broken? Stream #0:0: Video: rawvideo ([12][0]3Y / 0x5933000C), yuv444p12be, 320x240 [SAR 1:1 DAR 4:3], q=2-31, 13824 kb/s, 5 fps, 5 tbn, 5 tbc (default) the format used seems the native one that has different endianness and thus different checksums md5sum out.png mips/out.png 012e1906fe84ee6f757ef3f1473cc26d out.png 012e1906fe84ee6f757ef3f1473cc26d mips/out.png i tried forcing le but it alone didnt give the same result ive to sleep now so i cant look further into this today but iam happy to test patches tomorrow, also iam just testing with mips qemu not real hw so if it wasnt for the one time work to set build env up and qemu than it would be rather easy for anyone to test [...]
On 21.07.2017 02:47, Michael Niedermayer wrote: > On Fri, Jul 21, 2017 at 01:04:03AM +0200, Nicolas George wrote: >> Le duodi 2 thermidor, an CCXXV, Michael Niedermayer a écrit : >>> breaks on mips (probably big endian issue) >> >> Breaks on MIPS, or detects that something is already broken in MIPS and >> nobody noticed until now? > > this patch breaks the selftests on mips or maybe more precissely > it adds a test which fails > > >> >> Could you perhaps re-run the test with V=1, then re-run the command line >> with -f png, to see how the output is broken? > > Stream #0:0: Video: rawvideo ([12][0]3Y / 0x5933000C), yuv444p12be, 320x240 [SAR 1:1 DAR 4:3], q=2-31, 13824 kb/s, 5 fps, 5 tbn, 5 tbc (default) > > the format used seems the native one that has different endianness > and thus different checksums > > md5sum out.png mips/out.png > 012e1906fe84ee6f757ef3f1473cc26d out.png > 012e1906fe84ee6f757ef3f1473cc26d mips/out.png > > i tried forcing le but it alone didnt give the same result > ive to sleep now so i cant look further into this today > but iam happy to test patches tomorrow, also iam just testing with > mips qemu not real hw so if it wasnt for the one time work to set > build env up and qemu than it would be rather easy for anyone to test I tested the opposite direction by forcing yuv444p12be on x86-64 and got different frame-CRCs "0xda6a937e" compared to the MIPS-CRCs you posted earlier "0xba079be7". Not sure what is happening ... Regards, Tobias
Le tridi 3 thermidor, an CCXXV, Michael Niedermayer a écrit : > this patch breaks the selftests on mips or maybe more precissely > it adds a test which fails Yes, I understand that. What I ask is who is responsible for the failure: the test or the code being tested? If somebody adds a test on a code that uses floats, it will fail on x86_32: the code is correct, the failure is the test's fault. If somebody adds a test on a code that has bogus SSE optimizations, it will fail on machines with SSE: the test is correct, the failure is the test's fault. What I am asking is: which one of these is it today? > the format used seems the native one that has different endianness > and thus different checksums > > md5sum out.png mips/out.png > 012e1906fe84ee6f757ef3f1473cc26d out.png > 012e1906fe84ee6f757ef3f1473cc26d mips/out.png md5sum tells us the files are identical, but it does not tell us if the output is correct. To see if the output is correct, I would suggest to convert it using a tool entirely different from FFmpeg (ImageMagick?) losslessly into a common format and compare. Plus, of course, visually checking the frames. The real question here is: on MIPS, is this filter's output correct? Regards,
On Fri, Jul 21, 2017 at 12:18:53PM +0200, Tobias Rapp wrote: > On 21.07.2017 02:47, Michael Niedermayer wrote: > >On Fri, Jul 21, 2017 at 01:04:03AM +0200, Nicolas George wrote: > >>Le duodi 2 thermidor, an CCXXV, Michael Niedermayer a écrit : > >>>breaks on mips (probably big endian issue) > >> > >>Breaks on MIPS, or detects that something is already broken in MIPS and > >>nobody noticed until now? > > > >this patch breaks the selftests on mips or maybe more precissely > >it adds a test which fails > > > > > >> > >>Could you perhaps re-run the test with V=1, then re-run the command line > >>with -f png, to see how the output is broken? > > > >Stream #0:0: Video: rawvideo ([12][0]3Y / 0x5933000C), yuv444p12be, 320x240 [SAR 1:1 DAR 4:3], q=2-31, 13824 kb/s, 5 fps, 5 tbn, 5 tbc (default) > > > >the format used seems the native one that has different endianness > >and thus different checksums > > > >md5sum out.png mips/out.png > >012e1906fe84ee6f757ef3f1473cc26d out.png > >012e1906fe84ee6f757ef3f1473cc26d mips/out.png > > > >i tried forcing le but it alone didnt give the same result > >ive to sleep now so i cant look further into this today > >but iam happy to test patches tomorrow, also iam just testing with > >mips qemu not real hw so if it wasnt for the one time work to set > >build env up and qemu than it would be rather easy for anyone to test > > I tested the opposite direction by forcing yuv444p12be on x86-64 and > got different frame-CRCs "0xda6a937e" compared to the MIPS-CRCs you > posted earlier "0xba079be7". Not sure what is happening ... mybe the le<->be is the BE output you get identical to LE ? (besides the endianness) [...]
On Fri, Jul 21, 2017 at 12:28:13PM +0200, Nicolas George wrote: > Le tridi 3 thermidor, an CCXXV, Michael Niedermayer a écrit : > > this patch breaks the selftests on mips or maybe more precissely > > it adds a test which fails > > Yes, I understand that. What I ask is who is responsible for the > failure: the test or the code being tested? > > If somebody adds a test on a code that uses floats, it will fail on > x86_32: the code is correct, the failure is the test's fault. > > If somebody adds a test on a code that has bogus SSE optimizations, it > will fail on machines with SSE: the test is correct, the failure is the > test's fault. > > What I am asking is: which one of these is it today? first the pixel format had wrong endianness (bug in the test) but theres seems to be a 2nd issue [...]
On 21.07.2017 13:16, Michael Niedermayer wrote: > On Fri, Jul 21, 2017 at 12:18:53PM +0200, Tobias Rapp wrote: >> On 21.07.2017 02:47, Michael Niedermayer wrote: >>> On Fri, Jul 21, 2017 at 01:04:03AM +0200, Nicolas George wrote: >>>> Le duodi 2 thermidor, an CCXXV, Michael Niedermayer a écrit : >>>>> breaks on mips (probably big endian issue) >>>> >>>> Breaks on MIPS, or detects that something is already broken in MIPS and >>>> nobody noticed until now? >>> >>> this patch breaks the selftests on mips or maybe more precissely >>> it adds a test which fails >>> >>> >>>> >>>> Could you perhaps re-run the test with V=1, then re-run the command line >>>> with -f png, to see how the output is broken? >>> >>> Stream #0:0: Video: rawvideo ([12][0]3Y / 0x5933000C), yuv444p12be, 320x240 [SAR 1:1 DAR 4:3], q=2-31, 13824 kb/s, 5 fps, 5 tbn, 5 tbc (default) >>> >>> the format used seems the native one that has different endianness >>> and thus different checksums >>> >>> md5sum out.png mips/out.png >>> 012e1906fe84ee6f757ef3f1473cc26d out.png >>> 012e1906fe84ee6f757ef3f1473cc26d mips/out.png >>> >>> i tried forcing le but it alone didnt give the same result >>> ive to sleep now so i cant look further into this today >>> but iam happy to test patches tomorrow, also iam just testing with >>> mips qemu not real hw so if it wasnt for the one time work to set >>> build env up and qemu than it would be rather easy for anyone to test >> >> I tested the opposite direction by forcing yuv444p12be on x86-64 and >> got different frame-CRCs "0xda6a937e" compared to the MIPS-CRCs you >> posted earlier "0xba079be7". Not sure what is happening ... > > mybe the le<->be > is the BE output you get identical to LE ? (besides the endianness) Finally managed to setup a MIPS cross-compiler plus qemu. Looking at the (verbose) log output of the command line indicates that the yuvtestsrc filter output is configured to yuv444p instead of the assumed yuv444p12be : $ qemu-mips -cpu 74Kf ./build-mips/ffmpeg-dbg -nostdin -nostats -cpuflags all -lavfi "yuvtestsrc=rate=5:duration=1" -pix_fmt yuv444p12le -flags +bitexact -fflags +bitexact -f framecrc - -loglevel verbose [...] [Parsed_yuvtestsrc_0 @ 0x1c99eb0] size:320x240 rate:5/1 duration:1.000000 sar:1/1 [auto_scaler_0 @ 0x1c9a550] w:iw h:ih flags:'bilinear' interl:0 [format @ 0x1c9a380] auto-inserting filter 'auto_scaler_0' between the filter 'Parsed_yuvtestsrc_0' and the filter 'format' [auto_scaler_0 @ 0x1c9a550] w:320 h:240 fmt:yuv444p sar:1/1 -> w:320 h:240 fmt:yuv444p12le sar:1/1 flags:0x2 [...] Not sure if this is "normal". After forcing the yuvtestsrc output to (native) yuv444p12 the tests succeeds on both platforms: CMD = framecrc -lavfi yuvtestsrc=rate=5:duration=1,format=yuv444p12 -pix_fmt yuv444p12le Will post a patch update. Regards, Tobias
--- tests/ref/fate/filter-yuvtestsrc-yuv444p12 2017-07-20 22:22:52.083416873 +0200 +++ tests/data/fate/filter-yuvtestsrc-yuv444p12 2017-07-20 22:42:07.043441205 +0200 @@ -3,8 +3,8 @@ #codec_id 0: rawvideo #dimensions 0: 320x240 #sar 0: 1/1 -0, 0, 0, 1, 460800, 0x3ec49be7 -0, 1, 1, 1, 460800, 0x3ec49be7 -0, 2, 2, 1, 460800, 0x3ec49be7 -0, 3, 3, 1, 460800, 0x3ec49be7 -0, 4, 4, 1, 460800, 0x3ec49be7 +0, 0, 0, 1, 460800, 0xba079be7 +0, 1, 1, 1, 460800, 0xba079be7 +0, 2, 2, 1, 460800, 0xba079be7 +0, 3, 3, 1, 460800, 0xba079be7 +0, 4, 4, 1, 460800, 0xba079be7