From 8bbce0a2a2039fbb253eee3b87453b4351989601 Mon Sep 17 00:00:00 2001 From: Stuart Cunningham Date: Wed, 12 Feb 2014 21:21:06 +1100 Subject: [PATCH 1/4] Fix reading of 16-bit TIFF images on big endian host. Use correct integer types for arguments to TIFFGetField to avoid corruption of values and failed loads of TIFF file when using cv::imread(). Added test where both big and little endian TIFF files are read using imread(). Fixed build of 3rdparty libtiff on big endian hosts. Reduced memory required during decode_tile16384x16384 test by not converting large grayscale test image to color image during read. --- 3rdparty/libtiff/CMakeLists.txt | 2 ++ 3rdparty/libtiff/tif_config.h.cmakein | 2 +- modules/highgui/src/grfmt_tiff.cpp | 15 ++++---- modules/highgui/test/test_grfmt.cpp | 52 +++++++++++++++++++++++++-- 4 files changed, 62 insertions(+), 9 deletions(-) diff --git a/3rdparty/libtiff/CMakeLists.txt b/3rdparty/libtiff/CMakeLists.txt index addbb5551c..5793021b78 100644 --- a/3rdparty/libtiff/CMakeLists.txt +++ b/3rdparty/libtiff/CMakeLists.txt @@ -6,6 +6,7 @@ project(${TIFF_LIBRARY}) include(CheckFunctionExists) include(CheckIncludeFile) +include(TestBigEndian) check_include_file(assert.h HAVE_ASSERT_H) check_include_file(fcntl.h HAVE_FCNTL_H) @@ -16,6 +17,7 @@ check_include_file(search.h HAVE_SEARCH_H) check_include_file(string.h HAVE_STRING_H) check_include_file(sys/types.h HAVE_SYS_TYPES_H) check_include_file(unistd.h HAVE_UNISTD_H) +test_big_endian(HOST_BIGENDIAN) if(WIN32 AND NOT HAVE_WINRT) set(USE_WIN32_FILEIO 1) diff --git a/3rdparty/libtiff/tif_config.h.cmakein b/3rdparty/libtiff/tif_config.h.cmakein index 182f2833d1..55c6cb26a0 100644 --- a/3rdparty/libtiff/tif_config.h.cmakein +++ b/3rdparty/libtiff/tif_config.h.cmakein @@ -54,7 +54,7 @@ /* Native cpu byte order: 1 if big-endian (Motorola) or 0 if little-endian (Intel) */ -#define HOST_BIGENDIAN 0 +#define HOST_BIGENDIAN @HOST_BIGENDIAN@ /* Set the native cpu bit order (FILLORDER_LSB2MSB or FILLORDER_MSB2LSB) */ #define HOST_FILLORDER FILLORDER_LSB2MSB diff --git a/modules/highgui/src/grfmt_tiff.cpp b/modules/highgui/src/grfmt_tiff.cpp index 5179531f50..c86b4e3654 100644 --- a/modules/highgui/src/grfmt_tiff.cpp +++ b/modules/highgui/src/grfmt_tiff.cpp @@ -111,18 +111,21 @@ bool TiffDecoder::readHeader() bool result = false; close(); - TIFF* tif = TIFFOpen( m_filename.c_str(), "rb" ); + // TIFFOpen() mode flags are different to fopen(). A 'b' in mode "rb" has no effect when reading. + // http://www.remotesensing.org/libtiff/man/TIFFOpen.3tiff.html + TIFF* tif = TIFFOpen( m_filename.c_str(), "r" ); if( tif ) { - int wdth = 0, hght = 0, photometric = 0; + uint wdth = 0, hght = 0; + ushort photometric = 0; m_tif = tif; if( TIFFGetField( tif, TIFFTAG_IMAGEWIDTH, &wdth ) && TIFFGetField( tif, TIFFTAG_IMAGELENGTH, &hght ) && TIFFGetField( tif, TIFFTAG_PHOTOMETRIC, &photometric )) { - int bpp=8, ncn = photometric > 1 ? 3 : 1; + ushort bpp=8, ncn = photometric > 1 ? 3 : 1; TIFFGetField( tif, TIFFTAG_BITSPERSAMPLE, &bpp ); TIFFGetField( tif, TIFFTAG_SAMPLESPERPIXEL, &ncn ); @@ -175,12 +178,12 @@ bool TiffDecoder::readData( Mat& img ) if( m_tif && m_width && m_height ) { TIFF* tif = (TIFF*)m_tif; - int tile_width0 = m_width, tile_height0 = 0; + uint tile_width0 = m_width, tile_height0 = 0; int x, y, i; int is_tiled = TIFFIsTiled(tif); - int photometric; + ushort photometric; TIFFGetField( tif, TIFFTAG_PHOTOMETRIC, &photometric ); - int bpp = 8, ncn = photometric > 1 ? 3 : 1; + ushort bpp = 8, ncn = photometric > 1 ? 3 : 1; TIFFGetField( tif, TIFFTAG_BITSPERSAMPLE, &bpp ); TIFFGetField( tif, TIFFTAG_SAMPLESPERPIXEL, &ncn ); const int bitsPerByte = 8; diff --git a/modules/highgui/test/test_grfmt.cpp b/modules/highgui/test/test_grfmt.cpp index 86954e3e10..edccc0280a 100644 --- a/modules/highgui/test/test_grfmt.cpp +++ b/modules/highgui/test/test_grfmt.cpp @@ -408,8 +408,8 @@ TEST(Highgui_Tiff, decode_tile16384x16384) try { - cv::imread(file3); - EXPECT_NO_THROW(cv::imread(file4)); + cv::imread(file3, CV_LOAD_IMAGE_UNCHANGED); + EXPECT_NO_THROW(cv::imread(file4, CV_LOAD_IMAGE_UNCHANGED)); } catch(const std::bad_alloc&) { @@ -419,4 +419,52 @@ TEST(Highgui_Tiff, decode_tile16384x16384) remove(file3.c_str()); remove(file4.c_str()); } + +TEST(Highgui_Tiff, write_read_16bit_big_little_endian) +{ + // see issue #2601 "16-bit Grayscale TIFF Load Failures Due to Buffer Underflow and Endianness" + + // Setup data for two minimal 16-bit grayscale TIFF files in both endian formats + uchar tiff_sample_data[2][86] = { { + // Little endian + 0x49, 0x49, 0x2a, 0x00, 0x0c, 0x00, 0x00, 0x00, 0xad, 0xde, 0xef, 0xbe, 0x06, 0x00, 0x00, 0x01, + 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0x01, 0x03, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x01, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00, + 0x00, 0x00, 0x06, 0x01, 0x03, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x11, 0x01, + 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x17, 0x01, 0x04, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x04, 0x00, 0x00, 0x00 }, { + // Big endian + 0x4d, 0x4d, 0x00, 0x2a, 0x00, 0x00, 0x00, 0x0c, 0xde, 0xad, 0xbe, 0xef, 0x00, 0x06, 0x01, 0x00, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00, 0x01, 0x01, 0x00, 0x03, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x01, 0x02, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x10, + 0x00, 0x00, 0x01, 0x06, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x01, 0x11, + 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x01, 0x17, 0x00, 0x04, 0x00, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x04 } + }; + + // Test imread() for both a little endian TIFF and big endian TIFF + for (int i = 0; i < 2; i++) + { + string filename = cv::tempfile(".tiff"); + + // Write sample TIFF file + FILE* fp = fopen(filename.c_str(), "wb"); + ASSERT_TRUE(fp != NULL); + ASSERT_EQ((size_t)1, fwrite(tiff_sample_data, 86, 1, fp)); + fclose(fp); + + Mat img = imread(filename, CV_LOAD_IMAGE_UNCHANGED); + + EXPECT_EQ(1, img.rows); + EXPECT_EQ(2, img.cols); + EXPECT_EQ(CV_16U, img.type()); + EXPECT_EQ(sizeof(ushort), img.elemSize()); + EXPECT_EQ(1, img.channels()); + EXPECT_EQ(0xDEAD, img.at(0,0)); + EXPECT_EQ(0xBEEF, img.at(0,1)); + + remove(filename.c_str()); + } +} + #endif From 5d43d3ca882d4e083fcf24484744f98346263f26 Mon Sep 17 00:00:00 2001 From: Stuart Cunningham Date: Wed, 12 Feb 2014 23:41:38 +1100 Subject: [PATCH 2/4] Fix Windows build of grfmt_tiff.cpp by using libtiff's integer types. --- modules/highgui/src/grfmt_tiff.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/highgui/src/grfmt_tiff.cpp b/modules/highgui/src/grfmt_tiff.cpp index c86b4e3654..2d976c2d0c 100644 --- a/modules/highgui/src/grfmt_tiff.cpp +++ b/modules/highgui/src/grfmt_tiff.cpp @@ -117,15 +117,15 @@ bool TiffDecoder::readHeader() if( tif ) { - uint wdth = 0, hght = 0; - ushort photometric = 0; + uint32 wdth = 0, hght = 0; + uint16 photometric = 0; m_tif = tif; if( TIFFGetField( tif, TIFFTAG_IMAGEWIDTH, &wdth ) && TIFFGetField( tif, TIFFTAG_IMAGELENGTH, &hght ) && TIFFGetField( tif, TIFFTAG_PHOTOMETRIC, &photometric )) { - ushort bpp=8, ncn = photometric > 1 ? 3 : 1; + uint16 bpp=8, ncn = photometric > 1 ? 3 : 1; TIFFGetField( tif, TIFFTAG_BITSPERSAMPLE, &bpp ); TIFFGetField( tif, TIFFTAG_SAMPLESPERPIXEL, &ncn ); @@ -178,12 +178,12 @@ bool TiffDecoder::readData( Mat& img ) if( m_tif && m_width && m_height ) { TIFF* tif = (TIFF*)m_tif; - uint tile_width0 = m_width, tile_height0 = 0; + uint32 tile_width0 = m_width, tile_height0 = 0; int x, y, i; int is_tiled = TIFFIsTiled(tif); - ushort photometric; + uint16 photometric; TIFFGetField( tif, TIFFTAG_PHOTOMETRIC, &photometric ); - ushort bpp = 8, ncn = photometric > 1 ? 3 : 1; + uint16 bpp = 8, ncn = photometric > 1 ? 3 : 1; TIFFGetField( tif, TIFFTAG_BITSPERSAMPLE, &bpp ); TIFFGetField( tif, TIFFTAG_SAMPLESPERPIXEL, &ncn ); const int bitsPerByte = 8; From 55b9c0374c943bd5d7224190bed8d3228dd0d792 Mon Sep 17 00:00:00 2001 From: Stuart Cunningham Date: Thu, 13 Feb 2014 22:59:30 +1100 Subject: [PATCH 3/4] Fix cmake detection of build platform endianness Improve comments to indicate actual usage of WORDS_BIGENDIAN where it is tested with #ifdef rather than #if --- 3rdparty/libtiff/CMakeLists.txt | 2 -- 3rdparty/libtiff/tif_config.h.cmakein | 14 +++----------- CMakeLists.txt | 6 ++++++ cmake/templates/cvconfig.h.in | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/3rdparty/libtiff/CMakeLists.txt b/3rdparty/libtiff/CMakeLists.txt index 5793021b78..addbb5551c 100644 --- a/3rdparty/libtiff/CMakeLists.txt +++ b/3rdparty/libtiff/CMakeLists.txt @@ -6,7 +6,6 @@ project(${TIFF_LIBRARY}) include(CheckFunctionExists) include(CheckIncludeFile) -include(TestBigEndian) check_include_file(assert.h HAVE_ASSERT_H) check_include_file(fcntl.h HAVE_FCNTL_H) @@ -17,7 +16,6 @@ check_include_file(search.h HAVE_SEARCH_H) check_include_file(string.h HAVE_STRING_H) check_include_file(sys/types.h HAVE_SYS_TYPES_H) check_include_file(unistd.h HAVE_UNISTD_H) -test_big_endian(HOST_BIGENDIAN) if(WIN32 AND NOT HAVE_WINRT) set(USE_WIN32_FILEIO 1) diff --git a/3rdparty/libtiff/tif_config.h.cmakein b/3rdparty/libtiff/tif_config.h.cmakein index 55c6cb26a0..d46761b525 100644 --- a/3rdparty/libtiff/tif_config.h.cmakein +++ b/3rdparty/libtiff/tif_config.h.cmakein @@ -54,7 +54,7 @@ /* Native cpu byte order: 1 if big-endian (Motorola) or 0 if little-endian (Intel) */ -#define HOST_BIGENDIAN @HOST_BIGENDIAN@ +#define HOST_BIGENDIAN @WORDS_BIGENDIAN@ /* Set the native cpu bit order (FILLORDER_LSB2MSB or FILLORDER_MSB2LSB) */ #define HOST_FILLORDER FILLORDER_LSB2MSB @@ -154,17 +154,9 @@ /* define to use win32 IO system */ #cmakedefine USE_WIN32_FILEIO -/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most +/* Define WORDS_BIGENDIAN if your processor stores words with the most significant byte first (like Motorola and SPARC, unlike Intel). */ -#if defined AC_APPLE_UNIVERSAL_BUILD -# if defined __BIG_ENDIAN__ -# define WORDS_BIGENDIAN 1 -# endif -#else -# ifndef WORDS_BIGENDIAN -/* # undef WORDS_BIGENDIAN */ -# endif -#endif +#cmakedefine WORDS_BIGENDIAN /* Support Deflate compression */ #define ZIP_SUPPORT 1 diff --git a/CMakeLists.txt b/CMakeLists.txt index c6518cfc0e..6489517659 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -444,6 +444,12 @@ endif() include(cmake/OpenCVPCHSupport.cmake) include(cmake/OpenCVModule.cmake) +# ---------------------------------------------------------------------------- +# Detect endianness of build platform +# ---------------------------------------------------------------------------- +include(TestBigEndian) +test_big_endian(WORDS_BIGENDIAN) + # ---------------------------------------------------------------------------- # Detect 3rd-party libraries # ---------------------------------------------------------------------------- diff --git a/cmake/templates/cvconfig.h.in b/cmake/templates/cvconfig.h.in index a6cee63684..d1c9e65d3d 100644 --- a/cmake/templates/cvconfig.h.in +++ b/cmake/templates/cvconfig.h.in @@ -161,6 +161,6 @@ /* Xine video library */ #cmakedefine HAVE_XINE -/* Define to 1 if your processor stores words with the most significant byte +/* Define if your processor stores words with the most significant byte first (like Motorola and SPARC, unlike Intel and VAX). */ #cmakedefine WORDS_BIGENDIAN From 1454843b81f642ca25716997522c6630f6bb624f Mon Sep 17 00:00:00 2001 From: Stuart Cunningham Date: Fri, 14 Feb 2014 16:16:17 +1100 Subject: [PATCH 4/4] Fix build of libtiff on big endian host due to defined but empty WORDS_BIGENDIAN macro --- 3rdparty/libtiff/tif_config.h.cmakein | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/3rdparty/libtiff/tif_config.h.cmakein b/3rdparty/libtiff/tif_config.h.cmakein index d46761b525..24f58119ba 100644 --- a/3rdparty/libtiff/tif_config.h.cmakein +++ b/3rdparty/libtiff/tif_config.h.cmakein @@ -154,9 +154,9 @@ /* define to use win32 IO system */ #cmakedefine USE_WIN32_FILEIO -/* Define WORDS_BIGENDIAN if your processor stores words with the most +/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most significant byte first (like Motorola and SPARC, unlike Intel). */ -#cmakedefine WORDS_BIGENDIAN +#cmakedefine WORDS_BIGENDIAN 1 /* Support Deflate compression */ #define ZIP_SUPPORT 1