mts: metadata should be versioned

[Alan Noble]

Our metadata scheme will evolve over time, so it should be versioned.

I recommend that the first 2 bytes of the metadata representation be reserved for version information.

[Saxon Milton]

Just to clarify, you would like version in all metadata for example, if we have the time desscriptor, you would like the version to be first two bytes of timeDescData { timeDescTag, timeDescLen, timeDescData }

and similarly if we had location, we would also have version in first two bytes of location,

{ locationDescTag, locationDescLen, locationDescData } ?

[Alan Noble]

No, the versioning applies to the meta data scheme in effect, so only has to occur once per PSI.

[Alan Noble]

Also, there should be a Version() method so clients know what version of metadata they are dealing with. That said, meta data methods such as TimeFrom should also be version aware and "do the right thing".

[Saxon Milton]

Okay, this is an example of specific data for a pmt. You'll note we have two psi.Desc (descriptors) one for time and one for location. The only place we're allowed to put custom things like a version number is in 'Dd' the descriptor data fields. So I'm wondering what we do in the circumstance where we have more than one type of metadata.

Sd: &psi.PMT{
            Pcrpid: 0x0100,
            Pil:    psi.PmtTimeLocationPil,
            Pd: []psi.Desc{
                {
                    Dt: psi.TimeDescTag,
                    Dl: psi.TimeDataSize,
                    Dd: make([]byte, psi.TimeDataSize),
                },
                {
                    Dt: psi.LocationDescTag,
                    Dl: psi.LocationDataSize,
                    Dd: make([]byte, psi.LocationDataSize),
                },
            },
            Essd: &psi.ESSD{
                St:   0x1b,
                Epid: 0x0100,
                Esil: 0x00,
            },
        },

[Alan Noble]

This is another reason why we need a more flexible representation for meta data. We should just encode a all of key/value pairs into one psi.Dec (prefixed with the version info). There seems to be no point in have multiple descriptors, unless I'm missing something.

[Saxon Milton]

sure, we can definitely just use one descriptor. I initially started using multiple to make it easier for players to parse if we wanted them to display descriptor data, but this might not be an issue.

[Alan Noble]

Perhaps it is easier to bite the bullet and collapse the representation down to a single psi.Desc now? Rename TimeDescTag to MetaDataTag, etc. and get rid of of the location desc.

Then write to the [] byte as follows:

bytes[0:2] // version
bytes[2:10] // timestamp
bytes[10:] // location

The existing methods (TimeFrom, etc.) can stay the same for now, but the implementation would change.

[Alan Noble]

Or, if you want a representation which supports arbitrary tags, which is where we are headed anyway:

bytes[0:2] // version
bytes[2:] // key/value string pairs in CSV, i.e., "ts=1545359624,lat=-35.345,lng=138.667"

Using strings for timestamps is not as bad as it sounds. Timestamps today are 10 bytes in length as a string, or 8 bytes as an int64, i.e., only 2 bytes difference. Similarly lat and lng are typically only 9 digits or 10 digits, not much worse than a float64. And everything is a string :-)

Trivial to parse and could also be passed to cvs.Reader.

[Saxon Milton]

Yeah let's do it. Second option sounds good to me.

[Alan Noble]

Re the version, I would suggest we keep byte[0] reserved for now (set to 0), and split byte[1] into a major version (MSB nibble) and a minor version (LSB nibble), e.g.,

0x10 // version 1.0 (our initial version)
0x11 // version 1.1 (minor version, e.g., addition of a new tag)
0x20 // version 2.0 (major version, e.g., meta data representation has changed)

Clients can safely assume that a minor version change is compatible with their current implementation.

[Alan Noble]

Also when bytes[0] and bytes[1] are both zero, no metadata follows (although all of our generated transport streams should have metadata, even if its empty.

[Saxon Milton]

Alright, sounds good!

[Alan Noble]

In retrospect, let's use tab-separated values (TSV) for representing values. That way we don't need to escape commas in values. I envision one of the metadata keys will be "tags", which will also be comma-separated values.

Related point, despite my example in the previous comment, there is no need for separate lat=a lng=b alt=c keys, just a loc key, i.e., loc=a,b,c.

[Saxon Milton]

I'm slightly confused about how you want the version done Alan. So first byte bytes[0] is zero, and bytes[1] is split into minor version and major version, but in that code snippet you have above, you have an initial version in there as well ? Where does that go ? Do want bytes[0] as reserved i.e. 0, bytes[1] as initial version and bytes[2] split into minor and major ?

[Saxon Milton]

and to clarify, the metadata after the version will look like this (as str): loc=a,b,c\ts=d ?

should we actually be doing a tag, then data length, and then data ? so that when we parse we know where the data ends ? We could have data lengths for certain metadata const, but imo this makes this less flexible and harder to incorporate arbitrary metadatas.

[Alan Noble]

Yes, it will look like loc=a,b,c\ts=d

We only need to encode the length for a whole string, not individual keys and values. I recommend a uint16 between the version and the first key, i.e.,

byte[0]   // reserved - always zero
byte[1]   // version (MS nibble = major, LS nibble = minor)
byte[1:2] // length
byte[2:]  // key value pairs in TSV

PS In the olden days we should just null terminate the the last value, i.e.,

loc=a,b,c\ts=d\0

Re the earlier remark, yes, our initial version is the major version 1.0. Therefore byte[1]'s MS nibble is 1 and the LS nibble is 0, i..e, byte[1] = 0x10

The others were just examples of what future versions would look like.

[Alan Noble]

So we have 4-byte metadata "header", which is neat. Read the header, then check the version. If it is version 1.0, interpret byte[1:2] as a uint16 length, then read that much data. In a future, version byte[1:2] might well be interpreted differently, but byte[1] will always be the version.

[kortschak]

Given that a packet is 188 bytes long, the metadata length field needs only be 1 byte long unless you are splitting over packets. The code as it exists does not look like this is the case.

[Saxon Milton]

Alright thanks, this makes sense now. I'm not sure why I thought we needed a length for each part of the meta.

Dan, what part of the code exactly ?

[Alan Noble]

I think we will want the option to split over multiple packets, even if initially we don't do so. Once we start tagging video with the names of species identified in each one second snippet, it will likely exceed one packet in size.

[kortschak]

The metadata serialisation currently operates on single packets, so currently there is no way to spill long metadata over multiple packets. This could change, but would require a few changes to the architecture of the encoding.

If they are aplit, there should probably also be some kind of integrity check on the collection, even if it's only a continuation counter.

[Alan Noble]

Good point but I think we can just use the underlying MTS continuity counter (CC). If the length exceeds 1 packet, the decoder just records the the CC for the current packet before reading the next packet. If the next packet's CC is not consecutive, the current PSI is treated as empty - which is a case we have to handle regardless.

[Alan Noble]

But none of the above need be implemented in V1.0. In V1.0 just throw an error if attempting to write more than fits into one packet.

[Saxon Milton]

This proposal was implemented in PR #131.