Ergonomics for external image formats

[fintelia]

At the moment, it is pretty difficult to work with formats that aren't implemented directly in this crate. Other crates largely haven't chosen to implement our ImageDecoder trait, and even if they did, there isn't a way to use them when working with ImageReader or our file format detection logic. This has created a lot of pressure for us to expand the list of supported formats.

The question is whether we could add some form of hooks support so that downstream users could inject their own format decoders to use in addition to the built-in ones. It is a bit messy, but perhaps something like this could work:

trait FormatHook<R> {
    fn file_extensions(&self) -> &[&str];
    fn magic_bytes_match(&self, file_prefix: &[u8]) -> bool;
    fn make_decoder(&self, reader: R) -> Box<dyn ImageDecoder>;
}

impl ImageReader<R> {
    fn with_hooks(&mut self, hooks: &[Box<dyn FormatHook<R>>]) {}
    fn open_with_hooks(path: &Path, hooks: &[Box<dyn FormatHook<R>>]) -> ImageResult<Self> {}
}

[kornelski]

Maybe the hooks could even have a global registry. Otherwise you still need to coordinate with all the deps using the image crate on how to collect and pass down the hooks. If there's a global registry, you can just register a format in your main(), and then have the format behave as if it was built-in.

[kornelski]

magic_bytes_match could cause problems if some implementations weren't good at sniffing them. Decoding the first format that matches would probably make sense, but could require users to carefully pick the order in which the formats are registered.

What if the image crate was still in charge of image format sniffing, and maintained a central list of formats? (formas that can have a custom codec hooked up). Contributing and maintaining magic bytes sniffing is less of a burden than maintaining full codecs, so for sniffing image crate could be maximally inclusive, and there's also a bit of value in reporting errors not as "unknown format" but "we don't support foo format".

[fintelia]

Including all the format selection logic into image might be a good option. We'd need some inclusion criteria, but something like only accepting formats supported by ImageMagick would probably be sufficient.

I'm not too worried about file extensions, but if we went this direction we'd need to improve our magic byte format sniffing. For instance, at the moment we assume that any file that starts with RIFF is WebP, even though there's a bunch of other file formats that also use RIFF containers...

Having a global format registry could work, though it would require some trickery to handle the generics. Our built-in codecs are monomorphized for each reader type. But for external formats the registry would have to deal with only a single kind of reader. We'd probably have to operate on BufReader<Box<dyn Read>> or perhaps a custom wrapper type so we could implement both BufRead and Seek on it.

[HeroicKatora]

A custom wrapper sounds fine, it just introduces one additional requirement to me: We should be able to change the interface for hooks to update those bounds, in a compatible release. There's two reasons, first I don't want a repeat of when we added the Seek bound, and second I'd like to speculate that the hooks could actually solve the problem of some crates being able to support Read without Seek if we had a proper way of dispatching. Currently everything goes to open and has the same bound because, I think, the bounds propagate quite far into our implementation with the original type parameter where the dispatch is burried. If we had hooks earlier then it should be also much easier to support different bounds in our internals—all of that should be type-erased away or at least able to be (¹ except lifetimes. We don't require 'static at the moment. That may be tricky but not impossible).

---

Note: https://docs.rs/flexible-io/latest/flexible_io/

I've been churning on this a bit low-key in the background, since the issue of optional traits and combining different io traits has been .. contentious. I realize it's unsafe, but motivating with_metadata_of in std. It currently does not give you Box<dyn Read + Seek> but it could. I'll read this issue as a form of use-case feedback :)

[fintelia]

Created #2372 to try out a global hooks design. I'm not quite sure how something like flexible-io could factor into the design in a backwards compatible way, given that user will be able to both pass in arbitrary readers and also provide decoder implementations which may or may not require Seek.