ImageOutputFormat should be used everywhere or nowhere

[fintelia]

Right now, the ImageOutputFormat enum is only used by DynamicImage::write_to, whereas everywhere else that we encode image files takes an instance of the ImageFormat enum. This distinction has led to confusion (see #1461) and is an API inconsistency. I see two possible ways to resolve this:

• Use ImageOutputFormat everywhere. This was attempted by @johannesvollmer who provided a possible patch in Require Seekable Writer #1477 (comment). May also want to implement An improved ImageOutputFormat design #1380 at the same time to avoid more API breakage in the future.

  • Pros: Retains and expands the opportunity for per-format encoding options
  • Cons: More API breakage, more complex API

• Change DynamicImage::write_to to take a ImageFormat, and remove ImageOutputFormat entirely.

  • Pros: decreases API complexity
  • Cons: can't specify JPEG quality without working with a JpegEncoder directly.

[johannesvollmer]

In the first case (Pro): Also, API usability is improved by the fact that a user cannot accidentally try to write an image with an unsupported ImageFormat. Explicitly declaring an ImageOutputFormat catches the error that a certain image format can not (yet) be written, ahead of runtime.

In the second case (Not Cons): Given you want your image to be decoded with a jpeg quality of 80, you already know you need a Jpeg Encoder, so it would be okay to have to work with an image encoder directly, right?

[fintelia]

Yeah, working with a Jpeg Encoder shouldn't be too much of a big deal. Just: JpegEncoder::new_with_quality(writer, 80).encode_image(img)?

Your point about not accidentally trying to write an image with an unsupported format is a good one. Makes me torn between that and the opportunity to reduce the API surface.

[johannesvollmer]

Just hypothetically, if one were to keep ImageOutputFormat and then rename ImageFormat to ImageInputFormat, would this be an obvious change? Or does the ImageFormat currently have more responsibilities than a hypothetical ImageInputFormat?

[fintelia]

Off hand I don't recall anything else that ImageFormat is used for, but a quick grep indicates nearly 350 references to the type so I don't want to say anything for sure. One notable difference between ImageFormat and ImageOutputFormat is that when working with the latter, you must specify encoding options. I think some users produce an ImageOutputFormat via ImageFormat::into() to get the defaults options instead.

[johannesvollmer]

That might be true. If we are going for breaking changes anyways, we could point users to ImageOutput::default(), so that would not be a problem. OutputFormat::from(InputFormat) would also make sense to me.

[ror6ax]

Hi, confused client here. I want to write out configurable compression JPEGs and PNGs in my program. Currently, I use save method on imagebuffer that does not take any arguments(see #1461 for context). Could you please advise what is the simplest way to go about it?

[fintelia]

@ror6ax the API isn't great right now, but given a DynamicImage something like this should work:

let mut encoded = Vec::new();
image::codecs::jpeg::JpegEncoder::new_with_quality(&mut encoded, 95)
    .encode(img.as_bytes(), img.width(), img.height(), img.color()).unwrap();
std::fs::write("img.jpeg", encoded).unwrap();

[ror6ax]

@fintelia I'm getting the trait std::io::Write is not implemented for ImageBuffer<Rgba<u8>, Vec<u8>> error. I can't figure out what is supposed to be used with what on nth attempt. Probably biting off more than I can chew with this one.

[fintelia]

With an ImageBuffer, the encode call should look something like:

.encode(&*img, img.width(), img.height(), ColorType::Rgba8)

[ror6ax]

Still getting method not found in `ImageBuffer<_, Vec<_>> for encode.

    let mut newimg = image::ImageBuffer::new(new_wi, max_hi);
    newimg.encode(&*newimg, newimg.width(), newimg.height(), ColorType::Rgba8);

[fintelia]

That's because encode is a method on JpegEncoder.

Here's a playground link with the full code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c84168946cbb374e745e84bdda08c10f

[ror6ax]

Thanks, got it working now.