Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WrapperDecompressImageEx API that supports use_16bit_dc_estimate #54

Merged
merged 1 commit into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
]
}
17 changes: 17 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
2 changes: 1 addition & 1 deletion package/Lepton.Jpeg.Rust.nuspec
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
38 changes: 35 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
74 changes: 73 additions & 1 deletion tests/end_to_end.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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]
Expand Down