Add API to set ImageReader format by extension to support hooks#2662
Add API to set ImageReader format by extension to support hooks#2662RunDevelopment wants to merge 1 commit intoimage-rs:mainfrom
ImageReader format by extension to support hooks#2662Conversation
| /// Supply the extension of the format as which to interpret the read image. | ||
| /// | ||
| /// The extension must be without the leading dot. E.g. `"png"` or `"jpeg"`. | ||
| pub fn set_format_with_extension(&mut self, ext: OsString) { |
There was a problem hiding this comment.
I think those names will be confusing. We have ImageFormat::from_extension but a very important point of this method is that it will not use a core decoder but a registered hook. My gut feeling is set_decoder_from_extension. And would you add a comment about the resolution order?
The decoder is chosen first checking the list of registered hooks for the extension, see [
hooks::decoding_hook_registered]. If no custom hook is found it will consider the builtin format which corresponds to the extension as if by [ImageFormat::from_extension].
Or something of sorts. For consistency with either decoding_hook_registered or ImageFormat::from_extension the argument should be &OsStr or impl AsRef<OsStr> respectively. Though I think we've been trying to reduce the amount of generic interfaces and the upside is small. The allocation to an owned variant doesn't really matter, we might change the internals to only store an index into the hook list or something.
There was a problem hiding this comment.
Should we switch things so that ImageReader always tries the hooks even if you specify a built-in format by calling with_format or set_format? We'd have to pick one specific file extension to use if you specified JPEG, TIFF, and PNM but I'm not sure that would actually be a problem. Hopefully no one would register different decoders for .jpg and .jpeg and then be upset at the result...
|
Another option is to use
Not entirely sure how I feel about that though, since it would add a bit more churn |
3 participants
Right now, plugin decoders can only be used when decoding a file. It's impossible to use them to decode arbitrary
BufRead+Seekdata. This is because hooks are currently only used byImageReaderand the only way to set its internalformatto an extension is viaImageReader::open(&Path).This PR solves this API limitation by adding a new
set_format_with_extensionmethod, which sets the internalformatto the given extension string.Bike shedding: I'm not sure about the name. Other options I've considered:
set_format_from_extension,set_format_by_extension,set_extension.Related to #2616