From 32080eb949ed7d090a89d661fc842d215981b070 Mon Sep 17 00:00:00 2001 From: pbkx <93405617+pbkx@users.noreply.github.com> Date: Thu, 10 Sep 2026 22:56:54 -0700 Subject: [PATCH] Merge pull request #29894 from pbkx:fix-sunras-encoding-checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit imgcodecs: fix Sun Raster encoding checks 🤖🤖🤖 - #29894 SunRasterDecoder saves the header's ras_type field in m_encoding but several validation and decoding checks compared RAS_BYTE_ENCODED and RAS_FORMAT_RGB against m_type. This caused valid byte-encoded and RGB-format Sun Raster images to be rejected before their existing decoding paths could be used. This patch uses the file encoding to make these decisions and limits byte encoding to 8-bit input and RGB-format input to the supported 24- and 32-bit paths. It also selects channel conversion from the file and requested output orders and avoids indexed-palette conversion when decoding truecolor inputs as grayscale. The regression tests use Sun Raster files written by Netpbm's `pnmtorast -rle` and ImageMagick's SUN encoder. The matching `opencv_extra` PR (opencv/opencv_extra#1408) contains these files and lets them be inspected independently with compatible image viewers. The tests verify exact RLE literal and run values, truncated RLE input, 24- and 32-bit RGB-format input, BGR and RGB output, and grayscale conversion. The Netpbm file also exposed an existing row-padding error in the RLE path: the decoder consumed a padding byte after even-width rows, although those rows require no padding. The patch now consumes that byte only for odd-width rows. No public API is changed. ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake --- modules/imgcodecs/src/grfmt_sunras.cpp | 23 +++--- modules/imgcodecs/test/test_sunraster.cpp | 95 +++++++++++++++++++++++ 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/modules/imgcodecs/src/grfmt_sunras.cpp b/modules/imgcodecs/src/grfmt_sunras.cpp index 2fc19de57f..78ab1bd502 100644 --- a/modules/imgcodecs/src/grfmt_sunras.cpp +++ b/modules/imgcodecs/src/grfmt_sunras.cpp @@ -80,7 +80,8 @@ bool SunRasterDecoder::readHeader() if( m_width > 0 && m_height > 0 && (m_bpp == 1 || m_bpp == 8 || m_bpp == 24 || m_bpp == 32) && (m_encoding == RAS_OLD || m_encoding == RAS_STANDARD || - (m_type == RAS_BYTE_ENCODED && m_bpp == 8) || m_type == RAS_FORMAT_RGB) && + (m_encoding == RAS_BYTE_ENCODED && m_bpp == 8) || + (m_encoding == RAS_FORMAT_RGB && (m_bpp == 24 || m_bpp == 32))) && ((m_maptype == RMT_NONE && m_maplength == 0) || (m_maptype == RMT_EQUAL_RGB && m_maplength <= palSize && m_maplength > 0 && m_bpp <= 8))) { @@ -141,6 +142,9 @@ bool SunRasterDecoder::readHeader() bool SunRasterDecoder::readData( Mat& img ) { bool color = img.channels() > 1; + const bool source_is_rgb = m_encoding == RAS_FORMAT_RGB; + const bool output_is_rgb = m_use_rgb; + const bool swap_blue_and_red = source_is_rgb != output_is_rgb; uchar* data = img.ptr(); size_t step = img.step; uchar gray_palette[256] = {0}; @@ -159,7 +163,7 @@ bool SunRasterDecoder::readData( Mat& img ) AutoBuffer _src(src_pitch + 32); uchar* src = _src.data(); - if( !color ) + if( !color && m_bpp <= 8 ) CvtPaletteToGray( m_palette, gray_palette, 1 << m_bpp ); try @@ -170,7 +174,7 @@ bool SunRasterDecoder::readData( Mat& img ) { /************************* 1 BPP ************************/ case 1: - if( m_type != RAS_BYTE_ENCODED ) + if( m_encoding != RAS_BYTE_ENCODED ) { for( y = 0; y < m_height; y++, data += step ) { @@ -239,7 +243,7 @@ bad_decoding_1bpp: break; /************************* 8 BPP ************************/ case 8: - if( m_type != RAS_BYTE_ENCODED ) + if( m_encoding != RAS_BYTE_ENCODED ) { for( y = 0; y < m_height; y++, data += step ) { @@ -304,7 +308,8 @@ bad_decoding_1bpp: if( data == line_end ) { - if( m_strm.getByte() != 0 ) + // Only odd-width 8-bit scanlines contain a padding byte. + if( (m_width & 1) != 0 && m_strm.getByte() != 0 ) goto bad_decoding_end; line_end += step; data = line_end - width3; @@ -325,7 +330,7 @@ bad_decoding_end: if( color ) { - if( m_type == RAS_FORMAT_RGB || m_use_rgb) + if( swap_blue_and_red ) icvCvt_RGB2BGR_8u_C3R(src, 0, data, 0, Size(m_width,1) ); else memcpy(data, src, std::min(step, (size_t)src_pitch)); @@ -333,7 +338,7 @@ bad_decoding_end: else { icvCvt_BGR2Gray_8u_C3C1R(src, 0, data, 0, Size(m_width,1), - m_type == RAS_FORMAT_RGB ? 2 : 0 ); + source_is_rgb ? 2 : 0 ); } } result = true; @@ -348,10 +353,10 @@ bad_decoding_end: if( color ) icvCvt_BGRA2BGR_8u_C4C3R( src + 4, 0, data, 0, Size(m_width,1), - (m_type == RAS_FORMAT_RGB || m_use_rgb) ? 2 : 0 ); + swap_blue_and_red ? 2 : 0 ); else icvCvt_BGRA2Gray_8u_C4C1R( src + 4, 0, data, 0, Size(m_width,1), - m_type == RAS_FORMAT_RGB ? 2 : 0 ); + source_is_rgb ? 2 : 0 ); } result = true; break; diff --git a/modules/imgcodecs/test/test_sunraster.cpp b/modules/imgcodecs/test/test_sunraster.cpp index 4de3e079f3..e63526a963 100644 --- a/modules/imgcodecs/test/test_sunraster.cpp +++ b/modules/imgcodecs/test/test_sunraster.cpp @@ -10,6 +10,8 @@ #include "test_precomp.hpp" +#include +#include #include namespace opencv_test { namespace { @@ -135,4 +137,97 @@ TEST(Imgcodecs_SunRaster, valid_8bpp_grayscale_decodes) EXPECT_EQ(result.type(), CV_8UC1); } +TEST(Imgcodecs_SunRaster, byte_encoded_8bpp_decodes) +{ + // Generated with Netpbm's pnmtorast -rle. + const String filename = findDataFile("readwrite/sunraster/byte_encoded_8bpp.ras"); + uint8_t expected_data[][16] = { + { 0x11, 0x80, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, + 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44 }, + { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }, + { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, + 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff }, + { 0x20, 0x20, 0x40, 0x40, 0x60, 0x60, 0x80, 0x80, + 0xa0, 0xa0, 0xc0, 0xc0, 0xe0, 0xe0, 0xff, 0xff }, + { 0x33, 0x33, 0x33, 0x33, 0x33, 0x33, 0x33, 0x33, + 0x33, 0x33, 0x33, 0x33, 0x33, 0x33, 0x33, 0x33 }, + { 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, + 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66 }, + { 0x99, 0x99, 0x99, 0x99, 0x99, 0x99, 0x99, 0x99, + 0x99, 0x99, 0x99, 0x99, 0x99, 0x99, 0x99, 0x99 }, + { 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, + 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc } + }; + const cv::Mat expected(8, 16, CV_8UC1, expected_data); + + cv::Mat result; + ASSERT_NO_THROW(result = cv::imread(filename, cv::IMREAD_GRAYSCALE)); + ASSERT_FALSE(result.empty()); + ASSERT_EQ(expected.size(), result.size()); + ASSERT_EQ(expected.type(), result.type()); + EXPECT_EQ(0, cv::norm(expected, result, cv::NORM_INF)); +} + +TEST(Imgcodecs_SunRaster, truncated_byte_encoded_8bpp_returns_empty) +{ + const String filename = findDataFile("readwrite/sunraster/byte_encoded_8bpp.ras"); + std::ifstream stream(filename.c_str(), std::ios::binary); + ASSERT_TRUE(stream.is_open()); + std::vector buf((std::istreambuf_iterator(stream)), + std::istreambuf_iterator()); + const size_t incompleteRleSize = 32 + 3 * 256 + 2; + ASSERT_GT(buf.size(), incompleteRleSize); + buf.resize(incompleteRleSize); // Keep an incomplete RLE escape after the color map. + + cv::Mat result; + ASSERT_NO_THROW(result = cv::imdecode(buf, cv::IMREAD_GRAYSCALE)); + EXPECT_TRUE(result.empty()); +} + +typedef tuple SunRasterColorParams; +typedef testing::TestWithParam Imgcodecs_SunRaster_Color; + +TEST_P(Imgcodecs_SunRaster_Color, decodes) +{ + const String filename = findDataFile(get<0>(GetParam())); + const int flags = get<1>(GetParam()); + + uint8_t bgr_data[] = { + 30, 20, 10, 60, 50, 40, 0, 0, 255, 0, 255, 0, + 255, 0, 0, 0, 255, 255, 255, 0, 255, 255, 255, 0 + }; + uint8_t rgb_data[] = { + 10, 20, 30, 40, 50, 60, 255, 0, 0, 0, 255, 0, + 0, 0, 255, 255, 255, 0, 255, 0, 255, 0, 255, 255 + }; + uint8_t gray_data[] = { 18, 48, 76, 150, 29, 226, 105, 179 }; + + cv::Mat expected; + if( flags == cv::IMREAD_COLOR ) + expected = cv::Mat(2, 4, CV_8UC3, bgr_data); + else if( flags == cv::IMREAD_COLOR_RGB ) + expected = cv::Mat(2, 4, CV_8UC3, rgb_data); + else + expected = cv::Mat(2, 4, CV_8UC1, gray_data); + + cv::Mat result; + ASSERT_NO_THROW(result = cv::imread(filename, flags)); + ASSERT_FALSE(result.empty()); + ASSERT_EQ(expected.size(), result.size()); + ASSERT_EQ(expected.type(), result.type()); + EXPECT_EQ(0, cv::norm(expected, result, cv::NORM_INF)); +} + +// Generated with ImageMagick's SUN encoder. +INSTANTIATE_TEST_CASE_P(RgbFormat, Imgcodecs_SunRaster_Color, + testing::Combine( + testing::Values( + "readwrite/sunraster/rgb_format_24bpp.ras", + "readwrite/sunraster/rgb_format_32bpp.ras"), + testing::Values( + cv::IMREAD_COLOR, + cv::IMREAD_COLOR_RGB, + cv::IMREAD_GRAYSCALE))); + }} // namespace opencv_test