Use positive indexes to acces the line buffer during encoding (#315)

The line buffer is 2 pixels longer then required to make is easier to encode pixels on the edges. The start of the pointers was set to the first real pixel.
This caused the problem that negative indexes needed to be used in the code, which looks incorrect at first sight and makes the code harder to port.
The same was already done for the decoding process.

src/jpeg_stream_writer.cpp

@@ -15,11 +15,6 @@ using std::array;
 using std::byte;
 using std::numeric_limits;
 
-jpeg_stream_writer::jpeg_stream_writer(const span<byte> destination) noexcept : destination_{destination}
-{
-}
-
-
 void jpeg_stream_writer::write_start_of_image()
 {
     write_segment_without_data(jpeg_marker_code::start_of_image);

src/jpeg_stream_writer.h

@@ -20,7 +20,6 @@ class jpeg_stream_writer final
 {
 public:
     jpeg_stream_writer() = default;
-    explicit jpeg_stream_writer(span<std::byte> destination) noexcept;
     ~jpeg_stream_writer() = default;
 
     jpeg_stream_writer(const jpeg_stream_writer&) = delete;
@@ -230,7 +229,7 @@ private:
         if (UNLIKELY(byte_offset_ + 2 > destination_.size()))
             impl::throw_jpegls_error(jpegls_errc::destination_buffer_too_small);
 
-        write_byte(jpeg_marker_start_byte);
+        write_byte(jpeg_marker_start_byte); // TODO : use write_marker.
         write_byte(static_cast<std::byte>(marker_code));
     }
 

src/jpegls_algorithm.h

@@ -8,7 +8,7 @@
 
 namespace charls {
 
-
+[[nodiscard]]
 constexpr int32_t log2_ceiling(const int32_t n) noexcept
 {
     ASSERT(n >= 0);
@@ -23,6 +23,24 @@ constexpr int32_t log2_ceiling(const int32_t n) noexcept
 }
 
 
+/// <summary>
+/// Computes how many bytes are needed to hold the number of bits.
+/// </summary>
+[[nodiscard]]
+constexpr uint32_t bit_to_byte_count(const int32_t bit_count) noexcept
+{
+    return static_cast<uint32_t>((bit_count + 7) / 8);
+}
+
+
+[[nodiscard]]
+constexpr int32_t calculate_maximum_sample_value(const int32_t bits_per_sample)
+{
+    ASSERT(bits_per_sample > 0 && bits_per_sample <= 16);
+    return static_cast<int32_t>((1U << bits_per_sample) - 1);
+}
+
+
 [[nodiscard]]
 constexpr int compute_maximum_near_lossless(const int maximum_sample_value) noexcept
 {

src/scan_codec.h

@@ -168,6 +168,14 @@ protected:
         run_index_ = std::max(0, run_index_ - 1);
     }
 
+    template<typename PixelType>
+    static void initialize_edge_pixels(PixelType* previous_line, PixelType* current_line, const uint32_t width) noexcept
+    {
+        // Initialize edge pixels used for prediction
+        previous_line[width + 1] = previous_line[width];
+        current_line[0] = previous_line[1];
+    }
+
     charls::frame_info frame_info_;
     coding_parameters parameters_;
     int32_t t1_{};

src/scan_decoder_impl.h

@@ -114,9 +114,7 @@ private:
                 {
                     run_index_ = run_index[component];
 
-                    // initialize edge pixels used for prediction
+                    initialize_edge_pixels(previous_line_, current_line_, width_);
-                    previous_line_[width_ + 1] = previous_line_[width_];
-                    current_line_[0] = previous_line_[1];
 
                     if constexpr (std::is_same_v<pixel_type, sample_type>)
                     {

src/scan_encoder_impl.h

@@ -97,22 +97,20 @@ private:
 
         for (uint32_t line{}; line < frame_info().height; ++line)
         {
-            previous_line_ = &line_buffer[1];
+            previous_line_ = line_buffer.data();
-            current_line_ = &line_buffer[1 + static_cast<size_t>(component_count) * pixel_stride];
+            current_line_ = line_buffer.data() + static_cast<size_t>(component_count) * pixel_stride;
             if ((line & 1) == 1)
             {
                 std::swap(previous_line_, current_line_);
             }
 
-            on_line_begin(current_line_, width_, pixel_stride);
+            on_line_begin(current_line_ + 1, width_, pixel_stride);
 
             for (size_t component{}; component < component_count; ++component)
             {
                 run_index_ = run_index[component];
 
-                // initialize edge pixels used for prediction
+                initialize_edge_pixels(previous_line_, current_line_, width_);
-                previous_line_[width_] = previous_line_[width_ - 1];
-                current_line_[-1] = previous_line_[0];
 
                 if constexpr (std::is_same_v<pixel_type, sample_type>)
                 {
@@ -138,11 +136,11 @@ private:
     /// <summary>Encodes a scan line of samples</summary>
     FORCE_INLINE void encode_sample_line()
     {
-        int32_t index{};
+        int32_t index{1};
         int32_t rb{previous_line_[index - 1]};
         int32_t rd{previous_line_[index]};
 
-        while (static_cast<uint32_t>(index) < width_)
+        while (static_cast<uint32_t>(index) <= width_)
         {
             const int32_t ra{current_line_[index - 1]};
             const int32_t rc{rb};
@@ -168,8 +166,8 @@ private:
     /// <summary>Encodes a scan line of triplets in ILV_SAMPLE mode</summary>
     void encode_triplet_line()
     {
-        int32_t index{};
+        int32_t index{1};
-        while (static_cast<uint32_t>(index) < width_)
+        while (static_cast<uint32_t>(index) <= width_)
         {
             const triplet<sample_type> ra{current_line_[index - 1]};
             const triplet<sample_type> rc{previous_line_[index - 1]};
@@ -202,8 +200,8 @@ private:
     /// <summary>Encodes a scan line of quads in ILV_SAMPLE mode</summary>
     void encode_quad_line()
     {
-        int32_t index{};
+        int32_t index{1};
-        while (static_cast<uint32_t>(index) < width_)
+        while (static_cast<uint32_t>(index) <= width_)
         {
             const quad<sample_type> ra{current_line_[index - 1]};
             const quad<sample_type> rc{previous_line_[index - 1]};
@@ -239,7 +237,7 @@ private:
     [[nodiscard]]
     int32_t encode_run_mode(const int32_t index)
     {
-        const int32_t count_type_remain = width_ - index;
+        const int32_t count_type_remain = width_ - (index - 1);
         pixel_type* type_cur_x{current_line_ + index};
         const pixel_type* type_prev_x{previous_line_ + index};
 

src/util.h

@@ -8,7 +8,6 @@
 #include <cassert>
 #include <cstdlib>
 #include <cstring>
-#include <limits>
 #include <type_traits>
 
 
@@ -313,22 +312,6 @@ inline void check_interleave_mode(const interleave_mode mode, const jpegls_errc
 }
 
 
-constexpr int32_t calculate_maximum_sample_value(const int32_t bits_per_sample)
-{
-    ASSERT(bits_per_sample > 0 && bits_per_sample <= 16);
-    return static_cast<int32_t>((1U << bits_per_sample) - 1);
-}
-
-
-/// <summary>
-/// Computes how many bytes are needed to hold the number of bits.
-/// </summary>
-constexpr uint32_t bit_to_byte_count(const int32_t bit_count) noexcept
-{
-    return static_cast<uint32_t>((bit_count + 7) / 8);
-}
-
-
 /// <summary>
 /// Converts an enumeration to its underlying type. Equivalent to C++23 std::to_underlying
 /// </summary>

unittest/jpeg_stream_reader_test.cpp

@@ -87,7 +87,8 @@ public:
     TEST_METHOD(read_header_jpegls_preset_parameter_segment) // NOLINT
     {
         vector<byte> source(100);
-        jpeg_stream_writer writer({source.data(), source.size()});
+        jpeg_stream_writer writer;
+        writer.destination({source.data(), source.size()});
         writer.write_start_of_image();
 
         constexpr jpegls_pc_parameters presets{1, 2, 3, 4, 5};
@@ -688,7 +689,8 @@ public:
     TEST_METHOD(read_mapping_table) // NOLINT
     {
         vector<byte> source(100);
-        jpeg_stream_writer writer({source.data(), source.size()});
+        jpeg_stream_writer writer;
+        writer.destination({source.data(), source.size()});
         writer.write_start_of_image();
 
         constexpr array table_data_expected{byte{2}};
@@ -718,7 +720,8 @@ public:
     TEST_METHOD(read_mapping_table_too_small_buffer_throws) // NOLINT
     {
         vector<byte> source(100);
-        jpeg_stream_writer writer({source.data(), source.size()});
+        jpeg_stream_writer writer;
+        writer.destination({source.data(), source.size()});
         writer.write_start_of_image();
 
         constexpr array table_data_expected{byte{2}, byte{3}};
@@ -803,7 +806,8 @@ public:
     {
         constexpr size_t table_size{100000};
         vector<byte> source(table_size + 100);
-        jpeg_stream_writer writer({source.data(), source.size()});
+        jpeg_stream_writer writer;
+        writer.destination({source.data(), source.size()});
         writer.write_start_of_image();
 
         vector<byte> table_data_expected(table_size);

unittest/jpeg_stream_writer_test.cpp

@@ -12,6 +12,7 @@
 
 using Microsoft::VisualStudio::CppUnitTestFramework::Assert;
 using std::array;
+using std::vector;
 using std::byte;
 using std::numeric_limits;
 using std::to_integer;
@@ -31,7 +32,8 @@ public:
     TEST_METHOD(write_start_of_image) // NOLINT
     {
         array<byte, 2> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_start_of_image();
 
@@ -43,7 +45,8 @@ public:
     TEST_METHOD(write_start_of_image_in_too_small_buffer_throws) // NOLINT
     {
         array<byte, 1> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         assert_expect_exception(jpegls_errc::destination_buffer_too_small, [&writer] { writer.write_start_of_image(); });
         Assert::AreEqual(size_t{}, writer.bytes_written());
@@ -52,7 +55,8 @@ public:
     TEST_METHOD(write_end_of_image) // NOLINT
     {
         array<byte, 2> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_end_of_image(false);
 
@@ -64,7 +68,8 @@ public:
     TEST_METHOD(write_end_of_image_even_no_extra_byte_needed) // NOLINT
     {
         array<byte, 2> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_end_of_image(true);
 
@@ -76,7 +81,8 @@ public:
     TEST_METHOD(write_end_of_image_even_extra_byte_needed) // NOLINT
     {
         array<byte, 5 + 3> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         // writer.
         constexpr byte comment{99};
@@ -97,7 +103,8 @@ public:
     TEST_METHOD(write_end_of_image_even_extra_byte_needed_not_enabled) // NOLINT
     {
         array<byte, 5 + 2> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         // writer.
         constexpr byte comment{99};
@@ -117,7 +124,8 @@ public:
     TEST_METHOD(write_end_of_image_in_too_small_buffer_throws) // NOLINT
     {
         array<byte, 1> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         assert_expect_exception(jpegls_errc::destination_buffer_too_small, [&writer] { writer.write_end_of_image(false); });
         Assert::AreEqual(size_t{}, writer.bytes_written());
@@ -126,7 +134,8 @@ public:
     TEST_METHOD(write_spiff_segment) // NOLINT
     {
         array<byte, 34> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         constexpr spiff_header header{spiff_profile_id::none,
                                       3,
@@ -197,7 +206,8 @@ public:
     TEST_METHOD(write_spiff_segment_in_too_small_buffer_throws) // NOLINT
     {
         array<byte, 33> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         spiff_header header{spiff_profile_id::none,
                             3,
@@ -218,7 +228,8 @@ public:
     TEST_METHOD(write_spiff_end_of_directory_segment) // NOLINT
     {
         array<byte, 10> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_spiff_end_of_directory_entry();
 
@@ -246,7 +257,8 @@ public:
     TEST_METHOD(write_spiff_directory_entry) // NOLINT
     {
         array<byte, 10> buffer{};
-        jpeg_stream_writer writer{{buffer.data(), buffer.size()}};
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         constexpr array data{byte{0x77}, byte{0x66}};
 
@@ -277,7 +289,8 @@ public:
         constexpr int32_t component_count{3};
 
         array<byte, 19> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         const bool oversized_image{
             writer.write_start_of_frame_segment({100, numeric_limits<uint16_t>::max(), bits_per_sample, component_count})};
@@ -315,7 +328,8 @@ public:
         constexpr int32_t component_count{3};
 
         array<byte, 19> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         const bool oversized_image{writer.write_start_of_frame_segment(
             {100, numeric_limits<uint16_t>::max() + 1U, bits_per_sample, component_count})};
@@ -353,7 +367,8 @@ public:
         constexpr int32_t component_count{1};
 
         array<byte, 13> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_start_of_frame_segment({1, 1, bits_per_sample, component_count});
 
@@ -365,7 +380,8 @@ public:
     TEST_METHOD(write_start_of_frame_marker_segment_with_high_boundary_values_and_serialize) // NOLINT
     {
         array<byte, 775> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_start_of_frame_segment(
             {numeric_limits<uint16_t>::max(), numeric_limits<uint16_t>::max(), 16, numeric_limits<uint8_t>::max()});
@@ -380,11 +396,12 @@ public:
     TEST_METHOD(write_color_transform_segment) // NOLINT
     {
         constexpr color_transformation transformation = color_transformation::hp1;
-
         array<byte, 9> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_color_transform_segment(transformation);
+
         Assert::AreEqual(buffer.size(), writer.bytes_written());
 
         // Verify mrfx identifier string.
@@ -401,7 +418,8 @@ public:
         constexpr jpegls_pc_parameters presets{2, 1, 2, 3, 7};
 
         array<byte, 15> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_jpegls_preset_parameters_segment(presets);
         Assert::AreEqual(buffer.size(), writer.bytes_written());
@@ -433,7 +451,8 @@ public:
     TEST_METHOD(write_jpegls_preset_parameters_segment_for_oversized_image_dimensions) // NOLINT
     {
         array<byte, 14> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_jpegls_preset_parameters_segment(100, numeric_limits<uint32_t>::max());
         Assert::AreEqual(buffer.size(), writer.bytes_written());
@@ -457,10 +476,11 @@ public:
         Assert::AreEqual(byte{255}, buffer[13]);
     }
 
-    TEST_METHOD(write_start_of_scan_marker) // NOLINT
+    TEST_METHOD(write_start_of_scan_segment) // NOLINT
     {
         array<byte, 10> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_start_of_scan_segment(1, 2, interleave_mode::none);
 
@@ -476,7 +496,8 @@ public:
     TEST_METHOD(rewind) // NOLINT
     {
         array<byte, 10> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         writer.write_start_of_scan_segment(1, 2, interleave_mode::none);
         writer.rewind();
@@ -490,7 +511,8 @@ public:
     TEST_METHOD(write_minimal_table) // NOLINT
     {
         array<byte, 8> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         constexpr array table_data{byte{77}};
         writer.write_jpegls_preset_parameters_segment(100, 1, table_data);
@@ -509,7 +531,8 @@ public:
     TEST_METHOD(write_table_max_entry_size) // NOLINT
     {
         array<byte, 7 + 255> buffer{};
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
         constexpr array<byte, 255> table_data{};
         writer.write_jpegls_preset_parameters_segment(100, 255, table_data);
@@ -527,10 +550,11 @@ public:
 
     TEST_METHOD(write_table_fits_in_single_segment) // NOLINT
     {
-        std::vector<byte> buffer(size_t{2} + std::numeric_limits<uint16_t>::max());
+        vector<byte> buffer(size_t{2} + std::numeric_limits<uint16_t>::max());
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
-        std::vector<byte> table_data(std::numeric_limits<uint16_t>::max() - 5);
+        vector<byte> table_data(std::numeric_limits<uint16_t>::max() - 5);
         writer.write_jpegls_preset_parameters_segment(255, 1, {table_data.data(), table_data.size()});
 
         Assert::AreEqual(buffer.size(), writer.bytes_written());
@@ -546,10 +570,11 @@ public:
 
     TEST_METHOD(write_table_that_requires_two_segment) // NOLINT
     {
-        std::vector<byte> buffer(size_t{2} + std::numeric_limits<uint16_t>::max() + 8);
+        vector<byte> buffer(size_t{2} + std::numeric_limits<uint16_t>::max() + 8);
-        jpeg_stream_writer writer({buffer.data(), buffer.size()});
+        jpeg_stream_writer writer;
+        writer.destination({buffer.data(), buffer.size()});
 
-        std::vector<byte> table_data(static_cast<size_t>(std::numeric_limits<uint16_t>::max()) - 5 + 1);
+        vector<byte> table_data(static_cast<size_t>(std::numeric_limits<uint16_t>::max()) - 5 + 1);
         writer.write_jpegls_preset_parameters_segment(255, 1, {table_data.data(), table_data.size()});
 
         Assert::AreEqual(buffer.size(), writer.bytes_written());

unittest/util.cpp

@@ -142,7 +142,8 @@ vector<byte> create_test_spiff_header(const uint8_t high_version, const uint8_t
     const size_t spiff_header_size{buffer.size()};
     buffer.resize(buffer.size() + 100);
 
-    jpeg_stream_writer writer({buffer.data() + spiff_header_size, buffer.size() - spiff_header_size});
+    jpeg_stream_writer writer;
+    writer.destination({buffer.data() + spiff_header_size, buffer.size() - spiff_header_size});
 
     if (end_of_directory)
     {