Image retrieval for multi-channel camera sequence acquisition

[marktsuchida]

Current CMMCore API:

void* popNextImage();
void* popNextImageMD(Metadata& md);
void* popNextImageMD(unsigned channel, unsigned slice, Metadata& md);

void* getLastImage(); // const removed in 2012, no commit message
void* getLastImageMD(Metadata& md) const;
void* getLastImageMD(unsigned channel, unsigned slice, Metadata& md) const;
void* getNBeforeLastImageMD(unsigned long n, Metadata& md) const;

• channel must be 0 (nonzero is error as of MMCore: Avoid allocating more than 1 "channel" in sequence buffer #748, 8765b61; before that, nonzero returned garbage image)
• slice must be 0 (nonzero is error since e2a686d (2017))
• All versions will pop or peek with no regard to the source camera or camera channel
• So the versions with channel and slice are effectively the same as the versions without

This means that currently applications need to check the image metadata (md) to determine the camera channel, and sort the images accordingly. And in the case of Live view (get, not pop), getNBeforeLastImageMD() is used to try to find an image from each channel — this works most of the time but is a bit sketchy.

There is also another problem that is not limited to multi-channel: these functions all return a pointer to the frame buffer within the sequence buffer, without providing any lifetime guarantees. We get away with this (at least the vast majority of the time) because the frame buffers are not overwritten until the buffer nearly fills up, which is a failure anyway for MDA. For Live we only look at the newest image so this failure mode is probably even rarer. In all cases, however, we are relying on the fact that MMCoreJ or pymmcore immediately makes a copy of the image.

I think the API we want to aim for might be something like the following:

bool dequeueImage(void* buffer, size_t bufsize, Metadata& md, int channel = 0);
bool peekImage(void* buffer, size_t bufsize, Metadata& md, int channel = 0);

(Possible alternative names: popOldestImage() and peekNewestImage(), but I don't like the common initial p.)

Observations:

• The image is copied by the Core into the caller-provided buffer, whose bufsize must be correct for the image. In MMCoreJ, this should be a caller-provided direct buffer. In pymmcore, this could be a caller-provided NumPy ndarray, or any object implementing the Python buffer protocol
• The return value can be used to indicate whether an image was retrieved (the existing API throws, which is awkward because the buffer being empty is not an error but a regular occurrence)
• To support popping an image for the specified channel, we need separate sequence buffers for each channel. This is doable, especially if the Core already has the information proposed in MMDevice AddTag/RemoveTag should be replaced #840
• There is some overlap here with New image buffer #570, but I'm proposing a much smaller change to the APIs and internals. Multicam-like cameras will continue to work. This is not mutually exclusive with the new buffer management strategy proposed there and may even help with more smoothly incorporating those ideas.
• When wrapped by MMCoreJ and pymmcore, the Java/Python methods don't necessarily need to take a destination buffer; they could just allocate the buffer. Or both options could be offered.

So the main heavy lifting that is needed is to have per-channel sequence buffers.

This just means a per-channel queue plus a stack of unused frame buffers. On InsertImage(), we take a frame buffer off the stack, copy in the image/metadata, and enqueue into the per-channel queue. On dequeueImage(), we dequeue a frame buffer from the requested per-channel queue, copy the image/metadata into caller-provided destinations, and push the frame buffer onto the stack. On peekImage(), we "just" copy the front of the queue (more on lifetime management below).

Note that both copying steps still happen outside of (or without locking) the queue, so there is no performance penalty.

For MDA-like use with dequeueImage(), that's it. But for Live-like use (startContinuousSequenceAcquisition()) with peekImage(), we need to address 2 additional issues:

• When the buffer fills up, current behavior is to empty the (single) sequence buffer and start over. We could do the same, but we could also "steal" the oldest frame buffers (perhaps starting with the same channel and falling back to other channels). (This would have been hard before Automatically clear circular buffer in continuous sequence acquisition #588 and Remove camera "clear on buffer overflow" behavior #717.)
• Such emptying or stealing must not clash with an ongoing copy by peekImage(). It is not enough to say we don't steal the newest frame (although that is probably also a good idea) because the frame being copied may no longer be the newest. Here, I think we need to explicitly model shared ownership of the frame buffer by the queue and the peek-copier. std::shared_ptr with a custom deleter can be used for reference counting; the deleter returns the buffer to the unused-stack.

Compatibility

• The existing retrieval methods should be deprecated, but we need to support them for a while on top of the new per-channel buffers. (We now have the infrastructure to test for compatible behavior)
• For popNextImage*(), dequeue from channels in round-robin fashion, skipping empty queues and failing when all are found empty. For getLastImage*(), peek in round-robin fashion. Both should keep the last dequeued/peeked channel and start from the next on the next call. getNBeforeLastImageMD() should do the same, except that the round-robin check should always start at the given channel n % numChans; I think this will preserve the behavior needed by existing Live views.
  • Okay, there is a lifetime problem here: the new-style dequeue/peek cannot be used to return a void* with no later cleanup. So we need a two-stage migration:
    1. After returning void* pointing to a frame buffer, place the ownership (shared_ptr) of that buffer into a dedicated queue (the "slow-expiring queue") instead of dropping ownership and returning it to the unused-stack. When InsertImage() needs a frame buffer and the stack is empty, it instead dequeues a buffer from the slow-expiring queue. This is yucky, but it should provide behavior very close to what we currently have. If the application uses only the new retrieval methods, the slow-expiring queue is not used.
    2. MMCoreJ and pymmcore can replace their popNextImage*() and get*LastImage*() with versions that call the new methods. This is clean, because both copy the data anyway.

Additional thoughts:

• Should we also add a blocking version or a future-returning version? Probably, but later.
• Should we support the case of the camera channels having different image size/format? Yes, but later.
• What happens if startSequenceAcquisition(cameraLabel, ...) is called on multiple cameras?
  • The ASIdiSPIM plugin might use this (but it's not supported in MM 2.x)
  • I think use is rare enough that we perhaps don't need to maintain perfect compatibility, but it would be good to have an alternative that is easy to migrate to. TODO.
• Doesn't this API make it hard to offer "zero-copy" image transmission to Java/Python in the future? I don't think this matters, because we can always add new methods for that (and almost certainly will need to anyway). One way would be for the client to allocate a collection of frame buffers and "enqueue" them. Dequeuing/peeking returns a reference to a filled buffer. The client re-enqueues the buffer when done with it. In any case, out of scope for this issue.
• Do we even need to keep a full queue of images for Live mode? Can we just keep the newest image (per channel)? Good question, and we probably should do so at some future time. But for now, the need to support the existing retrieval methods (see above) and the Live Replay plugin mean that we shouldn't make this change quite yet.

Prerequisites

• Unit tests for the current behavior
• MMDevice AddTag/RemoveTag should be replaced #840, as mentioned above
• PrepareSequenceAcqusition should be removed #595 will simplify things
• Probably better to clean up the metadata tag additions in CircularBuffer. They should be moved somewhere else so that the buffer is just a buffer and doesn't mess with the contents of the data.

[marktsuchida]

Regarding startSequenceAcquisition(cameraLabel, ...), it is not currently possible to run two cameras simultaneously (correctly, at least) because of a change made in 2023 (#362) — which failed to notice the doc comment saying "The difference between this method and the one with the same name but operating on the "default" camera is that it does not automatically initialize the circular buffer." The buffer is initialized on every call now.

I'd be inclined to keep the current behavior (possibly with a fix to throw on attempts at simultaneous acquisitions), given that no issue has been raised in the 2.5 years since. Also, it's not clear that a method to perform said initialization is currently available (prepareSequenceAcquisition() does not initialize the buffer, and initializeCircularBuffer() does not take a camera label).

This means that no special considerations need to be made for concurrent acquisitions, for now. If such a need arises again (which I'm sure will), we can certainly add support for it, and the changes proposed here will only make it easier to do it right.

[marktsuchida]

it is not currently possible to run two cameras simultaneously (correctly, at least)

Well, not quite. Initializing the buffer with the same image size/format does NOT empty the buffer, so concurrent acquisitions are still possible.

One possible way to keep support for this is to

• Make it a rule that the cameraLabel sequence acquisitions cannot be mixed with the default-camera one
• Change startSequenceAcquisition(cameraLabel, ...) to return int, which we call M (currently the return type is void)
• Frames from this acquisition are placed in the queue for channel M + N, where N is the camera channel index

I guess we could also support mixing with default-camera acquisitions if we use large or negative values of M. This scheme won't support existing code to work unmodified, but migration should be easy.

On the other hand, it's hacky. It might be better to just offer a proper multi-acquisition API (with some kind of 'acquisition handle') if we're going to break the API anyway.