More feedback and prototyping of new camera API (chapter 3)

[henrypinkard]

This issue is for providing feedback on the new camera API.

With this v2, we're now in a prototyping phase where I will be implementing it on a Basler camera.

Previous discussions of this API can be found in:

Chapter 1

Chapter 2

Version 1 of the proposed API can be found here

[marktsuchida]

@henrypinkard Sorry for the silence - I'll try to comment on this and your other proposals in the next few days.

[henrypinkard]

@marktsuchida No worries, I am mid-prototyping on this one right now and am accumulating questions as a go. I think it would be more efficient to batch it all into one larger discussion later

[henrypinkard]

*Accumulating questions and fixing mistakes

[marktsuchida]

@henrypinkard Regarding the timing events (such as CameraEventFrameEnd),

• These seem like a good opportunity for breaking up the problem into smaller units: presumably these can be added in a second round, completely independent from the main trigger configuration API.
• Can you point me to one or more actual camera API docs that correspond to these events? I'm trying to think about whether they should be callbacks more strongly associated with individual acquisitions rather than being "events".

[henrypinkard]

• Can you point me to one or more actual camera API docs that correspond to these events? I'm trying to think about whether they should be callbacks more strongly associated with individual acquisitions rather than being "events".

https://docs.baslerweb.com/event-notification

and from GenICam

So far as I can tell, events/callbacks like ExposureEnd/Line0RisingEdge/etc (or some equivalent) are needed to implement triggers and acquisitions correctly in the GenICam way of doing things.

• These seem like a good opportunity for breaking up the problem into smaller units: presumably these can be added in a second round, completely independent from the main trigger configuration API

Based on everything I've seen thus far, I think breaking this into multiple units would create substantially more work, because I would have to figure out how to interleave our existing camera API with GenICam's concepts, which seems to me a lot more challenging and with potential for edge cases and errors than to simply adopt the subset of its specifications that make sense for micro-manager all at once.

[marktsuchida]

So far as I can tell, events/callbacks like ExposureEnd/Line0RisingEdge/etc (or some equivalent) are needed to implement triggers and acquisitions correctly in the GenICam way of doing things.

Now that I think about it, this seems correct -- at least if using software triggers.

• These seem like a good opportunity for breaking up the problem into smaller units: presumably these can be added in a second round, completely independent from the main trigger configuration API

Based on everything I've seen thus far, I think breaking this into multiple units would create substantially more work, because I would have to figure out how to interleave our existing camera API with GenICam's concepts, which seems to me a lot more challenging and with potential for edge cases and errors than to simply adopt the subset of its specifications that make sense for micro-manager all at once.

Fair enough.

[marktsuchida]

Do non-GenICam-based camera APIs also have a similar event mechanism?

[henrypinkard]

Yes, looking through several of the scientific cameras in 3rdparty, it seems this is a common mechanism

[marktsuchida]

Which ones should I look at?

[henrypinkard]

I didn't want to post info about 3rd party SDKs here and now I forget which I looked at :), but it should be pretty easy to look through a few SDKs and find them

Can you point me to one or more actual camera API docs that correspond to these events? I'm trying to think about whether they should be callbacks more strongly associated with individual acquisitions rather than being "events".

What do you think is the advantage of making them callbacks rather than events? It seems like the functionality is fairly similar, but the latter has terminology more consistent with GenICam, which is the design principle we've embraced for most of this API

[marktsuchida]

That comment was just due to me not having looked at any of the event APIs. I think events are fine if that is what all/most of the SDKs use.

The nice thing about acquisition-specific callbacks is that an app wouldn't have to explicitly de-register its event handler -- the end of the acquisition would be the end of the calls received by that handler. It prevents a category of bugs. But the MMDevice API (and probably even the MMCore API in this case) should probably follow what the device SDKs have; we definitely don't want each device adapter to replicate logic to convert semantics.

[henrypinkard]

@marktsuchida quick question: Is there a hard reason/do you have a preference that the new camera API be additions to the existing CCameraBase API, rather than subclass of this? I'm working on standardized device properties (#258) right now and trying to find the cleanest way to associate certain standard properties with certain device types. Specifically, I want to make certain standard properties required for the new camera API but not the old, and if they're the same class I can't see an obvious way to do this without runtime checks that might make things more confusing for device adapter developers

[marktsuchida]

@henrypinkard My thinking (and I may have told you, can't remember) was that the new camera class should be a sibling of CCameraBase, with the common base class of the two being a cleaned-up version of the current CCameraBase but with the pretend sequence acquisition implementation that calls snap (and all associated stuff, like BaseSequenceThread) removed.

It would certainly be best if the step of introducing the new base class were its own PR rather than tied into the new camera API. And many cameras should be able to derive directly from the new base if they don't make use of the messy stuff. The subclass that preserves the current CCameraBase fully can be deprecated (and maybe renamed to LegacyCameraBase).

[henrypinkard]

Got it, makes sense, thanks. Then what about the MM::Camera virtual API class. Does that need to contain the union of current and new camera APIs? Or should MM::NewCamera be a subclass of MM::Camera? or should it be an entirely separate class?

[marktsuchida]

That may depend a bit on how extensive the changes for the new API are. If, as suggested, the legacy base (which will keep the name CCameraBase in the beginning) and the new base (I'll tentatively call it CCameraBase3) are both subclasses of CCameraBase2 (the cleaned-up interface), then necessarily there can only be one interface class (MM::Camera), because multiple inheritance in C++, at least in this scenario, is a can of worms (including at the binary interface level). That hopefully is not limiting, because MMCore can query MM::Camera to ask if it is CCameraBase3 or not and act accordingly.

But I guess it's possible that I'll change my mind depending on what the actual new interface will look like. If there is not much in common between the old- and new-style interfaces, of course it wouldn't make sense to share the interface -- but I'd argue that as much as possible should be kept common to ease migration.

It might be more flexible to have the following hierarchy:

MM::Camera <- CCameraBaseBase <-+- CCameraBase2 <- CCameraBase
                                |
                                +- CCameraBase3

so that requirements specific to the old-style cameras can be enforced by CCameraBase2.

Note that virtual functions and their overrides can now be marked final. So CCameraBase3 can, for example, forbid the concrete camera from implementing any (old-style) functions that it should not. If CCameraBase3 adds new functions that CCameraBase[2] cameras must not implement, the same can be done.

(The idea behind the tentative names is that all existing cameras will continue to compile with CCameraBase, and many of them can be "upgraded" to derive (directly) from CCameraBase2, without any or much code change; others may require some work. CCameraBase would be deprecated. At an appripriate point in time, CCameraBase can be renamed to LegacyCameraBase or similar, to further clarify its status. In the meantime, cameras deriving from CCameraBase2 can be upgraded to CCameraBase3 by conforming to the new requirements.)