Move ImageMetadata.h from MMDevice (back) to MMCore

[marktsuchida]

MMDevice/ImageMetadata.h defines the Metadata class and associated classes (MetadataSingleTag, MetadataKeyError, etc.).

Metadata is a somewhat confusing class because it's a key-value container, but the keys can be either (device label + tag) or just tag, except that half the time the device + tag is treated as a single string. And it appears to contain support for "array tags", which were never fully implemented (probably a good thing because I have issues with its design, but I digress). There is also a "read only" flag, also never fully supported.

It currently performs double duty:

• It is the container for camera devices to pass per-frame metadata to the Core (in serialized form). It is also used internally in CCameraBase for the AddTag()/RemoveTag()/GetTags() mechanism used by Multi Camera et al.

• It is the container for CMMCore to provide per-frame metadata for sequence acquisition images. In MMCoreJ it is more standard to use TaggedImage, but Metadata and related classes are also exposed and used in the MMCoreJ API. They are also exposed in the pymmcore API.

This double role makes it hard to make changes and improvements, because any change will potentially affect both camera device source compatibility (not the device interface version, any more) and the MMCore API at the same time. Also, MMDevice and MMCore could use different potential improvements. The fewer ways in which MMDevice constrains MMCore's API, the better, and the Metadata classes are the worst offender (the others are mostly just constants).

There are additional issues with the Metadata class being used in MMDevice:

• Some of the functions throw exceptions. This is not an appropriate design for something used in device adapters. We get away with it because use cases that could cause errors is rare.

• The license header says ImageMetadata.h is part of MMCore and is LGPL-licensed. This is purely an error we can correct, but betrays the origin of the code (explains why it's poorly matched to MMDevice's needs).

'Nuff said. So I propose that we move ImageMetadata.h back to MMCore, where it can continue to serve as an API type in MMCoreJ/pymmcore (and evolve or get deprecated as needed).

We would then need a replacement for MMDevice's purposes. In fact, I suggest we first add a replacement, then convert all cameras to use the replacement, then finally perform the moving of ImageMetadata.h.

