Pure Rust lib for zlib

[bvssvni]

There are many people interested in this, so I'm opening this issue for discussion. The PR #418 replaces the Rust implementation, which currently has some bugs, with flate2.

[nwin]

It does not seem to be so trivial. There are three independent Rust zlib inflaters I know of:

1. Ours
2. The one from @alexcrichton in https://github.com/alexcrichton/rust-compress
3. The one from @eddyb in https://github.com/cmr/TEMP-rust-png

They all work ok for some images but they all fail for the examples given in #300 and #376. They are working fine using flate2.

[tomaka]

I'm going to try tackling this by writing a very clean implementation and lots of unit tests.

[nwin]

That would be great!

If you’re interested, I ported the third decoder to the latest nightly: https://gist.github.com/nwin/d874ae14f1c5804b93c6 this one is most promising as it allows to stream data.

[eddyb]

@nwin Whoa, thanks, I didn't expect that code to live on.
I'm tempted to look into the failures, I guess I'll have to slice out the compressed IDAT chunks out of the PNGs with a hex editor, so I don't need a full decoder.

[nwin]

@eddyb don’t be surprised, that particular version takes one minute to compile, due to a regression in rustc

[eddyb]

@nwin hehe it's not a regression, it always took very long to compile. That's what you get for abusing macros to get zlib's performance.
EDIT: If you're talking about the operator type-checking regression, yes, that could make it slower than it was because the macros expand to a lot of binops IIRC.

[nwin]

@eddyb It is unfortunately. :( rust-lang/rust#25069 The version posted there compiles a bit faster because I added some more type annotation which I didn’t add here to be sure I didn’t make a mistake.

[tomaka]

I started https://github.com/tomaka/flate3

Since I'm not familiar with the PNG format, could someone extract the zlib data from #300 and #376 so that I can test it? (hopefully it's not too hard to do)

[eddyb]

@tomaka This is my test for #300:

#![feature(box_patterns, core)]

extern crate flate2;
mod inflate;

use std::fs;
use std::io::prelude::*;

use flate2::FlateReadExt;

fn main() {
    let idat_offset = 0x7a;
    let idat_size = 0x3f64;

    let mut data = vec![];
    fs::File::open("test1.png").unwrap().read_to_end(&mut data).unwrap();
    let data = &data[idat_offset..][..idat_size];

    let mut flate2 = vec![];
    if let Err(e) = data.zlib_decode().read_to_end(&mut flate2) {
        println!("flate2 failed: {}", e);
    }

    let mut inflate = vec![];
    {
        let mut inflate_stream = inflate::InflateStream::from_zlib();
        let mut data = data;
        while !data.is_empty() {
            let (used, out) = inflate_stream.update(data).unwrap();
            data = &data[used..];
            inflate.extend(out.iter().cloned());
        }
    }

    println!("{} vs {}", flate2.len(), inflate.len());

    for i in 0..flate2.len() {
        if flate2[i] != inflate[i] {
            panic!("found mismatch at offset 0x{:x}: {:02x} vs {:02x}", i, flate2[i], inflate[i]);
        }
    }
}

idat_offset is the position of the first byte after IDAT and idat_size is the big-endian value of the 4 bytes before IDAT (in this case, 00 00 3f 64).

You should be able to get the respective offset/size pair from any PNG file with just a hex editor.

[nwin]

You can use the example "pngcheck" of https://github.com/nwin/png to get these offsets (you’ll have to add 4 to it). Just call it with pngcheck -v file.png Please note that the data may span over multiple IDAT chunks…

[nwin]

@eddyb made a tremendous effort to fix the remaining bugs in his inflate implementation. Impressive work!

Chances are good that there are no mayor bug left as it is able to decode the whole pngsuite properly. I’ll probably do some fuzzing on it before merging it into image.

[bvssvni]

What's the status?

[jonas-schievink]

Any progress? How's @eddyb's implementation looking, @nwin? I couldn't find an updated version anywhere.

[nwin]

@jonas-schievink to my knowledge there is no update on this.

[oyvindln]

Someone also made a full rust port of miniz.c, it seems inactive now, but maybe it could be a starting point.

[jonas-schievink]

@oyvindln I took a look at it (since I just stated doing the exact same thing - porting miniz.c to Rust), and it uses (x86-only?) inline assembly to implement the decoding - miniz.c uses coroutines implemented with C macros and some switch-construction akin to Duff's device (this pattern is probably well-known among C programmers), so that looks like it'd be a non-starter.

The bad thing is that tinfl_decompress is a very C-specific monstrosity and you can't just 1-to-1 port it to Rust (also, there's no mature, portable coroutine implementation that runs on stable Rust), so inline assembly actually looks like the preferable option :(

(I begin to feel like every single deflate implementation in Rust is cursed - there are so many, yet not a single one that satisfies many use cases or is bug free)

[oyvindln]

Indeed, the linked port is not very idiomatic rust. Maybe starting a rust implementation from scratch and focusing on safety and correctness first before venturing into low-level optimisation is the better approach. In any case I'm interested in working on this issue, but I'm not quite sure where to start seeing as there are a number of inactive half-finished attempts at implementing zlib in rust.

[eddyb]

@nwin Not sure what you mean, but I believe the updated inflate code lives at https://github.com/PistonDevelopers/inflate.

[jonas-schievink]

Heh, so apparently inflate is already used by image-png (and thus image) for PNG decoding, there's just no way to turn off PNG encoding so flate2 is always included as well... It should be quite easy to get the PNG decoding part flate2-free by adding and propagating the right Cargo features. That would already help me since I don't strictly need encoding.

[nwin]

@eddyb this discussion was about de- and encoding and I was assuming that @jonas-schievink was asking if there was any update to the encoding part.

[kladd]

Is this the reason for the dependency on GCC or was that bug introduced some other way?

[jonas-schievink]

I think it is

Kyle Ladd notifications@github.com schrieb am Sa., 18. Juni 2016, 16:45:

Is this the reason for the dependency on GCC or was that bug introduced
some other way?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#419 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABtCRjl4NcIB4Dhb0UNXFI0Y95yaSH09ks5qNASVgaJpZM4Eq3OJ
.

[oyvindln]

So in case there is still interest in this, I've now got a pretty functional deflate encoder written in rust up and running here: https://crates.io/crates/deflate

Then there is also https://crates.io/crates/zopfli if you want a very high level of compression not achievable using the normal deflate encoders.

[bvssvni]

@oyvindln That's great!

[martinlindhe]

Current status:

• png support has been extracted to a separate crate "png" in Use external PNG crate. #418
• png crate uses the deflate crate by @oyvindln

I think we can close this issue ?