From f0df78b7e743a8d735a4bc669ae64a155f08645d Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Fri, 18 Nov 2022 18:09:26 +0000 Subject: [PATCH] imgcodecs: ensure parameters are key-value pairs, fix HDR encoder --- .../imgcodecs/include/opencv2/imgcodecs.hpp | 7 +++ modules/imgcodecs/src/grfmt_hdr.cpp | 18 ++++++- modules/imgcodecs/src/grfmt_hdr.hpp | 6 --- modules/imgcodecs/src/loadsave.cpp | 48 +++++++++++++++++-- modules/imgcodecs/test/test_grfmt.cpp | 40 ++++++++++++---- modules/imgcodecs/test/test_read_write.cpp | 19 ++++++++ 6 files changed, 119 insertions(+), 19 deletions(-) diff --git a/modules/imgcodecs/include/opencv2/imgcodecs.hpp b/modules/imgcodecs/include/opencv2/imgcodecs.hpp index 49f76826b9..c4b570e68c 100644 --- a/modules/imgcodecs/include/opencv2/imgcodecs.hpp +++ b/modules/imgcodecs/include/opencv2/imgcodecs.hpp @@ -96,6 +96,7 @@ enum ImwriteFlags { IMWRITE_PXM_BINARY = 32, //!< For PPM, PGM, or PBM, it can be a binary format flag, 0 or 1. Default value is 1. IMWRITE_EXR_TYPE = (3 << 4) + 0, /* 48 */ //!< override EXR storage type (FLOAT (FP32) is default) IMWRITE_WEBP_QUALITY = 64, //!< For WEBP, it can be a quality from 1 to 100 (the higher is the better). By default (without any parameter) and for quality above 100 the lossless compression is used. + IMWRITE_HDR_COMPRESSION = (5 << 4) + 0, /* 80 */ //!< specify HDR compression IMWRITE_PAM_TUPLETYPE = 128,//!< For PAM, sets the TUPLETYPE field to the corresponding string value that is defined for the format IMWRITE_TIFF_RESUNIT = 256,//!< For TIFF, use to specify which DPI resolution unit to set; see libtiff documentation for valid values. IMWRITE_TIFF_XDPI = 257,//!< For TIFF, use to specify the X direction DPI. @@ -145,6 +146,12 @@ enum ImwritePAMFlags { IMWRITE_PAM_FORMAT_RGB_ALPHA = 5 }; +//! Imwrite HDR specific values for IMWRITE_HDR_COMPRESSION parameter key +enum ImwriteHDRCompressionFlags { + IMWRITE_HDR_COMPRESSION_NONE = 0, + IMWRITE_HDR_COMPRESSION_RLE = 1 +}; + //! @} imgcodecs_flags /** @brief Loads an image from a file. diff --git a/modules/imgcodecs/src/grfmt_hdr.cpp b/modules/imgcodecs/src/grfmt_hdr.cpp index a274b2233c..6e2f565e32 100644 --- a/modules/imgcodecs/src/grfmt_hdr.cpp +++ b/modules/imgcodecs/src/grfmt_hdr.cpp @@ -141,14 +141,28 @@ bool HdrEncoder::write( const Mat& input_img, const std::vector& params ) if(img.depth() != CV_32F) { img.convertTo(img, CV_32FC3, 1/255.0f); } - CV_Assert(params.empty() || params[0] == HDR_NONE || params[0] == HDR_RLE); + + int compression = IMWRITE_HDR_COMPRESSION_RLE; + for (size_t i = 0; i + 1 < params.size(); i += 2) + { + switch (params[i]) + { + case IMWRITE_HDR_COMPRESSION: + compression = params[i + 1]; + break; + default: + break; + } + } + CV_Check(compression, compression == IMWRITE_HDR_COMPRESSION_NONE || compression == IMWRITE_HDR_COMPRESSION_RLE, ""); + FILE *fout = fopen(m_filename.c_str(), "wb"); if(!fout) { return false; } RGBE_WriteHeader(fout, img.cols, img.rows, NULL); - if(params.empty() || params[0] == HDR_RLE) { + if (compression == IMWRITE_HDR_COMPRESSION_RLE) { RGBE_WritePixels_RLE(fout, const_cast(img.ptr()), img.cols, img.rows); } else { RGBE_WritePixels(fout, const_cast(img.ptr()), img.cols * img.rows); diff --git a/modules/imgcodecs/src/grfmt_hdr.hpp b/modules/imgcodecs/src/grfmt_hdr.hpp index fa29fbe0d2..f0a920083f 100644 --- a/modules/imgcodecs/src/grfmt_hdr.hpp +++ b/modules/imgcodecs/src/grfmt_hdr.hpp @@ -50,12 +50,6 @@ namespace cv { -enum HdrCompression -{ - HDR_NONE = 0, - HDR_RLE = 1 -}; - // Radiance rgbe (.hdr) reader class HdrDecoder CV_FINAL : public BaseImageDecoder { diff --git a/modules/imgcodecs/src/loadsave.cpp b/modules/imgcodecs/src/loadsave.cpp index bd87c379ab..1bdc7029c0 100644 --- a/modules/imgcodecs/src/loadsave.cpp +++ b/modules/imgcodecs/src/loadsave.cpp @@ -662,7 +662,7 @@ bool imreadmulti(const String& filename, std::vector& mats, int flags) } static bool imwrite_( const String& filename, const std::vector& img_vec, - const std::vector& params, bool flipv ) + const std::vector& params_, bool flipv ) { bool isMultiImg = img_vec.size() > 1; std::vector write_vec; @@ -696,7 +696,27 @@ static bool imwrite_( const String& filename, const std::vector& img_vec, } encoder->setDestination( filename ); - CV_Assert(params.size() <= CV_IO_MAX_IMAGE_PARAMS*2); +#if CV_VERSION_MAJOR < 5 && defined(HAVE_IMGCODEC_HDR) + bool fixed = false; + std::vector params_pair(2); + if (dynamic_cast(encoder.get())) + { + if (params_.size() == 1) + { + CV_LOG_WARNING(NULL, "imwrite() accepts key-value pair of parameters, but single value is passed. " + "HDR encoder behavior has been changed, please use IMWRITE_HDR_COMPRESSION key."); + params_pair[0] = IMWRITE_HDR_COMPRESSION; + params_pair[1] = params_[0]; + fixed = true; + } + } + const std::vector& params = fixed ? params_pair : params_; +#else + const std::vector& params = params_; +#endif + + CV_Check(params.size(), (params.size() & 1) == 0, "Encoding 'params' must be key-value pairs"); + CV_CheckLE(params.size(), (size_t)(CV_IO_MAX_IMAGE_PARAMS*2), ""); bool code = false; try { @@ -936,7 +956,7 @@ Mat imdecode( InputArray _buf, int flags, Mat* dst ) } bool imencode( const String& ext, InputArray _image, - std::vector& buf, const std::vector& params ) + std::vector& buf, const std::vector& params_ ) { CV_TRACE_FUNCTION(); @@ -958,6 +978,28 @@ bool imencode( const String& ext, InputArray _image, image = temp; } +#if CV_VERSION_MAJOR < 5 && defined(HAVE_IMGCODEC_HDR) + bool fixed = false; + std::vector params_pair(2); + if (dynamic_cast(encoder.get())) + { + if (params_.size() == 1) + { + CV_LOG_WARNING(NULL, "imwrite() accepts key-value pair of parameters, but single value is passed. " + "HDR encoder behavior has been changed, please use IMWRITE_HDR_COMPRESSION key."); + params_pair[0] = IMWRITE_HDR_COMPRESSION; + params_pair[1] = params_[0]; + fixed = true; + } + } + const std::vector& params = fixed ? params_pair : params_; +#else + const std::vector& params = params_; +#endif + + CV_Check(params.size(), (params.size() & 1) == 0, "Encoding 'params' must be key-value pairs"); + CV_CheckLE(params.size(), (size_t)(CV_IO_MAX_IMAGE_PARAMS*2), ""); + bool code; if( encoder->setDestination(buf) ) { diff --git a/modules/imgcodecs/test/test_grfmt.cpp b/modules/imgcodecs/test/test_grfmt.cpp index cbf6289d23..81550bffba 100644 --- a/modules/imgcodecs/test/test_grfmt.cpp +++ b/modules/imgcodecs/test/test_grfmt.cpp @@ -301,21 +301,45 @@ TEST(Imgcodecs_Hdr, regression) Mat img_no_rle = imread(name_no_rle, -1); ASSERT_FALSE(img_no_rle.empty()) << "Could not open " << name_no_rle; - double min = 0.0, max = 1.0; - minMaxLoc(abs(img_rle - img_no_rle), &min, &max); - ASSERT_FALSE(max > DBL_EPSILON); + EXPECT_EQ(cvtest::norm(img_rle, img_no_rle, NORM_INF), 0.0); + string tmp_file_name = tempfile(".hdr"); - vectorparam(1); + vector param(2); + param[0] = IMWRITE_HDR_COMPRESSION; for(int i = 0; i < 2; i++) { - param[0] = i; + param[1] = i; imwrite(tmp_file_name, img_rle, param); Mat written_img = imread(tmp_file_name, -1); - ASSERT_FALSE(written_img.empty()) << "Could not open " << tmp_file_name; - minMaxLoc(abs(img_rle - written_img), &min, &max); - ASSERT_FALSE(max > DBL_EPSILON); + EXPECT_EQ(cvtest::norm(written_img, img_rle, NORM_INF), 0.0); } remove(tmp_file_name.c_str()); } + +TEST(Imgcodecs_Hdr, regression_imencode) +{ + string folder = string(cvtest::TS::ptr()->get_data_path()) + "/readwrite/"; + string name = folder + "rle.hdr"; + Mat img_ref = imread(name, -1); + ASSERT_FALSE(img_ref.empty()) << "Could not open " << name; + + vector params(2); + params[0] = IMWRITE_HDR_COMPRESSION; + { + vector buf; + params[1] = IMWRITE_HDR_COMPRESSION_NONE; + imencode(".hdr", img_ref, buf, params); + Mat img = imdecode(buf, -1); + EXPECT_EQ(cvtest::norm(img_ref, img, NORM_INF), 0.0); + } + { + vector buf; + params[1] = IMWRITE_HDR_COMPRESSION_RLE; + imencode(".hdr", img_ref, buf, params); + Mat img = imdecode(buf, -1); + EXPECT_EQ(cvtest::norm(img_ref, img, NORM_INF), 0.0); + } +} + #endif #ifdef HAVE_IMGCODEC_PXM diff --git a/modules/imgcodecs/test/test_read_write.cpp b/modules/imgcodecs/test/test_read_write.cpp index 985d5c110a..30692452b2 100644 --- a/modules/imgcodecs/test/test_read_write.cpp +++ b/modules/imgcodecs/test/test_read_write.cpp @@ -288,4 +288,23 @@ TEST(Imgcodecs_Image, write_umat) EXPECT_EQ(0, remove(dst_name.c_str())); } +TEST(Imgcodecs_Params, imwrite_regression_22752) +{ + const Mat img(16, 16, CV_8UC3, cv::Scalar::all(0)); + vector params; + params.push_back(IMWRITE_JPEG_QUALITY); +// params.push_back(100)); // Forget it. + EXPECT_ANY_THROW(cv::imwrite("test.jpg", img, params)); // parameters size or missing JPEG codec +} + +TEST(Imgcodecs_Params, imencode_regression_22752) +{ + const Mat img(16, 16, CV_8UC3, cv::Scalar::all(0)); + vector params; + params.push_back(IMWRITE_JPEG_QUALITY); +// params.push_back(100)); // Forget it. + vector buf; + EXPECT_ANY_THROW(cv::imencode("test.jpg", img, buf, params)); // parameters size or missing JPEG codec +} + }} // namespace -- 2.34.1