The requirements for the replacement (let's call it MM::MetadataMap):

• The class name should be different from Metadata to avoid confusion when deealing with old camera code (e.g., those not in our repos).

• Provide straightforward replacements for all current and correct uses of Metadata by camera adapters

  • Most cameras just add tags and serialize

• Have no auxiliary classes (tags, exceptions) -- we don't need them in devices

• Support serialization/deserialization compatible with the existing Metadata (so that we can migrate cameras without bumping the DIV)

• Have unit tests, especially for the serialization/deserialization

  • We'll start by adding unit tests for Metadata's existing ser/des

Limiting the device-facing API to the absolute minimum will leave room for future improvements, including performance optimizations (some of which will require bumping the DIV; best to do that separately).

Here is what MetadataMap needs, I believe:

• Construct empty or copy of another instance; copy-assign
• PutTag(name, deviceLabel, value)
  • TBD: do we also need a version without deviceLabel? Or only such a version? See below.
• Serialize() (probably return internal const char *, not std::string)
• Deserialize() (not needed by camera devices, but it's probably best if MMCore uses a paired implementation)
• Clear() (strictly speaking optional, but reusing an existing instance could be good for performance so don't discourage it)

The following are probably NOT needed in the public API (some may be needed in CCameraBase's internal use):

• GetKeys(), GetSingleTag()
• RemoveTag()
• Everything else in Metadata (currently not called by devices, even indirectly)

There are two camera adapters (Andor, TIScam) that call CCameraBase::GetTagKeys() and CCameraBase::GetTagValue() (both protected). But the use is probably unnecessary and should be removed after verification. (EDIT: #832)

Leaving out random-access read/remov leaves open the possibility of a single-buffer implementation that serializes as it goes -- a possible future performance optimization.

Most uses in device adapters look like the following and will convert easily:

Pattern 1 - MetadataSingleTag + SetTag:

Metadata md;
MetadataSingleTag mst(MM::g_Keyword_Elapsed_Time_ms, label, true);
mst.SetValue(CDeviceUtils::ConvertToString(elapsed));
md.SetTag(mst);

Pattern 2 - PutImageTag (edited to reflect #830):

md.PutImageTag(MM::g_Keyword_Elapsed_Time_ms, elapsed);

-- and it's at this point that I realize that cameras produce different metadata tags depending on whether they use Pattern 1 or 2 😱

I'll need to create a separate issue for this inconsistency. But this is precisely why we need a structure that can be evolved separately for devices vs MMCore API. (EDIT: #831, #833)

[marktsuchida]

The Metadata class is also used by the ImgBuffer class in MMDevice (not to be confused with MMCore's ImgBuffer class). Fortunately, no device uses the member functions SetMetadata() and GetMetadata() in question so they can just be removed.

[marktsuchida]

Here is what MetadataMap needs, I believe:

Redesign after noticing that camera-generated metadata should be plain key-value, no device label:

• Let's call the class CameraImageMetadata so its purpose is clear
• Construct empty or copy of another instance; copy-assign
• PutTag(key, value)
• Serialize() (probably return internal const char *, not std::string)
• Clear() (strictly speaking optional, but reusing an existing instance could be good for performance so don't discourage it)
• There is intentionally no direct query or individual tag removal
• PutTag() on the same key overwrites (but may consume memory)

Leave out Deserialize(); MMCore needs to deserialize into a different type (existing Metadata for now) for downstream handling. The common serialization format (part of the Device Interface and also the MMCore API) will allow that.

Internal representation options:

1. Always keep serialized form; just append on PutTag(); clear on Clear(). This should be possible due to the details of the serialization format.
2. Keep 2 vector<string> for key and value; append on PutTag(). Probably not necessary.

[marktsuchida]

The metadata serialization format (formalized from Metadata::Serialize() and especially Metadata::Restore()):

• The format is line-oriented, terminated by \n
• The first line is the number of tags, in a format parsable by std::atol()
  • This implies that if the whole string is empty, we get 0 tags
  • Leading whitespace (no newlines, or course) is tolerated
  • Trailing garbage (starting with non-digit) is tolerated (but we should probably stipulate that only whitespace be used)
• Each tag follows as a multi-line sequence (5 lines for a single tag):
  • Line 1: s (single) or a (array); must be exact. Array tags have never been used and should be considered unsupported.
  • Line 2: key
  • Line 3: device label or _ for plain key (so always _ for camera-generated metadata)
  • Line 4: read-only flag: parsable by std::atoi(); nonzero = read-only (of unknown intent and never interpreted to my knowledge; cameras always set to true); should be 0 or 1 only
  • Line 5: value: for single tags, this is a single line and the whole line (without the \n) is the string value. On the writing side, value formatting is whatever operator<< produces for the type to a default std::ostringstream, but realistically it is string, integer, or floating-point (we should probably disallow others; producers can always format other types)
• If a tag with the same (key, label) is repeated (within the expected tag count), it overwrites the previous value and read-only flag for that (key, label)
• Any lines beyond what is expected from the tag count are ignored

Note how this allows MM::CameraImageMetadata to be implemented in a serialize-as-we-go fashion using a single std::string data member: we just append lines when PutTag() is called, and update the tag count, which can be made possible by having enough space-padded room in the first line. This should save an allocation per tag on the camera side, plus a bunch of pointer traversals and string comparisons.

The format spec should be documented in the new MM::CameraImageMetadata class, because the primary purpose of the format is transmitting data from camera device to Core. Any format changes require a DIV bump. In the Metadata class, once it is removed from MMDevice, we should warn that the serialization format may change without notice (and just deprecate Serialize() for application use, along with other unnecessary methods and array tags).

[marktsuchida]

It should be noted that keys, labels, and values containing newlines will corrupt the serialization. Maybe we should check and reject.

[marktsuchida]

Argh. Metadata::Restore() leaks a tag object when the same (key, label) is encountered. A simple bug to fix, but this means that we can only safely serialize duplicate tags from devices after bumping the DIV. Maybe first do it conservatively and simplify on or after DIV 75.