AldaronLau / png_pong

A pure Rust PNG image decoder and encoder based on lodepng.
https://docs.rs/png_pong
Apache License 2.0
26 stars 3 forks source link

Benchmark for libpng-sys seems to be wrong. #7

Closed FrankenApps closed 3 years ago

FrankenApps commented 3 years ago

Hi, I was looking for a fast way to encode .pngs in Rust, so I stumbled over your performance comparison. Looking at the table it seems like libpng-sys just totally rocks for sRGBA images. For example sRGBA 4096x4096 took 0.039266 ms, which if true would be totally insane.

So I tried it. Turns out it is as fast as measured, but only because it does exactly nothing. Here is my test script, heavily inspired by your benchmark:

#[macro_use] extern crate const_cstr;

fn main() {
    let data = std::fs::read("noise.png").expect("Failed to open PNG");
    let data = std::io::Cursor::new(data);
    let decoder = png_pong::Decoder::new(data).expect("Not PNG").into_steps();
    let step = decoder
        .last()
        .expect("No frames in PNG")
        .expect("PNG parsing error");

    let raster = match step.raster {
        png_pong::PngRaster::Rgba8(ok) => ok,
        _ => unreachable!(),
    };

    let before = std::time::Instant::now();

    // 1. Declare png_image struct, 2. Set members to describe image
    let mut png_image = libpng_sys::ffi::png_image {
        opaque: std::ptr::null_mut(),
        version: 0,
        width: raster.width(),
        height: raster.height(),
        format: libpng_sys::ffi::PNG_FORMAT_RGBA as u32,
        flags: 0,
        colormap_entries: 0,
        warning_or_error: 0,
        message: [0; 64],
    };
    // 3. Call png_image_write...
    /* let memory: *mut std::ffi::c_void = std::ptr::null_mut();
    let mut memory_bytes = 0;
    unsafe{
        let _r = libpng_sys::ffi::png_image_write_to_memory(
            &mut png_image,
            memory,
            &mut memory_bytes,
            0,
            raster.as_u8_slice().as_ptr().cast(),
            raster.width() as i32,
            std::ptr::null(),
        );
    } */

    unsafe {
        let path = const_cstr!("output.png").as_cstr().as_ptr();
        libpng_sys::ffi::png_image_write_to_file(
            &mut png_image,
            path, 
            0, 
            raster.as_u8_slice().as_ptr().cast(),
            raster.width() as i32,
            std::ptr::null(),
        );
    }

    println!("Encoding took {:#?}.", before.elapsed());
}

I had to use const-cstr in my Cargo.toml:

[package]
name = "libpng_exporter"
version = "0.1.0"
authors = ["FrankenApps <de.frankenapps@gmail.com>"]
edition = "2018"

[dependencies]
libpng-sys = "1.1.8"
png_pong = "0.8.0"
const-cstr = "0.3.0"

As you can see by running the code, no file was ever created, which I suspect is also true for encoding in memory. I have not tested, if sRGB 4096x4096 works, but at 3.7263 ms that would still be insanely fast for .png encoding (I tried quite a bunch of stuff already ;-) ).

So it seems to me, that if have not missed anything libpng-sys should better be removed from the comparison, because it does not seem to work, or needs a fix to run properly...

AldaronLau commented 3 years ago

Yeah, it's probably wrong - I am skeptical of the benchmark I made for libpng-sys, too. I'd rather fix it than remove it, though. If you have any ideas PRs are welcome, just don't include any I/O in the benchmark.