image
image copied to clipboard
ImageOutputFormat should be used everywhere or nowhere
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 https://github.com/image-rs/image/pull/1477#issuecomment-844069443. May also want to implement #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.
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?
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.
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?
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.
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.
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?
@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();
@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.
With an ImageBuffer, the encode call should look something like:
.encode(&*img, img.width(), img.height(), ColorType::Rgba8)
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);
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
Thanks, got it working now.