Compress::write_scanlines fixes

[lautat]

This fixes two issues in Compress::write_scanlines. The function panicked when images with a height larger than MAX_MCU_HEIGHT. The issue is fixed by assigning the following batch of rows to the same array starting from index 0, and passing the number of rows in the batch as a parameter to jpeg_write_scanlines.

The other issue emerges when writing progressive JPEG, which requires multiple scans. One way of achieving this is to call write_scanlines in a loop:

while compress.write_scanlines(&image) {
}

This resulted in a warning:

Application transferred too many scanlines

This issue is fixed by returning self.cinfo.next_scanline < self.cinfo.image_height, so the loop finishes when the required amount of scans are completed.

[kornelski]

Thanks for the fixes.

I'm surprised that you've removed arrayvec, because in a previous fix I've had to add it:

https://github.com/ImageOptim/mozjpeg-rust/commit/21c5672d47bba9576fd9a112dce7cddbf136fa7f

[kornelski]

I can't reproduce the problem. I've added a test that checks multiple sizes, and it seems OK.

[lautat]

I can't reproduce the problem. I've added a test that checks multiple sizes, and it seems OK.

With the tests you added, I am able to reproduce the following error:

---- color_jpeg stdout ----
thread 'color_jpeg' panicked at 'called `Result::unwrap()` on an `Err` value: CapacityError: insufficient capacity', libcore/result.rs:1009:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:221
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:476
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: rust_begin_unwind
             at libstd/panicking.rs:325
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::result::unwrap_failed
             at libcore/macros.rs:26
   9: <core::result::Result<T, E>>::unwrap
             at libcore/result.rs:808
  10: <arrayvec::ArrayVec<A>>::push
             at /home/atte/.cargo/registry/src/github.com-1ecc6299db9ec823/arrayvec-0.4.10/src/lib.rs:181
  11: mozjpeg::compress::Compress::write_scanlines
             at src/compress.rs:120
  12: test::color_jpeg
             at tests/test.rs:32
  13: test::color_jpeg::{{closure}}
             at tests/test.rs:22
  14: core::ops::function::FnOnce::call_once
             at libcore/ops/function.rs:238
  15: <F as alloc::boxed::FnBox<A>>::call_box
             at libtest/lib.rs:1468
             at libcore/ops/function.rs:238
             at liballoc/boxed.rs:672
  16: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:102

rustc version is 1.31.1.

[lautat]

I'm surprised that you've removed arrayvec, because in a previous fix I've had to add it:

21c5672

ArrayVec seems to be a wrapper for primitive array with a dynamic length up to the limit. Calling ArrayVec::clear between each chunk of rows would fix the issue as well. To me, the fix looks like arrayvec was added as a dependency only for the length counting, which can be done easily with a few lines of code.

[kornelski]

It's bizarre that it fails for you and not for me. But I think I've understood the bug now — thanks!

I've pushed c292b3e33b4f60cfcccded2624c0c79c85fa52ab