It would be great to have a way to check if a file is supported without having to read the file.
My specific use case for this functionality is:
I'm filtering the results of a glob search, and only want to retain paths to files that can be read by this crate (including formats supported in the future, so not hardcoding the list of image formats). Without having to read each one, since they are gigapixel images.
Several ways of offering this functionality come to mind:
I see that you are preventing exhaustiveness checking on the enum, so you can't derive strum::EnumIter but it could be a manual iter method on the enum. This wouldn't go against the reasons who preventing exhaustiveness checking, since it's a legitimate desire to be able to iterate all supported formats.
Note: For being able to implement my filter correctly, I'd also need a way to filter the image format iterator by whether that format is supported for reading. So there could be a method ImageFormat::can_read() -> bool.
Both parts seems reasonably small and useful. I can offer guidance on implementation if necessary.
I see that you are preventing exhaustiveness checking on the enum, so you can't derive strum::EnumIter but it could be a manual iter method on the enum. This wouldn't go against the reasons who preventing exhaustiveness checking, since it's a legitimate desire to be able to iterate all supported formats.
Exactly. A downstream crate shouldn't depend on the exact set of formats that are known. The simplest way would be to provide an opaque struct that implements Iterator<Item=ImageFormat> and _internally_ iterates over a slice of all formats. A slight alteration of the concept would be an associated method ImageFormat::iter() -> impl Iterator<Item=Self>. It wouldn't be as easy to store that in a struct but would that be required?
Yeah, I think the struct that impls Iterator is the way to go. I think there's an unwritten rule that crates should only use -> impl for private functions, at least as long as existential types are not on stable. After that, it'd be easy to also store these in a struct, if the crate defines a pub type FooIter = impl Iterator<Item = Foo>; and the function returns -> FooIter.
One caution I have is that the just because a variant is present in ImageFormat does not mean that we guarantee support decoding that format. For example, right now we have ImageFormat::Avif because there is an _encoder_ for the format (behind a feature flag no less...)
@fintelia That's why I also suggested adding the method ImageFormat::can_read() -> bool (and the analogous ImageFormat::can_write() -> bool).
I would like to help out with this.
One thing that is confusing is that the terms read/write and decode/encode are used interchangeably, from what I know I think what we want here is can_decode & can_decode as that's how it's referred to in the crate.
So what I propose this to become is:
['img.png', 'img2.mp4'].map(|path| ImageFormat::can_decode(path)) // [true, false]
So what I propose this to become is:
['img.png', 'img2.mp4'].map(|path| ImageFormat::can_decode(path)) // [true, false]
I would interpret that interface as meaning those specific PNG and MP4 files could/couldn't be decoded. Whereas, I think the prior discussion was just to query whether PNG and/or MP4 files in general could be decoded by the crate.
There already is a function for mapping paths to formats, ImageFormat::{from_path,from_extension]. Thus having methods on ImageFormat should be clear and ergonomic enough.. It's a relatively small change:
["img.png", "img2.mp4"].map(|path| ImageFormat::from_path(path).map_or(false, ImageFormat::can_read))
As for arguing terminology, I don't think we're 100% consistent to say for sure鹿 :sweat_smile: But: To my understanding, decode is used as sucessfully loading the entire image matrix. That suggest more than a rough understanding of the format itself and requires data. For example, we might not support all color types鈥攅specially in tiff and other flexible container formats. But we can read those images to parse the header and read the size etc. For formats where !format.can_read() the result of all those operations would _already_ be an error. Effectively, I would expect a can_decode method to already parse data and more or less execute the deocding without actually producing pixel data鈥攚hich might be more optimized and require less memory. (Btw: interesting idea for a PR to any of the base crates). Thus, can_read seems more consistent.
鹿The names are open, load, decode, Reader, ImageDecoder. There is generally some logic behind those, for example to distinguish methods taking paths from methods taking io::{Read,Write} implementors and from fs-types, but maybe that only contributes to complexity. In hindsight, I'm no longer too sure about io::Reader either. Decoder might be the more consistent name as even though it _takes_ a reader it is not itself a reader, but then again it's not a ImageDecoder either.)
That makes a lot of sense (on the terminology) so I will implement it as sugested:
["img.png", "img2.mp4"].map(|path| ImageFormat::from_path(path).map_or(false, ImageFormat::can_read))
https://github.com/image-rs/image/pull/1388 Was merged which adds the functionality requested by @Boscop Once he can check that this was enough for him we should close this issue.
Most helpful comment
That makes a lot of sense (on the terminology) so I will implement it as sugested: