From 1ba2294516c6b4c89f45d4379aa6163171283f37 Mon Sep 17 00:00:00 2001 From: Victor Derks Date: Sat, 20 Jan 2024 22:09:55 +0100 Subject: [PATCH] Use span and improve spelling (#301) --- CharLS.sln.DotSettings | 3 + benchmark/benchmark.cpp | 289 ++++++++++++++++++++++++- benchmark/benchmark.vcxproj | 12 - include/charls/charls_jpegls_decoder.h | 20 +- include/charls/charls_jpegls_encoder.h | 18 +- include/charls/public_types.h | 18 +- src/charls_jpegls_encoder.cpp | 11 +- src/default_traits.h | 2 +- src/jpeg_stream_writer.cpp | 36 ++- src/jpeg_stream_writer.h | 25 ++- src/span.h | 1 + test/main.cpp | 1 - unittest/jpeg_stream_writer_test.cpp | 4 +- unittest/jpegls_algorithm_test.cpp | 2 +- unittest/scan_encoder_test.cpp | 2 +- 15 files changed, 356 insertions(+), 88 deletions(-) diff --git a/CharLS.sln.DotSettings b/CharLS.sln.DotSettings index 95d2ca77..60d91456 100644 --- a/CharLS.sln.DotSettings +++ b/CharLS.sln.DotSettings @@ -43,6 +43,8 @@ <NamingElement Priority="12"><Descriptor Static="Indeterminate" Constexpr="Indeterminate" Const="Indeterminate" Volatile="Indeterminate" Accessibility="NOT_APPLICABLE"><type Name="union member" /></Descriptor><Policy Inspect="False" Prefix="" Suffix="" Style="aa_bb" /></NamingElement> False True + True + True True True True @@ -89,6 +91,7 @@ True True True + True True True True diff --git a/benchmark/benchmark.cpp b/benchmark/benchmark.cpp index 9765e4a1..2f76848e 100644 --- a/benchmark/benchmark.cpp +++ b/benchmark/benchmark.cpp @@ -313,16 +313,299 @@ overwrite_buffer allocate_overwrite_buffer(const size_t size) return buffer; } - static void bm_resize_overwrite_buffer(benchmark::State& state) { for (const auto _ : state) { - benchmark::DoNotOptimize(allocate_buffer(size_t{512} * 512 * 16)); - benchmark::DoNotOptimize(allocate_buffer(size_t{1024} * 1024 * 8 * 3)); + benchmark::DoNotOptimize(allocate_overwrite_buffer(size_t{512} * 512 * 16)); + benchmark::DoNotOptimize(allocate_overwrite_buffer(size_t{1024} * 1024 * 8 * 3)); } } BENCHMARK(bm_resize_overwrite_buffer); +int memset_buffer(uint8_t* data, const size_t size) +{ + memset(data, 0, size); + return 0; +} +static void bm_memset_buffer(benchmark::State& state) +{ + std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(memset_buffer(buffer.data(), size_t{512} * 512 * 16)); + benchmark::DoNotOptimize(memset_buffer(buffer.data(), size_t{1024} * 1024 * 8 * 3)); + } +} +BENCHMARK(bm_memset_buffer); + + +bool has_ff_byte_classic(const unsigned int value) +{ + // Check if any byte is equal to 0xFF + return ((value & 0xFF) == 0xFF) || (((value >> 8) & 0xFF) == 0xFF) || (((value >> 16) & 0xFF) == 0xFF) || + (((value >> 24) & 0xFF) == 0xFF); +} +static void bm_has_ff_byte_classic(benchmark::State& state) +{ + for (const auto _ : state) + { + benchmark::DoNotOptimize(has_ff_byte_classic(0)); + benchmark::DoNotOptimize(has_ff_byte_classic(0xFF)); + } +} +BENCHMARK(bm_has_ff_byte_classic); + +bool has_ff_byte_loop(const unsigned int value) +{ + // Iterate over each byte and check if it is equal to 0xFF + for (int i = 0; i < sizeof(unsigned int); ++i) + { + if ((value & (0xFF << (8 * i))) == (0xFFU << (8 * i))) + { + return true; + } + } + return false; +} +static void bm_has_ff_byte_loop(benchmark::State& state) +{ + for (const auto _ : state) + { + benchmark::DoNotOptimize(has_ff_byte_loop(0)); + benchmark::DoNotOptimize(has_ff_byte_loop(0xFF)); + } +} +BENCHMARK(bm_has_ff_byte_loop); + +bool has_ff_byte_simd(const unsigned int value) { + // Use SSE instructions for parallel comparison + const __m128i xmm_value = _mm_set1_epi32(value); + const __m128i xmm_ff = _mm_set1_epi32(0xFF); + + // Compare each byte for equality with 0xFF + const __m128i comparison = _mm_cmpeq_epi8(xmm_value, xmm_ff); + + // Check if any comparison result is true + return _mm_testz_si128(comparison, comparison) == 0; + } +static void bm_has_ff_byte_simd(benchmark::State& state) +{ + for (const auto _ : state) + { + benchmark::DoNotOptimize(has_ff_byte_simd(0)); + benchmark::DoNotOptimize(has_ff_byte_simd(0xFF)); + } +} +BENCHMARK(bm_has_ff_byte_simd); + + +const std::byte* find_jpeg_marker_start_byte(const std::byte* position, const std::byte* end_position) noexcept +{ + constexpr std::byte jpeg_marker_start_byte{0xFF}; + + // Use memchr to find next start byte (0xFF). memchr is optimized on some platforms to search faster. + return static_cast( + memchr(position, std::to_integer(jpeg_marker_start_byte), end_position - position)); +} +static void bm_find_jpeg_marker_start_byte(benchmark::State& state) +{ + const std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(find_jpeg_marker_start_byte(buffer.data(), buffer.data() + buffer.size())); + } +} +BENCHMARK(bm_find_jpeg_marker_start_byte); + +// A simple overload with uint64_t\uint32_t doesn't work for macOS. size_t is not the same type as uint64_t. +template +constexpr bool is_uint_v = sizeof(T) == BitCount / 8 && std::is_integral_v && !std::is_signed_v; + +template +[[nodiscard]] +auto byte_swap(const T value) noexcept +{ + if constexpr (is_uint_v<16, T>) + { +#ifdef _MSC_VER + return _byteswap_ushort(value); +#else + // Note: GCC and Clang will optimize this pattern to a built-in intrinsic. + return static_cast(value << 8 | value >> 8); +#endif + } + else if constexpr (is_uint_v<32, T>) + { +#ifdef _MSC_VER + return _byteswap_ulong(value); +#else + // Note: GCC and Clang will optimize this pattern to a built-in intrinsic. + return value >> 24 | (value & 0x00FF0000) >> 8 | (value & 0x0000FF00) << 8 | value << 24; +#endif + } + else + { + static_assert(is_uint_v<64, T>); +#ifdef _MSC_VER + return _byteswap_uint64(value); +#else + // Note: GCC and Clang will optimize this pattern to a built-in intrinsic. + return (value << 56) | ((value << 40) & 0x00FF'0000'0000'0000) | ((value << 24) & 0x0000'FF00'0000'0000) | + ((value << 8) & 0x0000'00FF'0000'0000) | ((value >> 8) & 0x0000'0000'FF00'0000) | + ((value >> 24) & 0x0000'0000'00FF'0000) | ((value >> 40) & 0x0000'0000'0000'FF00) | (value >> 56); +#endif + } +} + + + +template +[[nodiscard]] +T read_unaligned(const void* buffer) noexcept +{ + // Note: MSVC, GCC and clang will replace this with a direct register read if the CPU architecture allows it + // On x86, x64 and ARM64 this will just be 1 register load. + T value; + memcpy(&value, buffer, sizeof(T)); + return value; +} + +template +T read_big_endian_unaligned(const void* buffer) noexcept +{ +#ifdef LITTLE_ENDIAN_ARCHITECTURE + return byte_swap(read_unaligned(buffer)); +#else + return read_unaligned(buffer); +#endif +} + +uint32_t read_all_bytes_with_ff_check(const std::byte* position, const std::byte* end_position) +{ + uint32_t result{}; + + for (; position < end_position; position += sizeof(uint32_t)) + { + if (const uint32_t value{read_big_endian_unaligned(position)}; + has_ff_byte_simd(value)) + { + result++; + } + else + { + result |= value; + } + + } + + return result; +} +static void bm_read_all_bytes_with_ff_check(benchmark::State& state) +{ + const std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(read_all_bytes_with_ff_check(buffer.data(), buffer.data() + buffer.size())); + } +} +BENCHMARK(bm_read_all_bytes_with_ff_check); + + +bool has_ff_byte_simd64(const uint64_t value) +{ + // Use SSE instructions for parallel comparison + const __m128i xmm_value = _mm_set1_epi64x(value); + const __m128i xmm_ff = _mm_set1_epi32(0xFF); + + // Compare each byte for equality with 0xFF + const __m128i comparison = _mm_cmpeq_epi8(xmm_value, xmm_ff); + + // Check if any comparison result is true + return _mm_testz_si128(comparison, comparison) == 0; +} + +uint64_t read_all_bytes_with_ff_check64(const std::byte* position, const std::byte* end_position) +{ + uint64_t result{}; + + for (; position < end_position; position += sizeof(uint64_t)) + { + if (const uint64_t value{read_big_endian_unaligned(position)}; has_ff_byte_simd64(value)) + { + result++; + } + else + { + result |= value; + } + } + + return result; +} +static void bm_read_all_bytes_with_ff_check64(benchmark::State& state) +{ + const std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(read_all_bytes_with_ff_check64(buffer.data(), buffer.data() + buffer.size())); + } +} +BENCHMARK(bm_read_all_bytes_with_ff_check64); + + +uint32_t read_all_bytes_no_check(const std::byte* position, const std::byte* end_position) +{ + uint32_t result{}; + + for (; position < end_position; position += sizeof(uint32_t)) + { + const uint32_t value{read_big_endian_unaligned(position)}; + result |= value; + } + + return result; +} +static void bm_read_all_bytes_no_check(benchmark::State& state) +{ + const std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(read_all_bytes_no_check(buffer.data(), buffer.data() + buffer.size())); + } +} +BENCHMARK(bm_read_all_bytes_no_check); + +uint64_t read_all_bytes_no_check64(const std::byte* position, const std::byte* end_position) +{ + uint64_t result{}; + + for (; position < end_position; position += sizeof(uint64_t)) + { + const uint64_t value{read_big_endian_unaligned(position)}; + result |= value; + } + + return result; +} +static void bm_read_all_bytes_no_check64(benchmark::State& state) +{ + const std::vector buffer(size_t{1024} * 1024 * 8 * 3); + + for (const auto _ : state) + { + benchmark::DoNotOptimize(read_all_bytes_no_check64(buffer.data(), buffer.data() + buffer.size())); + } +} +BENCHMARK(bm_read_all_bytes_no_check64); + + + + BENCHMARK_MAIN(); diff --git a/benchmark/benchmark.vcxproj b/benchmark/benchmark.vcxproj index 43259225..0212694e 100644 --- a/benchmark/benchmark.vcxproj +++ b/benchmark/benchmark.vcxproj @@ -96,25 +96,13 @@ - - true - - true false - - true - - - true - - true false - true false diff --git a/include/charls/charls_jpegls_decoder.h b/include/charls/charls_jpegls_decoder.h index 250ac20b..9b112dad 100644 --- a/include/charls/charls_jpegls_decoder.h +++ b/include/charls/charls_jpegls_decoder.h @@ -62,7 +62,7 @@ charls_jpegls_decoder_set_source_buffer(CHARLS_IN charls_jpegls_decoder* decoder /// /// Tries to read the SPIFF header from the source buffer. /// If a SPIFF header exists its content will be put into the spiff_header parameter and header_found will be set to 1. -/// Call charls_jpegls_decoder_read_header to read the normal JPEG header afterwards. +/// Call charls_jpegls_decoder_read_header to read the normal JPEG header afterward. /// /// Reference to the decoder instance. /// Output argument, will hold the SPIFF header when one could be found. @@ -240,7 +240,7 @@ class jpegls_decoder final /// Destination container that will hold the image data on return. Container will be resized automatically. /// /// - /// The maximum output size that may be allocated, default is 94 MiB (enough to decode 8 bit color 8K image). + /// The maximum output size that may be allocated, default is 94 MiB (enough to decode 8-bit color 8K image). /// /// An error occurred during the operation. /// Thrown when memory for the decoder could not be allocated. @@ -291,7 +291,7 @@ class jpegls_decoder final /// The passed container needs to remain valid until the stream is fully decoded. /// /// - /// A STL like container that provides the functions data() and size() and the type value_type. + /// STL like container that provides the functions data() and size() and the type value_type. /// /// /// If true the SPIFF and JPEG header will be directly read from the source. @@ -324,7 +324,7 @@ class jpegls_decoder final /// This container needs to remain valid until the stream is fully decoded. /// /// - /// A STL like container that provides the functions data() and size() and the type value_type. + /// STL like container that provides the functions data() and size() and the type value_type. /// /// An error occurred during the operation. template @@ -335,9 +335,9 @@ class jpegls_decoder final /// /// Tries to read the SPIFF header from the JPEG-LS stream. - /// If a SPIFF header exists its will be returned otherwise the struct will be filled with default values. + /// If a SPIFF header exists it will be returned otherwise the struct will be filled with default values. /// The header_found parameter will be set to true if the spiff header could be read. - /// Call read_header to read the normal JPEG header afterwards. + /// Call read_header to read the normal JPEG header afterward. /// /// An error occurred during the operation. /// True if a valid SPIFF header could be found. @@ -351,9 +351,9 @@ class jpegls_decoder final /// /// Tries to read the SPIFF header from the JPEG-LS stream. - /// If a SPIFF header exists its will be returned otherwise the struct will be filled with default values. + /// If a SPIFF header exists it will be returned otherwise the struct will be filled with default values. /// The header_found parameter will be set to true if the spiff header could be read. - /// Call read_header to read the normal JPEG header afterwards. + /// Call read_header to read the normal JPEG header afterward. /// /// The out-parameter for error reporting. /// True if a valid SPIFF header could be found. @@ -401,7 +401,7 @@ class jpegls_decoder final /// /// Returns true if a valid SPIFF header was found. /// - /// True of false, depending if a SPIFF header was found. + /// True when a SPIFF header was found, false otherwise. [[nodiscard]] bool spiff_header_has_value() const noexcept { @@ -541,7 +541,7 @@ class jpegls_decoder final /// Will decode the JPEG-LS byte stream set with source into the destination container. /// /// - /// A STL like container that provides the functions data() and size() and the type value_type. + /// STL like container that provides the functions data() and size() and the type value_type. /// /// Number of bytes to the next line in the buffer, when zero, decoder will compute it. /// An error occurred during the operation. diff --git a/include/charls/charls_jpegls_encoder.h b/include/charls/charls_jpegls_encoder.h index 28053b49..d09173d9 100644 --- a/include/charls/charls_jpegls_encoder.h +++ b/include/charls/charls_jpegls_encoder.h @@ -85,7 +85,7 @@ charls_jpegls_encoder_set_interleave_mode(CHARLS_IN charls_jpegls_encoder* encod /// /// Configures the preset coding parameters the encoder should use. /// If not set the encoder will use the default preset coding parameters as defined by the JPEG-LS standard. -/// Only when the coding parameters are different than the default parameters or when `include_pc_parameters_jai` is set, +/// Only when the coding parameters are different from the default parameters or when `include_pc_parameters_jai` is set, /// they will be written to the JPEG-LS stream during the encode phase. /// /// Reference to the encoder instance. @@ -99,7 +99,7 @@ charls_jpegls_encoder_set_preset_coding_parameters(CHARLS_IN charls_jpegls_encod /// /// Configures the HP color transformation the encoder should use. /// If not set the encoder will use no color transformation. -/// Color transformations are a HP extension and not defined by the JPEG-LS standard and can only be set for 3 component +/// Color transformations are an HP extension and not defined by the JPEG-LS standard and can only be set for 3 component /// encodings. /// /// Reference to the encoder instance. @@ -117,7 +117,7 @@ charls_jpegls_encoder_set_color_transformation(CHARLS_IN charls_jpegls_encoder* /// Size for dynamic extras like SPIFF entries and other tables are not included in this size. /// /// Reference to the encoder instance. -/// Reference to the size that will be set when the functions returns. +/// Reference to the size that will be set when the function returns. /// The result of the operation: success or a failure code. CHARLS_CHECK_RETURN CHARLS_API_IMPORT_EXPORT charls_jpegls_errc CHARLS_API_CALLING_CONVENTION charls_jpegls_encoder_get_estimated_destination_size(CHARLS_IN const charls_jpegls_encoder* encoder, @@ -186,7 +186,7 @@ charls_jpegls_encoder_write_spiff_entry(CHARLS_IN charls_jpegls_encoder* encoder /// /// Writes a SPIFF end of directory entry to the destination. -/// The encoder will normally does this automatically. It is made available +/// The encoder will normally do this automatically. It is made available /// for the scenario to create SPIFF headers in front of existing JPEG-LS streams. /// /// @@ -204,7 +204,7 @@ charls_jpegls_encoder_write_spiff_end_of_directory_entry(CHARLS_IN charls_jpegls /// Function should be called before encoding the image data. /// /// Reference to the encoder instance. -/// The 'comment' bytes. Application specific, usually human readable string. +/// The 'comment' bytes. Application specific, usually a human-readable string. /// The size in bytes of the comment [0-65533]. /// The result of the operation: success or a failure code. CHARLS_ATTRIBUTE_ACCESS((access(read_only, 2, 3))) @@ -252,7 +252,7 @@ charls_jpegls_encoder_encode_from_buffer(CHARLS_IN charls_jpegls_encoder* encode /// Returns the size in bytes, that are written to the destination. /// /// Reference to the encoder instance. -/// Reference to the size that will be set when the functions returns. +/// Reference to the size that will be set when the function returns. /// The result of the operation: success or a failure code. CHARLS_CHECK_RETURN CHARLS_API_IMPORT_EXPORT charls_jpegls_errc CHARLS_API_CALLING_CONVENTION charls_jpegls_encoder_get_bytes_written(CHARLS_IN const charls_jpegls_encoder* encoder, @@ -368,7 +368,7 @@ class jpegls_encoder final /// /// Configures the HP color transformation the encoder should use. /// If not set the encoder will use no color transformation. - /// Color transformations are a HP extension and not defined by the JPEG-LS standard + /// Color transformations are an HP extension and not defined by the JPEG-LS standard /// and can only be set for 3 component encodings. /// /// The color transformation parameters. @@ -409,7 +409,7 @@ class jpegls_encoder final } /// - /// Set the the container that will contain the encoded JPEG-LS byte stream data after encoding. + /// Set the container that will contain the encoded JPEG-LS byte stream data after encoding. /// This container needs to remain valid during the encoding process. /// /// @@ -471,7 +471,7 @@ class jpegls_encoder final /// /// Writes a SPIFF end of directory entry to the destination. - /// The encoder will normally does this automatically. It is made available + /// The encoder will normally do this automatically. It is made available /// for the scenario to create SPIFF headers in front of existing JPEG-LS streams. /// /// diff --git a/include/charls/public_types.h b/include/charls/public_types.h index e707be33..8be53f06 100644 --- a/include/charls/public_types.h +++ b/include/charls/public_types.h @@ -262,12 +262,12 @@ enum class [[nodiscard]] jpegls_errc invalid_marker_segment_size = impl::CHARLS_JPEGLS_ERRC_INVALID_MARKER_SEGMENT_SIZE, /// - /// This error is returned when the stream contains more then one SOI marker. + /// This error is returned when the stream contains more than one SOI marker. /// duplicate_start_of_image_marker = impl::CHARLS_JPEGLS_ERRC_DUPLICATE_START_OF_IMAGE_MARKER, /// - /// This error is returned when the stream contains more then one SOF marker. + /// This error is returned when the stream contains more than one SOF marker. /// duplicate_start_of_frame_marker = impl::CHARLS_JPEGLS_ERRC_DUPLICATE_START_OF_FRAME_MARKER, @@ -309,7 +309,7 @@ enum class [[nodiscard]] jpegls_errc restart_marker_not_found = impl::CHARLS_JPEGLS_ERRC_RESTART_MARKER_NOT_FOUND, /// - /// This error is returned when a callback function returns a non zero value. + /// This error is returned when a callback function returns a nonzero value. /// callback_failed = impl::CHARLS_JPEGLS_ERRC_CALLBACK_FAILED, @@ -380,12 +380,12 @@ enum class [[nodiscard]] jpegls_errc invalid_argument_encoding_options = impl::CHARLS_JPEGLS_ERRC_INVALID_ARGUMENT_ENCODING_OPTIONS, /// - /// This error is returned when the stream contains a width parameter defined more then once or in an incompatible way. + /// This error is returned when the stream contains a width parameter defined more than once or in an incompatible way. /// invalid_parameter_width = impl::CHARLS_JPEGLS_ERRC_INVALID_PARAMETER_WIDTH, /// - /// This error is returned when the stream contains a height parameter defined more then once in an incompatible way. + /// This error is returned when the stream contains a height parameter defined more than once in an incompatible way. /// invalid_parameter_height = impl::CHARLS_JPEGLS_ERRC_INVALID_PARAMETER_HEIGHT, @@ -477,7 +477,7 @@ enum class encoding_options : unsigned /// /// Writes explicitly the default JPEG-LS preset coding parameters when the - /// bits per sample is larger then 12 bits. + /// bits per sample is larger than 12 bits. /// The Java Advanced Imaging (JAI) JPEG-LS codec has a defect that causes it to use invalid /// preset coding parameters for these types of images. /// Most users of this codec are aware of this problem and have implemented a work-around. @@ -533,7 +533,7 @@ enum class color_transformation hp2 = impl::CHARLS_COLOR_TRANSFORMATION_HP2, /// - /// Defines the reversible lossless color transformation of Y-Cb-Cr): + /// Defines the reversible lossless color transformation of Y-Cb-Cr: /// R = R - G /// B = B - G /// G = G + (R + B) / 4 @@ -841,7 +841,7 @@ typedef int32_t charls_spiff_resolution_units; /// Defines the information that can be stored in a SPIFF header as defined in ISO/IEC 10918-3, Annex F /// /// -/// The type I.8 is an unsigned 8 bit integer, the type I.32 is an 32 bit unsigned integer in the file header itself. +/// The type I.8 is an unsigned 8-bit integer, the type I.32 is an 32-bit unsigned integer in the file header itself. /// The type is indicated by the symbol “F.” are 4-byte parameters in “fixed point” notation. /// The 16 most significant bits are essentially the same as a parameter of type I.16 and indicate the integer /// part of this number. @@ -947,7 +947,7 @@ using charls_at_comment_handler = int32_t(CHARLS_API_CALLING_CONVENTION*)(const /// /// /// -/// Id of the APPn segment [0 .. 15]. +/// Identifier of the APPn segment [0 - 15]. /// Reference to the data of the APPn segment. /// Size in bytes of the data of the APPn segment. /// Free to use context information that can be set during the installation of the diff --git a/src/charls_jpegls_encoder.cpp b/src/charls_jpegls_encoder.cpp index 1a374b0a..eb64c40e 100644 --- a/src/charls_jpegls_encoder.cpp +++ b/src/charls_jpegls_encoder.cpp @@ -122,15 +122,14 @@ struct charls_jpegls_encoder final vertical_resolution, horizontal_resolution}); } - void write_spiff_entry(const uint32_t entry_tag, CHARLS_IN_READS_BYTES(entry_data_size_bytes) const void* entry_data, - const size_t entry_data_size_bytes) + void write_spiff_entry(const uint32_t entry_tag, const span entry_data) { - check_argument(entry_data || entry_data_size_bytes == 0); + check_argument(entry_data.data() || entry_data.empty()); check_argument(entry_tag != spiff_end_of_directory_entry_type); - check_argument(entry_data_size_bytes <= 65528, jpegls_errc::invalid_argument_size); + check_argument(entry_data.size() <= 65528, jpegls_errc::invalid_argument_size); check_operation(state_ == state::spiff_header); - writer_.write_spiff_directory_entry(entry_tag, entry_data, entry_data_size_bytes); + writer_.write_spiff_directory_entry(entry_tag, entry_data); } void write_spiff_end_of_directory_entry() @@ -541,7 +540,7 @@ charls_jpegls_encoder_write_spiff_entry(charls_jpegls_encoder* encoder, const ui const size_t entry_data_size_bytes) noexcept try { - check_pointer(encoder)->write_spiff_entry(entry_tag, entry_data, entry_data_size_bytes); + check_pointer(encoder)->write_spiff_entry(entry_tag, {static_cast(entry_data), entry_data_size_bytes}); return jpegls_errc::success; } catch (...) diff --git a/src/default_traits.h b/src/default_traits.h index e501cf3b..471813bc 100644 --- a/src/default_traits.h +++ b/src/default_traits.h @@ -101,7 +101,7 @@ struct default_traits final /// /// Returns the value of errorValue modulo RANGE. ITU.T.87, A.4.5 (code segment A.9) - /// This ensures the error is reduced to the range (-⌊RANGE/2⌋ .. ⌈RANGE/2⌉-1) + /// This ensures the error is reduced to the range (-⌊RANGE/2⌋ - ⌈RANGE/2⌉-1) /// [[nodiscard]] FORCE_INLINE int32_t modulo_range(int32_t error_value) const noexcept diff --git a/src/jpeg_stream_writer.cpp b/src/jpeg_stream_writer.cpp index ec4bc712..61fb1fd6 100644 --- a/src/jpeg_stream_writer.cpp +++ b/src/jpeg_stream_writer.cpp @@ -44,11 +44,11 @@ void jpeg_stream_writer::write_spiff_header_segment(const spiff_header& header) ASSERT(header.height > 0); ASSERT(header.width > 0); - static constexpr array spiff_magic_id{{'S', 'P', 'I', 'F', 'F', '\0'}}; + static constexpr array spiff_magic_id{byte{'S'}, byte{'P'}, byte{'I'}, byte{'F'}, byte{'F'}, byte{'\0'}}; // Create a JPEG APP8 segment in Still Picture Interchange File Format (SPIFF), v2.0 write_segment_header(jpeg_marker_code::application_data8, 30); - write_bytes(spiff_magic_id.data(), spiff_magic_id.size()); + write_bytes(spiff_magic_id); write_uint8(spiff_major_revision_number); write_uint8(spiff_minor_revision_number); write_uint8(to_underlying_type(header.profile_id)); @@ -64,12 +64,12 @@ void jpeg_stream_writer::write_spiff_header_segment(const spiff_header& header) } -USE_DECL_ANNOTATIONS void jpeg_stream_writer::write_spiff_directory_entry(const uint32_t entry_tag, const void* entry_data, - const size_t entry_data_size_bytes) +USE_DECL_ANNOTATIONS void jpeg_stream_writer::write_spiff_directory_entry(const uint32_t entry_tag, + const span entry_data) { - write_segment_header(jpeg_marker_code::application_data8, sizeof(uint32_t) + entry_data_size_bytes); + write_segment_header(jpeg_marker_code::application_data8, sizeof(uint32_t) + entry_data.size()); write_uint32(entry_tag); - write_bytes(entry_data, entry_data_size_bytes); + write_bytes(entry_data); } @@ -78,11 +78,10 @@ void jpeg_stream_writer::write_spiff_end_of_directory_entry() // Note: ISO/IEC 10918-3, Annex F.2.2.3 documents that the EOD entry segment should have a length of 8 // but only 6 data bytes. This approach allows to wrap existing bit streams\encoders with a SPIFF header. // In this implementation the SOI marker is added as data bytes to simplify the design. - static constexpr array spiff_end_of_directory{ - {0, 0, 0, spiff_end_of_directory_entry_type, 0xFF, to_underlying_type(jpeg_marker_code::start_of_image)}}; - - write_segment_header(jpeg_marker_code::application_data8, spiff_end_of_directory.size()); - write_bytes(spiff_end_of_directory.data(), spiff_end_of_directory.size()); + static constexpr array spiff_end_of_directory{byte{0}, byte{0}, + byte{0}, byte{spiff_end_of_directory_entry_type}, + byte{0xFF}, byte{to_underlying_type(jpeg_marker_code::start_of_image)}}; + write_segment(jpeg_marker_code::application_data8, spiff_end_of_directory); } @@ -122,17 +121,14 @@ bool jpeg_stream_writer::write_start_of_frame_segment(const frame_info& frame) void jpeg_stream_writer::write_color_transform_segment(const color_transformation transformation) { - const array segment{'m', 'r', 'f', 'x', static_cast(transformation)}; - - write_segment_header(jpeg_marker_code::application_data8, segment.size()); - write_bytes(segment.data(), segment.size()); + const array segment{byte{'m'}, byte{'r'}, byte{'f'}, byte{'x'}, static_cast(transformation)}; + write_segment(jpeg_marker_code::application_data8, segment); } void jpeg_stream_writer::write_comment_segment(const span comment) { - write_segment_header(jpeg_marker_code::comment, comment.size()); - write_bytes(comment); + write_segment(jpeg_marker_code::comment, comment); } @@ -140,11 +136,9 @@ void jpeg_stream_writer::write_application_data_segment(const int32_t applicatio const span application_data) { ASSERT(application_data_id >= minimum_application_data_id && application_data_id <= maximum_application_data_id); - - write_segment_header( + write_segment( static_cast(static_cast(jpeg_marker_code::application_data0) + application_data_id), - application_data.size()); - write_bytes(application_data); + application_data); } diff --git a/src/jpeg_stream_writer.h b/src/jpeg_stream_writer.h index 718b00aa..4ca714af 100644 --- a/src/jpeg_stream_writer.h +++ b/src/jpeg_stream_writer.h @@ -33,8 +33,7 @@ class jpeg_stream_writer final /// Header info to write into the SPIFF segment. void write_spiff_header_segment(const spiff_header& header); - void write_spiff_directory_entry(uint32_t entry_tag, CHARLS_IN_READS_BYTES(entry_data_size_bytes) const void* entry_data, - size_t entry_data_size_bytes); + void write_spiff_directory_entry(uint32_t entry_tag, span entry_data); /// /// Write a JPEG SPIFF end of directory (APP8) segment. @@ -43,7 +42,7 @@ class jpeg_stream_writer final void write_spiff_end_of_directory_entry(); /// - /// Writes a HP color transformation (APP8) segment. + /// Writes an HP color transformation (APP8) segment. /// /// Color transformation to put into the segment. void write_color_transform_segment(color_transformation transformation); @@ -178,7 +177,8 @@ class jpeg_stream_writer final #else const UnsignedIntType big_endian_value{value}; #endif - write_bytes(&big_endian_value, sizeof big_endian_value); + const void* bytes{&big_endian_value}; + write_bytes({static_cast(bytes), sizeof big_endian_value}); } void write_byte(const std::byte value) noexcept @@ -189,14 +189,9 @@ class jpeg_stream_writer final void write_bytes(const span data) noexcept { - write_bytes(data.data(), data.size()); - } - - void write_bytes(CHARLS_IN_READS_BYTES(size) const void* data, const size_t size) noexcept - { - ASSERT(byte_offset_ + size <= destination_.size()); - memcpy(destination_.data() + byte_offset_, data, size); - byte_offset_ += size; + ASSERT(byte_offset_ + data.size() <= destination_.size()); + memcpy(destination_.data() + byte_offset_, data.data(), data.size()); + byte_offset_ += data.size(); } void write_marker(const jpeg_marker_code marker_code) noexcept @@ -214,6 +209,12 @@ class jpeg_stream_writer final write_byte(static_cast(marker_code)); } + void write_segment(const jpeg_marker_code marker_code, const span data) + { + write_segment_header(marker_code, data.size()); + write_bytes(data); + } + span destination_{}; size_t byte_offset_{}; uint8_t component_id_{1}; diff --git a/src/span.h b/src/span.h index 5ee5304f..29f2f4e9 100644 --- a/src/span.h +++ b/src/span.h @@ -37,6 +37,7 @@ class span final { } + // ReSharper disable once CppNonExplicitConvertingConstructor template constexpr span(const std::array& data) noexcept : data_{data.data()}, size_{Size} { diff --git a/test/main.cpp b/test/main.cpp index 54356f91..f0b51831 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -3,7 +3,6 @@ #include "../src/default_traits.h" #include "../src/lossless_traits.h" -#include "../src/process_decoded_line.h" #include "util.h" diff --git a/unittest/jpeg_stream_writer_test.cpp b/unittest/jpeg_stream_writer_test.cpp index 8ce13980..e7da2d58 100644 --- a/unittest/jpeg_stream_writer_test.cpp +++ b/unittest/jpeg_stream_writer_test.cpp @@ -248,9 +248,9 @@ TEST_CLASS(jpeg_stream_writer_test) array buffer{}; jpeg_stream_writer writer{{buffer.data(), buffer.size()}}; - const array data{byte{0x77}, byte{0x66}}; + constexpr array data{byte{0x77}, byte{0x66}}; - writer.write_spiff_directory_entry(2, data.data(), data.size()); + writer.write_spiff_directory_entry(2, data); // Verify Entry Magic Number (EMN) Assert::AreEqual(byte{0xFF}, buffer[0]); diff --git a/unittest/jpegls_algorithm_test.cpp b/unittest/jpegls_algorithm_test.cpp index f3620727..ffe07647 100644 --- a/unittest/jpegls_algorithm_test.cpp +++ b/unittest/jpegls_algorithm_test.cpp @@ -55,7 +55,7 @@ int32_t unmap_error_value_original(const int32_t mapped_error_value) noexcept /// /// This version will be auto optimized by GCC(trunk, not 10.2) using a cmove, -/// and clang(11.0) 8 instructions,. MSVC will create branches. +/// and clang(11.0) 8 instructions, MSVC will create branches. /// Optimized version uses 6 to 5 instructions. /// int32_t unmap_error_value_alternative1(const int32_t mapped_error_value) noexcept diff --git a/unittest/scan_encoder_test.cpp b/unittest/scan_encoder_test.cpp index 78f5e864..ae42fd91 100644 --- a/unittest/scan_encoder_test.cpp +++ b/unittest/scan_encoder_test.cpp @@ -52,7 +52,7 @@ TEST_CLASS(scan_encoder_test) strategy.append_to_bit_stream_forward(0xffff, 16); strategy.append_to_bit_stream_forward(0xffff, 16); - // Buffer is full with FFs and _isFFWritten = true: Flush can only write 30 date bits. + // Buffer is full of FFs and _isFFWritten = true: Flush can only write 30 date bits. strategy.append_to_bit_stream_forward(0x3, 31); strategy.flush_forward();