PROTOTYPE WIP: Add thread-safe read functions to input files

[lgritz]

The fact that OpenEXR read methods (readPixels, readTiles, etc.) all require a prior call to setFrameBuffer() prevents truly concurrent reads by multiple threads because the calling application needs to maintain a mutex that keeps any other threads from reading from the same file between the two function calls.

This PR is aimed to implement a new set of API calls, variety of the readPixels et al. that take a const FrameBuffer& as parameter rather than relying on saved state, and thus allows truly concurrent reads by multiple threads -- if they use these new safe versions, obviously the old ones continue to be unsafe to use concurrently.

It's a lot of work to do this right! And I'm not aiming to make it good right now, but only to minimally stake out the API and make it functional (even if inefficient) so that we are essentially "reserving" the API just in time for a 3.2 release, and then we can iterate on improving the implementation underneath in subsequent patches, without changing APIs or breaking ABI compatibility within the 3.2 family.

At the moment, I'm just posting a subset of the work as a prototype so people can see where I'm going with it. I've implemented a simple design (just lock internally) for a couple classes. So I'm seeking feedback on which of the following options are preferred:

1. Flesh this out for ALL the classes and relevant methods ASAP in time for 3.2.

2. Since these are not virtual methods, adding them doesn't actually break the ABIs, so don't rush this, we can add them (all at once, or incrementally) in subsequent 3.2.x patch releases.

3. Just hold off on all of it until 3.3.

[cary-ilm]

Clearing out old PRs, @lgritz OK to close this? Presumably the core API provides the concurrency you're after?

[lgritz]

We can close this prototype PR, but I think it is something we should eventually find a way to do in the C++ API.

Why do we offer a C++ API if you can only get performance from Core? Is it just for legacy reasons and we're going to phase it out?

[peterhillman]

In today's meeting we discussed updating the Slice API to use mdspans.
That might need a new Slice class and therefore a new FrameBuffer class. If and when that happens, we could make those natively threadsafe by a slight tweak that doesn't complicate the user code, perhaps replacing this:

FrameBuffer buf;
buf.insert(Slice(...));
inputFile.setFrameBuffer(buf);
inputFile.readPixels(ymin,ymax);

with this:

FrameBuffer2 buf;
buf.insert(Slice2(...));
buf.setInput(inputFile);
buf.readPixels(ymin,ymax);

(that said I also think we don't need to make the C++ API optimal for every access pattern; there will be cases where using the Core will be advantageous, but for straightforward cases the C++ API can still be optimal)

[lgritz]

Why have separate calls at all? Why not just have one readPixels call that takes all the information about what to read and where to put it, so that nothing about a read requires a stateful multi-call sequence?

[peterhillman]

Often setFrameBuffer is called once, then multiple readPixels calls are made with the same framebuffer but different scanline or tiles. It seems that would be less efficient if the setFrameBuffer logic had to run before every call to readPixels even if the framebuffer is unchanged. Also, calling readPixels does change the internal state of the InputFile object, which requires locks to keep things safe.

My original thought was something more like this:

FrameBuffer buf;
InputFileReader rdr = inputFile.getReader(buf);
rdr.readPixels(ymin,ymax);

A thread-safe convenience function could allow inputFile.readPixels(buf,ymin,ymax) by calling inputFile.getReader(buf).readPixels(ymin,ymax);

The inputFile.getReader() call would be a thread safe const function, so each thread can call that, hold the InputFileReader returned, and make calls to its readPixels method as required. The readPixels call could change the state of the InputFileReader but not the InputFile object