Implement copy_image for OpenEXR

[tkchanat]

trafficstars

Description

Base on the OIIO slack channel discussion, this PR implements the copy_image function for the OpenEXR format. The main motivation of this PR was due to a recompression issue where copying a lossy compressed EXR image (pixels are decoded and then encoded again), which can cause the image quality to downgrade on each iteration.

What's Changed?

• OpenEXROutput is now a friend class of OpenEXRInput. Since OpenEXROutput::copy_image() needs to access some of the private members in OpenEXRInput (This approach is referencing the implementation in JpegInput and JpegOutput).
• The OpenEXRInputStream and OpenEXRInput class have been moved from exrinput.cpp to exr_pvt.h. As OpenEXRInput depends on OpenEXRInputStream, so both have to be moved together. Such relocation is necessary due to the above reason, i.e. OpenEXROutput::copy_image() needs to access some of the private members in OpenEXRInput.
• And the implementation itself, OpenEXROutput::copy_image() overrides the base class method just like JpegOutput does.

How it Works

The OpenEXROutput::copy_image() implementation took the advantages of the existing OpenEXR function Imf::OutputFile::copyPixels, quoting the comment from the function:

//-------------------------------------------------------------- // Shortcut to copy all pixels from an InputFile into this file, // without uncompressing and then recompressing the pixel data. // This file's header must be compatible with the InputFile's // header: The two header's "dataWindow", "compression", // "lineOrder" and "channels" attributes must be the same. //-------------------------------------------------------------- IMF_EXPORT void copyPixels (InputFile &in);

When the output image is copied from an input image, the pixel data can only be located in one of the input parts. So it checks if both input and output part is initialized (non-null), then invoke the copyPixels function in place.

All the error handling is done within the OpenEXR copyPixels call. There is a try-catch block in OIIO side that emits an error message and fallback to the default image copy routine, e.g. ImageOutput::copy_image.

Tests

Checklist:

• [x] I have read the contribution guidelines.
• [ ] I have updated the documentation, if applicable.
• [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• [ ] If I added or modified a C++ API call, I have also amended the corresponding Python bindings (and if altering ImageBufAlgo functions, also exposed the new functionality as oiiotool options).
• [x] My code follows the prevailing code style of this project. If I haven't already run clang-format before submitting, I definitely will look at the CI test that runs clang-format and fix anything that it highlights as being nonconforming.

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: tkchanat / name: Andy Chan (1a296d73eb64e9796178511bf1a7d8fe24cee759, 5c205ab8432cf2e156f4bdbedfe1d73571850569, ce740c318dc576f763ea9cc18b003e6fd7c75290, ce5ac37ba4c93ef2b2a209867675afe4704c2860, 552ae53d5427f15c0e7467af1931c1f2fb013451)

[lgritz]

I think it would be a great addition to have copy_image working efficiently for OpenEXR.

I think you are mostly on the right track, but I fear that it may have escaped your notice that there are two different implementations of the exr reader: the one in exrinput.cpp, but also a second one in exrinput_c.cpp. The latter is based on the newer "OpenEXR Core" library, and is only used (and is the default) when OIIO builds against OpenEXR >= 3.1. We need to maintain both for a while, but once our minimum OpenEXR dependency is at least 3.1 -- which may be the case for next year's OIIO 2.6 -- then we'll remove the older exrinput.cpp. Or maybe the other way around? OpenEXR itself may reimplement its C++ API in terms of the C API of the Core library, which would make it just as efficient, in which case maybe the surviving one on our side will go back to the C++ based exrinput.cpp?

[tkchanat]

Hi @lgritz, thank you for reviewing this. The implementation for OpenEXRCoreInput is more challenging than I expected, mostly due to my limited knowledge to the core library codebase, and also the lack of exposed functions in the OpenEXR library to manipulate raw data. That being said, I won't recommend implementing the copy_image function for OpenEXRCoreInput in OIIO but instead it should be handled on the OpenEXR side (by overloading copyPixels). This way the current OpenEXR API is forward compatible with the core library, and it has access to the necessary encapsulated members to perform the copy. If you do agree with the above, I can revert some of my changes to only handle raw copy for OpenEXRInput, and leaving OpenEXRCoreInput to use the default copy.

[tkchanat]

Another thing to note was that the copyPixels implementation for tiled images copies all mip levels at once. This means if the user were to call copy_image on each mip level, it's going to throw an exception about output image already contains pixel data. So I made it fallback to default copy if the input is a tiled image and it contains mip level.