Add WrapperDecompressImageEx API that supports use_16bit_dc_estimate …

…argument (#54)

Co-authored-by: Gadi Brovman <[email protected]>

.vscode/launch.json

@@ -4,18 +4,28 @@
     // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
     "version": "0.2.0",
     "configurations": [
-
         {
             "name": "(Windows) Launch",
             "type": "cppvsdbg",
             "request": "launch",
             "program": "${workspaceFolder}/target/debug/lepton_jpeg_util.exe",
-            "args": ["-verify", "${workspaceFolder}\\images\\slrcity.jpg"],
+            "args": ["-dump", "${workspaceFolder}\\images\\slrcity.jpg"],
             "stopAtEntry": false,
             "cwd": "${fileDirname}",
             "environment": [],
             "console": "externalTerminal",
-        }
-
+        },
+        {
+            "name": "Debug unit test",
+            "type": "cppvsdbg",
+            "request": "launch",
+            "program": "${workspaceFolder}/target/debug/deps/end_to_end-5c5d5b533f217bea.exe",
+            "args": [ "verify_extern_16bit_math_retry" ],
+            "stopAtEntry": false,
+            "cwd": "${workspaceFolder}",
+            "environment": [],
+            "console": "externalTerminal",
+            "preLaunchTask": "rust: cargo test norun",
+        },
     ]
 }

.vscode/tasks.json

@@ -0,0 +1,17 @@
+{
+    "version": "2.0.0",
+    "tasks": [
+        {
+            "type": "cargo",
+            "command": "test",
+            "args": [
+                "--no-run"
+            ],
+            "problemMatcher": [
+                "$rustc"
+            ],
+            "group": "test",
+            "label": "rust: cargo test norun"
+        }
+    ]
+}

package/Lepton.Jpeg.Rust.nuspec

@@ -2,7 +2,7 @@
 <package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
   <metadata>
     <id>Lepton.Jpeg.Rust</id>
-    <version>0.3.4.2</version>
+    <version>0.3.4.3</version>
     <title>Lepton JPEG Compression Rust version binaries and libraries</title>
     <authors>kristofr</authors>
     <owners>kristofr</owners>

src/lib.rs

