OpenImageIO icon indicating copy to clipboard operation
OpenImageIO copied to clipboard

[BUG] Writing ImageIO Plugins section does not describe how to register a plug-in at run time

Open MarkCallow opened this issue 2 years ago • 6 comments

Describe the bug The Writing ImageIO Plugins section of the documentation does not describe how to register the plug-in at run time.

To Reproduce Steps to reproduce the behavior:

  1. Read the documentation section and observe that all it states is you have to write a subclass of ImageInput or ImageOutput and documents various methods you must implement.

Expected behavior Given all the talk about being able to add plugins without modifying OpenImageIO I expected this section to describe exactly and completely how to do so.

Having spent more time browsing the documentation I found the function OIIO::declare_imageio_format which takes ImageInput::Creator and ImageOutput::Creator pointers. What the Creators should point to is not clear. One can surmise that they are the name_{input,output}_imageio_create methods mentioned in ˆWriting ImageIO Plugins`.

Another question whose answer would be welcome is what happens if the format name of plugin you declare has the same name as an existing plug-in. Will it replace the existing one?

Evidence n/a

Platform information: n/a

MarkCallow avatar Sep 17 '21 06:09 MarkCallow

Another question whose answer would be welcome is what happens if the format name of plugin you declare has the same name as an existing plug-in. Will it replace the existing one?

I've found the answer to this. OIIO::declare_imageio_format will only add the new plugin if one does not already exist for the extension and format name. This is unfortunate. I can think of several uses for overriding an existing plug-in. For example fixing a bug in an existing plug-in without having to be able to build all of OIIO or, more importantly, being able to ship a fixed plug-in with one's app until such time as the various package managers catch up with an official version containing the fix.

MarkCallow avatar Sep 22 '21 03:09 MarkCallow

Curiously, this is still on the books: https://github.com/OpenImageIO/oiio/pull/621

I proposed this many years ago, and the response was just crickets chirping. So I never merged because I thought somebody would have spoken up if this solved any real problems. But I can certainly revive it.

lgritz avatar Sep 22 '21 03:09 lgritz

Merging #621 would be awesome. I want to create a fix for the png reader for the cHRM chunk I mentioned in another question. It is likely to be a while before my app is ready to use OIIO. If that PR was merged now, perhaps it might be in the versions supported by the various package managers (Linux apt and vcpkg are the most interesting for me) by the time I'm ready to release my app and have the fix developed.

MarkCallow avatar Sep 22 '21 13:09 MarkCallow

OK, I'll revive 621. The only concern I have is that this could be a security hole -- the ability to substitute arbitrary code at runtime is already a little squicky, but at least currently it can't affect how the built-in formats are read. Extending that to be able to runtime substitute code for a built-in format opens wider opportunity for mischief. I may need to think about how to safeguard this effectively.

It would be great to just fix the OIIO png reader as you want, instead of having to do it on your end.

lgritz avatar Sep 22 '21 15:09 lgritz

As for your original question on this issue: Yes, sorry, originally OIIO merely had the ability to look for and load DLL/so plugins for format readers when encountering an unknown format, and the documentation was entirely oriented to that. The ability for applications to declare and register (in their own already-loaded code) the implementation of a format reader was added much later, and apparently we neglected to update the docs to spell out how to do this. I'll make sure to get it added, sorry for the omission.

lgritz avatar Sep 22 '21 19:09 lgritz

It would be great to just fix the OIIO png reader as you want, instead of having to do it on your end.

It would but if I am going to provide a PR I need to be able to test it and I have been having troubles building OIIO.

MarkCallow avatar Sep 23 '21 05:09 MarkCallow