DanBloomberg / leptonica

Leptonica is an open source library containing software that is broadly useful for image processing and image analysis applications. The official github repository for Leptonica is: danbloomberg/leptonica. See leptonica.org for more documentation.
Other
1.81k stars 393 forks source link

Fix function pixReadMemBmp for big endian hosts #752

Closed stweil closed 1 month ago

stweil commented 1 month ago

Simplify also the code a little bit and remove a comment with test code.

convertOnBigEnd32 must not be called for ihbytes because it is already assigned the right value which is read from bmpih in little endian order.

stweil commented 1 month ago

I still get a failure for maze_reg on sparc64, but all other tests now pass. And there are still compiler warnings like this one when I build on macOS:

../../../../src/bmpio.c:616:5: warning: taking address of packed member 'bmpih' of class or structure 'BMP_HEADER' may result in an unaligned pointer value [-Waddress-of-packed-member]
  616 |     bmpih->biClrImportant = convertOnBigEnd32(ncolors);
      |     ^~~~~
../../../../src/bmpio.c:478:17: note: expanded from macro 'bmpih'

But those issues can be addressed in another pull request.

DanBloomberg commented 1 month ago

Thank you for simplifying & fixing it. What is the maze_reg failure?

stweil commented 1 month ago

Failure on SPARC64 / Solaris:

////////////////////////////////////////////////
////////////////   maze_reg   ///////////////
////////////////////////////////////////////////
leptonica-1.84.2 : libjpeg 9e : libpng 1.6.40 : libtiff 4.6.0 : zlib 1.2.13 : libwebp 1.3.2 : libopenjp2 2.5.2
Info in pixSearchBinaryMaze:  No path found
Error in pixDisplayPta: pta not defined
Error in pixScaleBySampling: pixs not defined
Error in pixaAddPix: pix not defined
Error in regTestWritePixAndCheck: pix not defined
Time:   0.557 sec
DanBloomberg commented 1 month ago

I do not know why it didn't find a path.

Question: could this be an endian issue, or is it specific to sparc64? Are there any other big-endian machines we test on? (e.g., is Apple big-endian?)

stweil commented 1 month ago

No, Apple is little endian. But I have some more big endian machines (PowerPC, MIPS64) and can try those.

stweil commented 1 month ago

MIPS64 fails for a different reason (not only maze_reg, but many other tests, too, require bmp format):

////////////////////////////////////////////////
////////////////   maze_reg   ///////////////
////////////////////////////////////////////////
leptonica-1.84.2 : libjpeg 6b (libjpeg-turbo 1.5.2) : zlib 1.2.11
Info in pixSearchBinaryMaze:  Path found
Warning in changeFormatForMissingLib: png library missing; output bmp format
Error in pixWriteStreamBmp: Write error
Error in findFileFormatStream: truncated file
Error in fopenReadStream: failed to open locally with tail maze_golden.00.bmp for filename /tmp/lept/golden/maze_golden.00.bmp
Leptonica Error in nbytesInFile: stream not opened: /tmp/lept/golden/maze_golden.00.bmp
Error in fopenReadStream: failed to open locally with tail maze_golden.00.bmp for filename /tmp/lept/golden/maze_golden.00.bmp
Leptonica Error in l_binaryRead: file stream not opened: /tmp/lept/golden/maze_golden.00.bmp
Error in filesAreIdentical: array2 not read
Failure in maze_reg, index 0: comparing /tmp/lept/regout/maze.00.bmp with /tmp/lept/golden/maze_golden.00.bmp
Warning in changeFormatForMissingLib: png library missing; output bmp format
Error in pixWriteStreamBmp: Write error
Error in findFileFormatStream: truncated file
Error in fopenReadStream: failed to open locally with tail maze_golden.01.bmp for filename /tmp/lept/golden/maze_golden.01.bmp
Leptonica Error in nbytesInFile: stream not opened: /tmp/lept/golden/maze_golden.01.bmp
Error in fopenReadStream: failed to open locally with tail maze_golden.01.bmp for filename /tmp/lept/golden/maze_golden.01.bmp
Leptonica Error in l_binaryRead: file stream not opened: /tmp/lept/golden/maze_golden.01.bmp
Error in filesAreIdentical: array2 not read
Failure in maze_reg, index 1: comparing /tmp/lept/regout/maze.01.bmp with /tmp/lept/golden/maze_golden.01.bmp
Warning in changeFormatForMissingLib: png library missing; output bmp format
Error in pixWriteStreamBmp: Write error
Error in findFileFormatStream: truncated file
Error in fopenReadStream: failed to open locally with tail maze_golden.02.bmp for filename /tmp/lept/golden/maze_golden.02.bmp
Leptonica Error in nbytesInFile: stream not opened: /tmp/lept/golden/maze_golden.02.bmp
Error in fopenReadStream: failed to open locally with tail maze_golden.02.bmp for filename /tmp/lept/golden/maze_golden.02.bmp
Leptonica Error in l_binaryRead: file stream not opened: /tmp/lept/golden/maze_golden.02.bmp
Error in filesAreIdentical: array2 not read
Failure in maze_reg, index 2: comparing /tmp/lept/regout/maze.02.bmp with /tmp/lept/golden/maze_golden.02.bmp
Time:   2.560 sec
DanBloomberg commented 1 month ago

I don't know what causes the truncation in the written bmp file. Adding a small diagnostic in bmpio.c that gives the amount of truncation when it occurs.

stweil commented 1 month ago

nbytes is always 0:

Error in pixWriteStreamBmp: Truncation: nbytes = 0, size = 1080054
stweil commented 1 month ago

I don't think that the error is an endianess issue. The error seems to be caused by a full filesystem for /tmp which can only store 100 MB on the test machine. The Leptonica tests consume this little free space very fast.

stweil commented 1 month ago

All tests pass on a big endian PowerPC64 (tests which require gnuplot were skipped):

============================================================================
Testsuite summary for leptonica 1.84.2
============================================================================
# TOTAL: 148
# PASS:  123
# SKIP:  25
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
DanBloomberg commented 1 month ago

Thank you for doing all the tests, Stefan.

So it appears that we are good on all platforms, except for failure to find a 1 bpp path in maze_reg on sparc64/solaris. And whatever causes that failure, it is not endian-related.