@@ -132,12 +132,42 @@ pub unsafe extern "C" fn WrapperDecompressImage(
     output_buffer_size: u64,
     number_of_threads: i32,
     result_size: *mut u64,
+) -> i32 {
+    return WrapperDecompressImageEx(
+        input_buffer,
+        input_buffer_size,
+        output_buffer,
+        output_buffer_size,
+        number_of_threads,
+        result_size,
+        false, // use_16bit_dc_estimate
+    );
+}
+
+/// C ABI interface for decompressing image, exposed from DLL.
+/// use_16bit_dc_estimate argument should be set to true only for images
+/// that were compressed by C++ version of Leptron (see comments below).
+#[no_mangle]
+pub unsafe extern "C" fn WrapperDecompressImageEx(
+    input_buffer: *const u8,
+    input_buffer_size: u64,
+    output_buffer: *mut u8,
+    output_buffer_size: u64,
+    number_of_threads: i32,
+    result_size: *mut u64,
+    use_16bit_dc_estimate: bool,
 ) -> i32 {
     match catch_unwind(|| {
         // For back-compat with C++ version we allow decompression of images with zeros in DQT tables
         let mut enabled_features = EnabledFeatures::all();
         enabled_features.reject_dqts_with_zeros = false;
 
+        // C++ version has a bug where it uses 16 bit math in the SIMD path and 32 bit math in the scalar path
+        // depending on the compiler options. If use_16bit_dc_estimate=true, the decompression uses a back-compat
+        // mode that considers it. The caller should set use_16bit_dc_estimate to true only for images that were
+        // compressed by C++ version with relevant compiler options.
+        enabled_features.use_16bit_dc_estimate = use_16bit_dc_estimate;
+
         loop {
             let input = std::slice::from_raw_parts(input_buffer, input_buffer_size as usize);
             let output = std::slice::from_raw_parts_mut(output_buffer, output_buffer_size as usize);
@@ -158,12 +188,14 @@ pub unsafe extern "C" fn WrapperDecompressImage(
                 Err(e) => {
                     let exit_code = translate_error(e).exit_code;
 
-                    // there's a bug in the C++ version where it uses 16 bit math in the SIMD path and 32 bit math in the scalar path depending on the compiler options.
-                    // unfortunately there's no way to tell ahead of time other than the fact that the image will be decoded with an error.
+                    // The retry logic below runs if the caller did not pass use_16bit_dc_estimate=true, but the decompression
+                    // encountered StreamInconsistent failure which is commonly caused by the the C++ 16 bit bug. In this case
+                    // we retry the decompression with use_16bit_dc_estimate=true.
+                    // Note that it's prefferable for the caller to pass use_16bit_dc_estimate properly and not to rely on this
+                    // retry logic, that may miss some cases leading to bad (corrupted) decompression results.
                     if exit_code == ExitCode::StreamInconsistent
                         && !enabled_features.use_16bit_dc_estimate
                     {
-                        // try again with the flag set
                         enabled_features.use_16bit_dc_estimate = true;
                         continue;
                     }

src/main.rs

@@ -75,6 +75,8 @@ fn main_with_result() -> anyhow::Result<()> {
                 enabled_features.progressive = false;
             } else if args[i] == "-acceptdqtswithzeros" {
                 enabled_features.reject_dqts_with_zeros = false;
+            } else if args[i] == "-use16bitdc" {
+                enabled_features.use_16bit_dc_estimate = true;
             } else {
                 return err_exit_code(
                     ExitCode::SyntaxError,

tests/end_to_end.rs

@@ -16,7 +16,7 @@ use lepton_jpeg::{
     lepton_error::{ExitCode, LeptonError},
     EnabledFeatures,
 };
-use lepton_jpeg::{WrapperCompressImage, WrapperDecompressImage};
+use lepton_jpeg::{WrapperCompressImage, WrapperDecompressImage, WrapperDecompressImageEx};
 
 use rstest::rstest;
 
@@ -90,6 +90,78 @@ fn verify_decode(
     assert!(output[..] == expected[..]);
 }
 
+/// Verifies that the decode will accept existing Lepton files and generate
+/// exactly the same jpeg from them when called by an external interface
+/// with use_16bit_dc_estimate=true for C++ backward compatibility.
+/// Used to detect unexpected divergences in coding format.
+#[rstest]
+fn verify_decode_external_interface_with_use_16bit_dc_estimate(
+    #[values(
+        "mathoverflow_16",
+        "android",
+        "androidcrop",
+        "androidcropoptions",
+        "androidprogressive",
+        "androidprogressive_garbage",
+        "androidtrail",
+        "colorswap",
+        "gray2sf",
+        "grayscale",
+        "hq",
+        "iphone",
+        "iphonecity",
+        "iphonecity_with_16KGarbage",
+        "iphonecity_with_1MGarbage",
+        "iphonecrop",
+        "iphonecrop2",
+        "iphoneprogressive",
+        "iphoneprogressive2",
+        "progressive_late_dht", // image has huffman tables that come very late which causes a verification failure 
+        "out_of_order_dqt",     // image with quanatization table dqt that comes after image definition SOF
+        "narrowrst",
+        "nofsync",
+        "slrcity",
+        "slrhills",
+        "slrindoor",
+        "tiny",
+        "trailingrst",
+        "trailingrst2",
+        "trunc",
+        "eof_and_trailingrst",    // the lepton format has a wrongly set unexpected eof and trailing rst
+        "eof_and_trailinghdrdata" // the lepton format has a wrongly set unexpected eof and trailing header data
+    )]
+    file: &str,
+) {
+    println!("decoding {0:?}", file);
+
+    let compressed = read_file(file, ".lep");
+    let jpg_file_name = match file {
+        "mathoverflow_16" => "mathoverflow",
+        _ => file,
+    };
+    let input = read_file(jpg_file_name, ".jpg");
+
+    let mut original = Vec::new();
+    original.resize(input.len() + 10000, 0);
+
+    let mut original_size: u64 = 0;
+    unsafe {
+        let retval = WrapperDecompressImageEx(
+            compressed[..].as_ptr(),
+            compressed.len() as u64,
+            original[..].as_mut_ptr(),
+            original.len() as u64,
+            8,
+            (&mut original_size) as *mut u64,
+            true, // use_16bit_dc_estimate
+        );
+
+        assert_eq!(retval, 0);
+    }
+    assert_eq!(input.len() as u64, original_size);
+    assert_eq!(input[..], original[..(original_size as usize)]);
+}
+
 /// encodes as LEP and codes back to JPG to mostly test the encoder. Can't check against
 /// the original LEP file since there's no guarantee they are binary identical (especially the zlib encoded part)
 #[rstest]