Add support to decode JPEG-LS images with restart markers (#99)

* Add support to parse DRI segments

* Add support to decode restart markers

* Update CharLSTest to also decode color images none interleaved to ppm

* Replace static_cast<void> with std::ignore

* Update readme, changelog and refactor

.clang-tidy

@@ -19,6 +19,7 @@
 # -clang-diagnostic-switch-enum => Rationale: options are handled by default case
 # -clang-diagnostic-exit-time-destructors => Rationale: Acceptable construction
 # -clang-diagnostic-pragma-once-outside-header => Rationale: Generates false warnings for usage in header files
+# -clang-diagnostic-unused-const-variable => Rationale: false warnings for constexpr in .h files
 # -clang-analyzer-core.NonNullParamChecker => Rationale: cannot be effective disabled, already checked by other checkers.
 # -misc-non-private-member-variables-in-classes => Rationale: design can be ok, manual review is better
 # -modernize-use-trailing-return-type => Rationale: A style recommendation, this style is selected for CharLS
@@ -58,6 +59,7 @@ Checks:          '*,
                   -clang-diagnostic-switch-enum,
                   -clang-diagnostic-exit-time-destructors,
                   -clang-diagnostic-pragma-once-outside-header,
+                  -clang-diagnostic-unused-const-variable,
                   -clang-analyzer-core.NonNullParamChecker,
                   -misc-non-private-member-variables-in-classes,
                   -modernize-use-trailing-return-type,

CHANGELOG.md

@@ -6,14 +6,18 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 ## [Next-Release]
 
+### Added
+
+- The encoder API has been extended with a rewind method that can be used to re-use a configered encoder to encode multiple images in a loop.
+- Added support to decode JPEG-LS images that use restart markers [#92](https://github.com/team-charls/charls/issues/92)
+
 ### Fixed
 
 - Fixed [#84](https://github.com/team-charls/charls/issues/84), Default preset coding parameters not computed for unset values.
 
 ### Changed
 
 - CMakeSettings.json has been replaced with CMakePresets.json.
-- The encoder API has been extended with a rewind method that can be used to re-use a configered encoder to encode multiple images in a loop.
 
 ## [2.2.0] - 2021-1-10
 

CharLS.sln.DotSettings

@@ -6,6 +6,7 @@
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyClangDiagnosticExitTimeDestructors/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyClangDiagnosticMissingBraces/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyClangDiagnosticSwitchEnum/@EntryIndexedValue">DO_NOT_SHOW</s:String>
+	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyClangDiagnosticUnusedConstVariable/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyCppcoreguidelinesAvoidMagicNumbers/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyCppcoreguidelinesInitVariables/@EntryIndexedValue">DO_NOT_SHOW</s:String>
 	<s:String x:Key="/Default/CodeInspection/Highlighting/InspectionSeverities/=CppClangTidyCppcoreguidelinesMacroUsage/@EntryIndexedValue">DO_NOT_SHOW</s:String>
@@ -46,6 +47,7 @@
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=bugprone/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=CHARLS/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=charlstest/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=cmove/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=cmyk/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=cpixel/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=cppcoreguidelines/@EntryIndexedValue">True</s:Boolean>

README.md

@@ -34,8 +34,8 @@ According to preliminary test results published on <http://imagecompression.info
 
 The following JPEG-LS options are not supported by the CharLS implementation. Most of these options are rarely used in practice.
 
-* No support for JPEG restart markers (RST).
-  Restart markers make it possible to recover from corrupted JPEG files, but are seldom used for data recovery scenarios.
+* No support to encode JPEG restart markers  
+  Decoding is supported, but no recovery mechanisme is implemented for corrupted JPEG-LS files.
 * No support for sub-sampled scans.
   Sub-sampling is a lossly encoding mechanism and not used in lossless scenarios.
 * No support for multi component frames with mixed component counts in a single scan.

include/charls/public_types.h

@@ -50,6 +50,8 @@ enum charls_jpegls_errc
     CHARLS_JPEGLS_ERRC_INVALID_JPEGLS_PRESET_PARAMETER_TYPE = 22,
     CHARLS_JPEGLS_ERRC_JPEGLS_PRESET_EXTENDED_PARAMETER_TYPE_NOT_SUPPORTED = 23,
     CHARLS_JPEGLS_ERRC_MISSING_END_OF_SPIFF_DIRECTORY = 24,
+    CHARLS_JPEGLS_ERRC_UNEXPECTED_RESTART_MARKER = 25,
+    CHARLS_JPEGLS_ERRC_RESTART_MARKER_NOT_FOUND = 26,
     CHARLS_JPEGLS_ERRC_INVALID_ARGUMENT_WIDTH = 100,
     CHARLS_JPEGLS_ERRC_INVALID_ARGUMENT_HEIGHT = 101,
     CHARLS_JPEGLS_ERRC_INVALID_ARGUMENT_COMPONENT_COUNT = 102,
@@ -285,6 +287,16 @@ enum class CHARLS_NO_DISCARD jpegls_errc
     /// </summary>
     missing_end_of_spiff_directory = impl::CHARLS_JPEGLS_ERRC_MISSING_END_OF_SPIFF_DIRECTORY,
 
+    /// <summary>
+    /// This error is returned when a restart marker is found outside the encoded entropy data.
+    /// </summary>
+    unexpected_restart_marker = impl::CHARLS_JPEGLS_ERRC_UNEXPECTED_RESTART_MARKER,
+
+    /// <summary>
+    /// This error is returned when an expected restart marker is not found. It may indicate data corruption in the JPEG-LS byte stream.
+    /// </summary>
+    restart_marker_not_found = impl::CHARLS_JPEGLS_ERRC_RESTART_MARKER_NOT_FOUND,
+
     /// <summary>
     /// The argument for the width parameter is outside the range [1, 65535].
     /// </summary>

src/charls_jpegls_encoder.cpp

@@ -210,7 +210,7 @@ struct charls_jpegls_encoder final
                                             component_count};
 
         const auto codec{jls_codec_factory<encoder_strategy>().create_codec(
-            frame_info, {near_lossless_, interleave_mode_, color_transformation_, false}, validated_pc_parameters_)};
+            frame_info, {near_lossless_, 0, interleave_mode_, color_transformation_, false}, validated_pc_parameters_)};
         std::unique_ptr<process_line> process_line(codec->create_process_line(source, stride));
         const size_t bytes_written{codec->encode_scan(move(process_line), writer_.remaining_destination())};
 

