Some images get corrupted during decoding

[mwap]

Some images like this one: https://ant-photo.eu/wp-content/uploads/2020/07/Cataglyphis-iberica-worker2.jpg end up looking like this: https://i.ibb.co/tp2d6Wy/out.png after simply decoding the image and saving the output in a human readable format.

Example program:

[dependencies]
bmp = "0.5.0"
jpeg-decoder = "0.1"

extern crate jpeg_decoder as jpeg;
#[macro_use]
extern crate bmp;

use std::fs::File;
use std::io;
use bmp::*;

fn main() {
    let file = File::open("broken.jpg").unwrap();
    let mut dec = jpeg::Decoder::new(io::BufReader::new(file));
    let pixels = dec.decode().unwrap();
    let mut img = Image::new(1920, 1080);
    for s in 0..1920 * 1080 {
        img.set_pixel(s % 1920, s / 1920, px!(pixels[s as usize * 3], pixels[s as usize * 3 + 1], pixels[s as usize * 3 + 2]));
    }
    img.save("out.bmp");
}

This happens to some of the other images at https://ant-photo.eu/galeria/zdjecia-mrowek

[mwap]

Worth noting that every other software I have used to process these images with has done so without any problems

[kaj]

I get the same (broken) results with the decode example in this crate. The original image looks good in eog (gnome image viewer) and firefox. So yes, this seems to be a bug in jpeg-decoder.

According to the linux file utility, the image trigging the bug is:

broken.jpg: JPEG image data, JFIF standard 1.01, resolution (DPI), density 72x72, segment length 16, progressive, precision 8, 1920x1080, components 3

[quilan1]

It looks like the error is first occurring on the second segment of the image (Y component, spectral components 1..6).

The data in the worker.jpg file there (offset 0x26E6) is: FE E1 FF D0 FE E1. The first two bytes corresponds to the huffman encoding for 0x70 (EOB for the next 7-bits' worth of MCUs, with an invisible tacked on '1' at the beginning) or: 11111110 (huffman code for 0x70) [1]1110000 (240 MCUs long) 1 (unused bit). So, the next 240 MCUs should be skipped. This is also the restart interval length (DRI), which corresponds to the row size in blocks (1920 / 8).

Note: Since I just picked up the JPEG standard yesterday, I'm not 100% sure on the following, so the following comes with a grain of salt.

So I think the intended behavior is (inside Decode::decode_scan) to read this eob_run 240, get to the end of the row & it should have exhausted the mcus_left_until_restart. It would then read the FF D0, process the restart and continue with the next row's FE E1 (another EOB 240).

However, what's actually happening is some weirdness with the horizontal / vertical sampling (2x2 here) and how it interacts with the MCU calculations. Once it's finished 240 blocks, it thinks it's only completed 60 MCUs, instead of all 240, so it instead continues processing with the unused '1' bit and keeps reading more bits and gets garbage from the bitstream (as it walks over the restart marker).

As I said, I just picked up the standard & this codebase yesterday, so I'm not sure the correct change, but this is the symptom as far as I see it.

[quilan1]

Quick update -- decided to isolate my changes to just the decode_scan function. This meant that things are still sent in batched form over to the worker threads, instead of single lines. This causes a little bit of wonkiness in the way the timing is done for sending buffers over to the worker. Perhaps a follow-up story can be done to address cleaning up that code, by allowing individual rows to be sent to the worker, instead of batches.

But, everything works for OP's image, and it passes the test suite. I've some code logic to clean up, a test case to be made, and perhaps more before I begin the process of asking for a PR. I'll push my current (ugly) code up to my personal fork within the next few days, if anyone would like to test it out -- I'll give a head's up here when it's ready.

This story also related to the venerable #89, so that person certainly called it early on.

[quilan1]

So I'm new to open source contribution -- how does one let the maintainers know that something's ready for review? Send up a flare? Light the beacons of Gondor?

[HeroicKatora]

Fixed in #176, the original example also does not reproduce.