image-rs / jpeg-decoder

JPEG decoder written in Rust
Apache License 2.0
150 stars 87 forks source link

JPEG image decoded incorrectly #249

Closed Shnatsel closed 1 year ago

Shnatsel commented 2 years ago

This happens in image v0.24.2

Expected

cbsimbanalaor4

Actual behaviour

image

Reproduction steps

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    use image::io::Reader as ImageReader;
    let input = std::env::args().nth(1).unwrap();
    let output = std::env::args().nth(2).unwrap();

    let img = ImageReader::open(input)?.decode()?;
    img.save(output)?;
    Ok(())
}

invoked with image-convert in.jpg out.png

Shnatsel commented 2 years ago

Also I think this is the only JPEG mismatch across multiple corpora of ~65,000 JPEGs total, which is incredibly impressive.

quilan1 commented 2 years ago

I might be wildly out there, because it's not my typical wheelhouse, but it seems like the following is occurring:

The application segment has a color transform value of AdobeColorTransform::Unknown (0) which, inside choose_color_convert_func, we see:

match component_count {
    3 => {
        // http://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/JPEG.html#Adobe
        // Unknown means the data is RGB, so we don't need to perform any color conversion on it.
        if color_transform == Some(AdobeColorTransform::Unknown) {
            Ok(color_convert_line_rgb)
        } else {
            Ok(color_convert_line_ycbcr)
        }
    }

I think, and I might be getting mixed up about this, that the data's actually in YCbCr, not RGB. In this case, a maxed out pixel YCbCr (1.0, 0.5, 0.5), or (0xFF, 0x80, 0x80) is interpreted as RGB 0xFF8080 -- or the magenta color of the background.

If you remove the segment @ file offset 0x190, the image decodes fine and if you remove the if statement in the code above and just replace it with Ok(color_convert_line_ycbcr), it works fine, but I don't know what the correct approach is to do. I'd love to see other files with the Unknown color transform, to get a better idea of the data out there.

AlanRace commented 1 year ago

@Shnatsel This should now be resolved in the master branch - can you check that this hasn't had any unintended consequences for your large set of JPEGs?

Shnatsel commented 1 year ago

I'll see if I can run a before/after test.

Shnatsel commented 1 year ago

Yep, no regressions as far as I can tell :+1:

I'll run some additional tests and file bugs if I find any issues.