src/coding_parameters.h

@@ -10,6 +10,7 @@ namespace charls {
 struct coding_parameters final
 {
     int32_t near_lossless;
+    uint32_t restart_interval;
     charls::interleave_mode interleave_mode;
     color_transformation transformation;
     bool output_bgr;

src/context.h

@@ -64,7 +64,7 @@ struct jls_context final
             {
                 b = -n + 1;
             }
-            C = C - (C > -128);
+            C = C - static_cast<int16_t>(C > -128);
         }
         else if (b > 0)
         {
@@ -73,7 +73,7 @@ struct jls_context final
             {
                 b = 0;
             }
-            C = C + (C < 127);
+            C = C + static_cast<int16_t>(C < 127);
         }
         B = b;
 

src/decoder_strategy.h

@@ -30,7 +30,7 @@ class decoder_strategy
     decoder_strategy& operator=(decoder_strategy&&) = delete;
 
     virtual std::unique_ptr<process_line> create_process_line(byte_span destination, size_t stride) = 0;
-    virtual void set_presets(const jpegls_pc_parameters& preset_coding_parameters) = 0;
+    virtual void set_presets(const jpegls_pc_parameters& preset_coding_parameters, uint32_t restart_interval) = 0;
     virtual void decode_scan(std::unique_ptr<process_line> output_data, const JlsRect& size, byte_span& compressed_data) = 0;
 
     void initialize(const byte_span source)
@@ -45,6 +45,15 @@ class decoder_strategy
         make_valid();
     }
 
+    void reset()
+    {
+        valid_bits_ = 0;
+        read_cache_ = 0;
+
+        next_ff_position_ = find_next_ff();
+        make_valid();
+    }
+
     FORCE_INLINE void skip(const int32_t length) noexcept
     {
         valid_bits_ -= length;
@@ -246,6 +255,14 @@ class decoder_strategy
         return (read_value(length - 24) << 24) + read_value(24);
     }
 
+    uint8_t read_byte() noexcept
+    {
+        // TODO: check end_position first.
+        const uint8_t value = *position_;
+        ++position_;
+        return value;
+    }
+
 protected:
     frame_info frame_info_;
     coding_parameters parameters_;

src/encoder_strategy.h

@@ -25,7 +25,7 @@ class encoder_strategy
     encoder_strategy& operator=(encoder_strategy&&) = delete;
 
     virtual std::unique_ptr<process_line> create_process_line(byte_span stream_info, size_t stride) = 0;
