libjxl icon indicating copy to clipboard operation
libjxl copied to clipboard

Some tests are failing on big-endian arch

Open malaterre opened this issue 3 years ago • 4 comments

Describe the bug

The following tests are consistently failing on big-endian arches:

	656 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBAtoRGBAf16#GetParam()=301x33RGBAtoRGBAf16 (Failed)
	662 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBAtoRGBAf16Callback#GetParam()=301x33RGBAtoRGBAf16Callback (Failed)
	668 - DecodeTest/DecodeTestParam.PixelTest/301x33GtoGf16#GetParam()=301x33GtoGf16 (Failed)
	674 - DecodeTest/DecodeTestParam.PixelTest/301x33GtoGf16Callback#GetParam()=301x33GtoGf16Callback (Failed)
	680 - DecodeTest/DecodeTestParam.PixelTest/301x33GAtoGf16#GetParam()=301x33GAtoGf16 (Failed)
	686 - DecodeTest/DecodeTestParam.PixelTest/301x33GAtoGf16Callback#GetParam()=301x33GAtoGf16Callback (Failed)
	692 - DecodeTest/DecodeTestParam.PixelTest/301x33GAtoGAf16#GetParam()=301x33GAtoGAf16 (Failed)
	698 - DecodeTest/DecodeTestParam.PixelTest/301x33GAtoGAf16Callback#GetParam()=301x33GAtoGAf16Callback (Failed)
	704 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBtoRGBf16#GetParam()=301x33RGBtoRGBf16 (Failed)
	710 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBtoRGBf16Callback#GetParam()=301x33RGBtoRGBf16Callback (Failed)
	716 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBAtoRGBf16#GetParam()=301x33RGBAtoRGBf16 (Failed)
	722 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBAtoRGBf16Callback#GetParam()=301x33RGBAtoRGBf16Callback (Failed)
	728 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBtoRGBAf16#GetParam()=301x33RGBtoRGBAf16 (Failed)
	734 - DecodeTest/DecodeTestParam.PixelTest/301x33RGBtoRGBAf16Callback#GetParam()=301x33RGBtoRGBAf16Callback (Failed)
	963 - RobustStatisticsTest.CleanLine (Failed)

To Reproduce

  • https://buildd.debian.org/status/fetch.php?pkg=jpeg-xl&arch=s390x&ver=0.6.1%2Bds-5&stamp=1639585115&raw=0
  • https://buildd.debian.org/status/fetch.php?pkg=jpeg-xl&arch=ppc64&ver=0.6.1%2Bds-5&stamp=1639586654&raw=0

Expected behavior

It would be nice if tests would report consistent results across arch endianess.

Environment

  • OS: Linux
  • Compiler version: gcc-11
  • CPU type: ppc64 & s390x
  • cjxl/djxl version string: 0.6.1

malaterre avatar Dec 17 '21 08:12 malaterre

Looks like there's an issue with the byte order of FLOAT16 on big-endian architectures...

Can you check if the problem still happens with the current git version?

jonsneyers avatar Dec 17 '21 15:12 jonsneyers

Here is the status as of git 20220120 (commit 0647da4)

  • https://buildd.debian.org/status/fetch.php?pkg=jpeg-xl&arch=s390x&ver=0.7.0%7Egit20220120.0647da4%2Bds-4&stamp=1643635642&raw=0

malaterre avatar Feb 07 '22 13:02 malaterre

For reference, I also see https://github.com/libjxl/libjxl/issues/518

malaterre avatar Mar 31 '22 06:03 malaterre

@eustas I see milestone "1.0" is this accurate ? or are you planning on fixing test suite on big-endian arch also for 0.7 release ?

Eg:

  • https://buildd.debian.org/status/fetch.php?pkg=jpeg-xl&arch=ppc64&ver=0.7.0%7Egit20220805.980c90f-3&stamp=1661438440&raw=0

malaterre avatar Aug 25 '22 14:08 malaterre

This endianess issue is causing more and more integration problems, eg. GDAL plugin:

  • https://salsa.debian.org/debian-gis-team/gdal/-/merge_requests/5#note_355498

malaterre avatar Nov 24 '22 08:11 malaterre

@szabadka The following test is failing on big-endian arch: ModularTest.PredictorIntegerOverflow. Looking at the code:

58fff0336 lib/jxl/modular_test.cc (Zoltan Szabadka     2022-07-11 11:19:15 +0200 465)     // After UnpackSigned this becomes (1 << 31) - 1, the largest pixel_type,
58fff0336 lib/jxl/modular_test.cc (Zoltan Szabadka     2022-07-11 11:19:15 +0200 466)     // and after adding the offset we get -(1 << 31).
58fff0336 lib/jxl/modular_test.cc (Zoltan Szabadka     2022-07-11 11:19:15 +0200 467)     bw->Write(8, 119);
58fff0336 lib/jxl/modular_test.cc (Zoltan Szabadka     2022-07-11 11:19:15 +0200 468)     bw->Write(28, 0xfffffff);
58fff0336 lib/jxl/modular_test.cc (Zoltan Szabadka     2022-07-11 11:19:15 +0200 469)     bw->ZeroPadToByte();

could you confirm this is portable on big-endian arch ?

malaterre avatar Jan 03 '23 08:01 malaterre

For later reference, I had to cherry-pick commit de08116d14db785431f3efb651dcf2af15bbb682 onto the 0.7.x branch to get all test to pass on big-endian arches:

  • https://buildd.debian.org/status/logs.php?pkg=jpeg-xl&ver=0.7.0-8&suite=sid

malaterre avatar Jan 04 '23 15:01 malaterre