Use SFINAE on Container::value_type to prevent ambiguous overloads (#156

) STL containers have the ::value_type typedef. Use this as simple SFINAE check to ensure API methods that should be used with an STL like container are only available if a container is passed. This prevents ambiguous overload problems. There are more advanced solutions: is_container in Boost, Concepts in C++20. but for the C++ API this solution meets its purpose (and is supported on older compilers).
team-charls · Jan 25, 2022 · 11bda37 · 11bda37
1 parent a7a9449
commit 11bda37
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project are documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [2.3.1] - 2021-1-25
+
+### Fixed
+
+- Fixed [#155](https://github.com/team-charls/charls/issues/155), charls::jpegls_decoder::decode: 2 overloads have similar conversions in v2.3.0
+
 ## [2.3.0] - 2022-1-24
 
 ### Added

diff --git a/SECURITY.md b/SECURITY.md
@@ -4,7 +4,8 @@
 
 | Version | Supported          |
 | ------- | ------------------ |
-| 2.3.0   | :white_check_mark: |
+| 2.3.1   | :white_check_mark: |
+| 2.3.0   | :x:                |
 | 2.2.0   | :x:                |
 | 2.1.0   | :x:                |
 | 2.0.0   | :x:                |

diff --git a/include/charls/charls_jpegls_decoder.h b/include/charls/charls_jpegls_decoder.h
@@ -271,7 +271,8 @@ class jpegls_decoder final
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
     /// <exception cref="std::bad_alloc">Thrown when memory for the decoder could not be allocated.</exception>
     /// <returns>Frame info of the decoded image and the interleave mode.</returns>
-    template<typename SourceContainer, typename DestinationContainer>
+    template<typename SourceContainer, typename DestinationContainer, typename T1 = typename SourceContainer::value_type,
+             typename T2 = typename DestinationContainer::value_type>
     static std::pair<charls::frame_info, charls::interleave_mode>
     decode(const SourceContainer& source, DestinationContainer& destination,
            const size_t maximum_size_in_bytes = 7680 * 4320 * 3)
@@ -324,7 +325,7 @@ class jpegls_decoder final
     /// </param>
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
     /// <exception cref="std::bad_alloc">Thrown when memory for the decoder could not be allocated.</exception>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     jpegls_decoder(const Container& source_container, const bool parse_header) :
         jpegls_decoder(source_container.data(), source_container.size() * sizeof(typename Container::value_type),
                        parse_header)
@@ -354,7 +355,7 @@ class jpegls_decoder final
     /// A STL like container that provides the functions data() and size() and the type value_type.
     /// </param>
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     jpegls_decoder& source(const Container& source_container)
     {
         return source(source_container.data(), source_container.size() * sizeof(typename Container::value_type));
@@ -557,7 +558,7 @@ class jpegls_decoder final
     /// </param>
     /// <param name="stride">Number of bytes to the next line in the buffer, when zero, decoder will compute it.</param>
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     void decode(CHARLS_OUT Container& destination_container, const uint32_t stride = 0) const
     {
         decode(destination_container.data(), destination_container.size() * sizeof(typename Container::value_type), stride);
@@ -569,7 +570,7 @@ class jpegls_decoder final
     /// <param name="stride">Number of bytes to the next line in the buffer, when zero, decoder will compute it.</param>
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
     /// <returns>Container with the decoded data.</returns>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     CHARLS_CHECK_RETURN Container decode(const uint32_t stride = 0) const
     {
         Container destination(destination_size() / sizeof(typename Container::value_type));

diff --git a/include/charls/charls_jpegls_encoder.h b/include/charls/charls_jpegls_encoder.h
@@ -285,7 +285,7 @@ class jpegls_encoder final
     /// <exception cref="charls::jpegls_error">An error occurred during the operation.</exception>
     /// <exception cref="std::bad_alloc">Thrown when memory for the encoder could not be allocated.</exception>
     /// <returns>Container with the JPEG-LS encoded bytes.</returns>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     static Container encode(const Container& source, const charls::frame_info& frame,
                             const charls::interleave_mode interleave_mode = charls::interleave_mode::none,
                             const encoding_options encoding_options = charls::encoding_options::none)
@@ -410,14 +410,14 @@ class jpegls_encoder final
     /// <param name="destination_container">
     /// The STL like container, that supports the functions data() and size() and the typedef value_type.
     /// </param>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     jpegls_encoder& destination(Container& destination_container)
     {
         return destination(destination_container.data(),
                            destination_container.size() * sizeof(typename Container::value_type));
     }
 
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     jpegls_encoder& destination(const Container& destination_container) = delete;
 
     /// <summary>
@@ -519,7 +519,7 @@ class jpegls_encoder final
     /// Stride is sometimes called pitch. If padding bytes are present, the stride is wider than the width of the image.
     /// </param>
     /// <returns>The number of bytes written to the destination.</returns>
-    template<typename Container>
+    template<typename Container, typename T = typename Container::value_type>
     size_t encode(const Container& source_container, const uint32_t stride = 0) const
     {
         return encode(source_container.data(), source_container.size() * sizeof(typename Container::value_type), stride);

diff --git a/include/charls/version.h b/include/charls/version.h
@@ -16,7 +16,7 @@ extern "C" {
 
 #define CHARLS_VERSION_MAJOR 2
 #define CHARLS_VERSION_MINOR 3
-#define CHARLS_VERSION_PATCH 0
+#define CHARLS_VERSION_PATCH 1
 
 /// <summary>
 /// Returns the version of CharLS in the semver format "major.minor.patch" or "major.minor.patch-pre_release"

diff --git a/unittest/jpegls_decoder_test.cpp b/unittest/jpegls_decoder_test.cpp
@@ -735,6 +735,28 @@ TEST_CLASS(jpegls_decoder_test)
         #endif
     }
 
+    TEST_METHOD(decode_to_buffer_with_uint16_size_works) // NOLINT
+    {
+        // These are compile time checks to detect issues with overloads that have similar conversions.
+        constexpr frame_info frame_info{100, 100, 8, 1};
+        const vector<uint8_t> source(static_cast<size_t>(frame_info.width) * frame_info.height);
+
+        const vector<uint8_t> encoded_source{
+            jpegls_encoder::encode(source, frame_info, interleave_mode::none, encoding_options::even_destination_size)};
+
+        jpegls_decoder decoder;
+        decoder.source(encoded_source);
+        decoder.read_header();
+
+        vector<uint8_t> destination(decoder.destination_size());
+
+        void* data{destination.data()};
+        const uint16_t size{static_cast<uint16_t>(destination.size())};
+
+        // size is not a perfect match and needs a conversion.
+        decoder.decode(data, size);
+    }
+
 private:
     static vector<uint8_t>::iterator find_scan_header(const vector<uint8_t>::iterator begin,
                                                       const vector<uint8_t>::iterator end) noexcept

diff --git a/unittest/jpegls_encoder_test.cpp b/unittest/jpegls_encoder_test.cpp
@@ -1324,6 +1324,32 @@ TEST_CLASS(jpegls_encoder_test)
         Assert::IsFalse(it == destination.cend());
     }
 
+    TEST_METHOD(encode_to_buffer_with_uint16_size_works) // NOLINT
+    {
+        // These are compile time checks to detect issues with overloads that have similar conversions.
+        constexpr frame_info frame_info{100, 100, 8, 1};
+
+        jpegls_encoder encoder;
+        encoder.frame_info(frame_info);
+
+        vector<uint8_t> destination(encoder.estimated_destination_size());
+
+        void* data1 = destination.data();
+        const uint16_t size1 = static_cast<uint16_t>(destination.size());
+        encoder.destination(data1, size1);
+
+        vector<uint8_t> source(static_cast<size_t>(frame_info.width) * frame_info.height);
+        void* data2 = source.data();
+        const uint16_t size2 = static_cast<uint16_t>(source.size());
+
+        // Set 1 value to prevent complains about const.
+        uint8_t* p = static_cast<uint8_t*>(data2);
+        *p = 7;
+
+        // size2 is not a perfect match and needs a conversion.
+        ignore = encoder.encode(data2, size2);
+    }
+
 private:
     static void test_by_decoding(const vector<uint8_t>& encoded_source, const frame_info& source_frame_info,
                                  const void* expected_destination, const size_t expected_destination_size,