-    virtual void set_presets(const jpegls_pc_parameters& preset_coding_parameters) = 0;
+    virtual void set_presets(const jpegls_pc_parameters& preset_coding_parameters, uint32_t restart_interval) = 0;
     virtual size_t encode_scan(std::unique_ptr<process_line> raw_data, byte_span destination) = 0;
 
     int32_t peek_byte();

src/jpeg_marker_code.h

@@ -16,12 +16,15 @@ namespace charls {
 // 0x4F - 0x6F, 0x90 - 0x93 are defined in ISO/IEC 15444-1: JPEG 2000
 
 constexpr uint8_t jpeg_marker_start_byte{0xFF};
+constexpr uint8_t jpeg_restart_marker_base{0xD0}; // RSTm: Marks the next restart interval (range is D0..D7)
+constexpr uint32_t jpeg_restart_marker_range{8};
 
 enum class jpeg_marker_code : uint8_t
 {
-    start_of_image = 0xD8, // SOI: Marks the start of an image.
-    end_of_image = 0xD9,   // EOI: Marks the end of an image.
-    start_of_scan = 0xDA,  // SOS: Marks the start of scan.
+    start_of_image = 0xD8,          // SOI: Marks the start of an image.
+    end_of_image = 0xD9,            // EOI: Marks the end of an image.
+    start_of_scan = 0xDA,           // SOS: Marks the start of scan.
+    define_restart_interval = 0xDD, // DRI: Defines the restart interval used in succeeding scans.
 
     // The following markers are defined in ISO/IEC 10918-1 | ITU T.81.
     start_of_frame_baseline_jpeg = 0xC0,       // SOF_0:  Marks the start of a baseline jpeg encoded frame.
@@ -55,8 +58,8 @@ enum class jpeg_marker_code : uint8_t
     application_data10 = 0xEA, // APP10: Application data 10.
     application_data11 = 0xEB, // APP11: Application data 11.
     application_data12 = 0xEC, // APP12: Application data 12: used for Picture info.
-    application_data13 = 0xEE, // APP13: Application data 13: used by PhotoShop IRB
-    application_data14 = 0xED, // APP14: Application data 14: used by Adobe
+    application_data13 = 0xED, // APP13: Application data 13: used by PhotoShop IRB
+    application_data14 = 0xEE, // APP14: Application data 14: used by Adobe
     application_data15 = 0xEF, // APP15: Application data 15.
     comment = 0xFE             // COM:   Comment block.
 };

src/jpeg_stream_reader.cpp

@@ -14,6 +14,7 @@
 #include <algorithm>
 #include <array>
 #include <memory>
+#include <tuple>
 
 namespace charls {
 
@@ -24,6 +25,16 @@ using std::find;
 using std::unique_ptr;
 using std::vector;
 
+namespace {
+
+constexpr bool is_restart_marker_code(const jpeg_marker_code marker_code) noexcept
+{
+    return static_cast<uint8_t>(marker_code) >= jpeg_restart_marker_base &&
+           static_cast<uint8_t>(marker_code) < jpeg_restart_marker_base + jpeg_restart_marker_range;
+}
+
+} // namespace
+
 jpeg_stream_reader::jpeg_stream_reader(const byte_span source) noexcept : source_{source}
 {
 }
@@ -212,6 +223,7 @@ void jpeg_stream_reader::validate_marker_code(const jpeg_marker_code marker_code
 
         return;
 
+    case jpeg_marker_code::define_restart_interval:
     case jpeg_marker_code::jpegls_preset_parameters:
     case jpeg_marker_code::comment:
     case jpeg_marker_code::application_data0:
@@ -253,6 +265,9 @@ void jpeg_stream_reader::validate_marker_code(const jpeg_marker_code marker_code
         throw_jpegls_error(jpegls_errc::unexpected_end_of_image_marker);
     }
 
+    if (is_restart_marker_code(marker_code))
+        throw_jpegls_error(jpegls_errc::unexpected_restart_marker);
+
     throw_jpegls_error(jpegls_errc::unknown_jpeg_marker_found);
 }
 
@@ -283,6 +298,9 @@ int jpeg_stream_reader::read_marker_segment(const jpeg_marker_code marker_code,
     case jpeg_marker_code::jpegls_preset_parameters:
         return read_preset_parameters_segment(segment_size);
 
+    case jpeg_marker_code::define_restart_interval:
+        return read_define_restart_interval(segment_size);
+
     case jpeg_marker_code::application_data0:
     case jpeg_marker_code::application_data1:
     case jpeg_marker_code::application_data2:
@@ -422,6 +440,30 @@ int jpeg_stream_reader::read_preset_parameters_segment(const int32_t segment_siz
 }
 
 
+int jpeg_stream_reader::read_define_restart_interval(const int32_t segment_size)
+{
+    // Note: The JPEG-LS standard supports a 2,3 or 4 byte restart interval (see ISO/IEC 14495-1, C.2.5)
+    //       The original JPEG standard only supports 2 bytes (16 bit big endian).
+    switch (segment_size)
+    {
+    case 2:
+        parameters_.restart_interval = read_uint16();
+        return 2;
+
+    case 3:
+        parameters_.restart_interval = read_uint24();
+        return 3;
+
+    case 4:
+        parameters_.restart_interval = read_uint32();
+        return 4;
+
+    default:
+        throw_jpegls_error(jpegls_errc::invalid_marker_segment_size);
+    }
+}
+
+
 void jpeg_stream_reader::read_start_of_scan()
 {
     const int32_t segment_size{read_segment_size()};
@@ -472,16 +514,24 @@ uint8_t jpeg_stream_reader::read_byte()
 
 void jpeg_stream_reader::skip_byte()
 {
-    static_cast<void>(read_byte());
+    std::ignore = read_byte();
 }
 
 
 uint16_t jpeg_stream_reader::read_uint16()
 {
-    const uint16_t value = read_byte() * 256U;
+    const uint32_t value{read_byte() * 256U};
     return static_cast<uint16_t>(value + read_byte());
 }
 
+
+uint32_t jpeg_stream_reader::read_uint24()
+{
+    const uint32_t value{static_cast<uint32_t>(read_byte()) << 16U};
+    return value + read_uint16();
+}
+
+
 uint32_t jpeg_stream_reader::read_uint32()
 {
     uint32_t value{read_uint16()};
@@ -491,6 +541,7 @@ uint32_t jpeg_stream_reader::read_uint32()
     return value;
 }
 
+
 int32_t jpeg_stream_reader::read_segment_size()
 {
     const int32_t segment_size{read_uint16()};

src/jpeg_stream_reader.h

@@ -60,6 +60,7 @@ class jpeg_stream_reader final
 private:
     void skip_byte();
     uint16_t read_uint16();
+    uint32_t read_uint24();
     uint32_t read_uint32();
     int32_t read_segment_size();
     std::vector<uint8_t> read_bytes(size_t byte_count);
@@ -74,6 +75,7 @@ class jpeg_stream_reader final
     int read_start_of_frame_segment(int32_t segment_size);
     static int read_comment() noexcept;
     int read_preset_parameters_segment(int32_t segment_size);
+    int read_define_restart_interval(int32_t segment_size);
     int try_read_application_data8_segment(int32_t segment_size, spiff_header* header, bool* spiff_header_found);
     int try_read_spiff_header_segment(OUT_ spiff_header& header, OUT_ bool& spiff_header_found);
 

src/jpeg_stream_writer.cpp

@@ -111,7 +111,7 @@ void jpeg_stream_writer::write_start_of_frame_segment(const uint32_t width, cons
 
 void jpeg_stream_writer::write_color_transform_segment(const color_transformation transformation)
 {
-    array<uint8_t, 5> segment{'m', 'r', 'f', 'x', static_cast<uint8_t>(transformation)};
+    const array<uint8_t, 5> segment{'m', 'r', 'f', 'x', static_cast<uint8_t>(transformation)};
 
     write_segment_header(jpeg_marker_code::application_data8, segment.size());
     write_bytes(segment.data(), segment.size());

src/jpegls.cpp

@@ -125,7 +125,7 @@ unique_ptr<Strategy> jls_codec_factory<Strategy>::create_codec(const frame_info&
         }
     }
 
-    codec->set_presets(preset_coding_parameters);
+    codec->set_presets(preset_coding_parameters, parameters.restart_interval);
     return codec;
 }
 

src/jpegls_error.cpp

@@ -102,6 +102,12 @@ const char* CHARLS_API_CALLING_CONVENTION charls_get_error_message(const charls_
     case jpegls_errc::missing_end_of_spiff_directory:
         return "Invalid JPEG-LS stream, SPIFF header without End Of Directory (EOD) entry";
 
+    case jpegls_errc::unexpected_restart_marker:
+        return "Invalid JPEG-LS stream, restart (RTSm) marker found outside encoded entropy data";
+
+    case jpegls_errc::restart_marker_not_found:
+        return "Invalid JPEG-LS stream, missing expected restart (RTSm) marker";
+
     case jpegls_errc::invalid_parameter_bits_per_sample:
         return "Invalid JPEG-LS stream, The bit per sample (sample precision) parameter is not in the range [2, 16]";