Opened 7 years ago

Closed 7 years ago

#6555 closed defect (fixed)

fate-filter-pixfmts-scale test fails on big-endian systems

Reported by: James Cowgill Owned by:
Priority: normal Component: swscale
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
The "fate-filter-pixfmts-scale" test fails on all big endian systems.

The test generates different output for the "gbrap16be" and "gbrap16le" pixel formats.

TEST    filter-pixfmts-scale
--- /build/upstream/tests/ref/fate/filter-pixfmts-scale 2017-07-28 16:03:50.083186073 +0000
+++ tests/data/fate/filter-pixfmts-scale        2017-07-28 16:07:47.674282702 +0000
@@ -23,8 +23,8 @@
 gbrap10le           cf974e23f485a10740f5de74a5c8c3df
 gbrap12be           1d9b57766ba9c2192403f43967cb9af0
 gbrap12le           bb1ba1c157717db3dd612a76d38a018e
-gbrap16be           81542b96575d1fe3b239d23899f5ece3
-gbrap16le           6feb8b9da131917abe867e0eaaf07b90
+gbrap16be           a0ef6e8e79fa685fbbf9ebd6f99fe624
+gbrap16le           3d99c63ce971b645b77995017a99ac5e
 gbrp                dc3387f925f972c61aae7eb23cdc19f0
 gbrp10be            0277d4c3a8498d75e2783fb81379e481
 gbrp10le            f3d70f8ab845c3c9b8f7452e4a6e285a
Test filter-pixfmts-scale failed. Look at tests/data/fate/filter-pixfmts-scale.err for details.

Running the test on an x86 (good) machine and mipsbe (bad) machine and comparing the results gives a diff which looks like this (--- x86, +++ mipsbe):

-0001d5b0: 0c87 5886 9300 7000 8e00 7000 8e00 7000  ..X...p...p...p.
-0001d5c0: 8e00 7000 8e00 7000 8e00 7000 8e00 7000  ..p...p...p...p.
-0001d5d0: 8e00 7000 8e00 7000 8e00 7000 8e00 7000  ..p...p...p...p.
-0001d5e0: 8e00 7000 8e00 7000 8e00 7000 8e00 7000  ..p...p...p...p.
-0001d5f0: 8e00 7000 8e00 7000 8e00 7000 8e00 7000  ..p...p...p...p.
-0001d600: 8e00 7000 8e00 7000 8e00 7000 8e00 7000  ..p...p...p...p.

+0001d5b0: 0c87 5886 9300 8e00 7000 8e00 7000 8e00  ..X.....p...p...
+0001d5c0: 7000 8e00 7000 8e00 7000 8e00 7000 8e00  p...p...p...p...
+0001d5d0: 7000 8e00 7000 8e00 7000 8e00 7000 8e00  p...p...p...p...
+0001d5e0: 7000 8e00 7000 8e00 7000 8e00 7000 8e00  p...p...p...p...
+0001d5f0: 7000 8e00 7000 8e00 7000 8e00 7000 8e00  p...p...p...p...
+0001d600: 7000 8e00 7000 8e00 7000 8e00 7000 8e00  p...p...p...p...

This continues for the entire alpha channel. It's not byteswapped, but each pixel is swapped with the next one.

The test started failing after this commit which added these pixel formats:
https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/6427c9ffee347306d0b00ef94d8cac70babc530c

I also tried applying that commit onto 3.2 and the test also gave the same results, so either the underlying bug is also present in 3.2, or the above commit forgot to do something it should have.

Change History (3)

comment:1 by James, 7 years ago

Status: newopen

PowerPC BE: http://fate.ffmpeg.org/history.cgi?slot=ppc64be-RHEL7.0-gcc-4.8.5-ibmcrl

Seeing that only the scale test fails, it could mean the bug is in the filter rather than libswscale.

comment:2 by James Cowgill, 7 years ago

Found a fix. I think the bug was that alpSrc needed to be cast to an int32_t ** first. I can submit a proper patch tomorrow, but if someone else wants to then please go ahead.

--- a/libswscale/output.c
+++ b/libswscale/output.c
@@ -2026,16 +2026,17 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t *lumFilter,
                     const int16_t **lumSrcx, int lumFilterSize,
                     const int16_t *chrFilter, const int16_t **chrUSrcx,
                     const int16_t **chrVSrcx, int chrFilterSize,
-                    const int16_t **alpSrc, uint8_t **dest,
+                    const int16_t **alpSrcx, uint8_t **dest,
                     int dstW, int y)
 {
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(c->dstFormat);
     int i;
-    int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrc;
+    int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrcx;
     uint16_t **dest16 = (uint16_t**)dest;
     const int32_t **lumSrc  = (const int32_t**)lumSrcx;
     const int32_t **chrUSrc = (const int32_t**)chrUSrcx;
     const int32_t **chrVSrc = (const int32_t**)chrVSrcx;
+    const int32_t **alpSrc = (const int32_t**)alpSrcx;
     int A = 0; // init to silence warning
 
     for (i = 0; i < dstW; i++) {
diff --git a/tests/ref/fate/filter-pixfmts-scale b/tests/ref/fate/filter-pixfmts-scale
index 6ab39aea..3232476a 100644
--- a/tests/ref/fate/filter-pixfmts-scale
+++ b/tests/ref/fate/filter-pixfmts-scale
@@ -23,8 +23,8 @@ gbrap10be           6d89abb9248006c3e9017545e9474654
 gbrap10le           cf974e23f485a10740f5de74a5c8c3df
 gbrap12be           1d9b57766ba9c2192403f43967cb9af0
 gbrap12le           bb1ba1c157717db3dd612a76d38a018e
-gbrap16be           81542b96575d1fe3b239d23899f5ece3
-gbrap16le           6feb8b9da131917abe867e0eaaf07b90
+gbrap16be           38d45837954fdd0115a854dde3e5f91e
+gbrap16le           76061a0dfc883449aebec1aaac188b0f
 gbrp                dc3387f925f972c61aae7eb23cdc19f0
 gbrp10be            0277d4c3a8498d75e2783fb81379e481
 gbrp10le            f3d70f8ab845c3c9b8f7452e4a6e285a

comment:3 by James, 7 years ago

Component: undeterminedswscale
Resolution: fixed
Status: openclosed
Note: See TracTickets for help on using tickets.