Add data tracks schema metadata#1553
Conversation
|
| bytes definition = 2; | ||
| } | ||
|
|
||
| enum DataTrackSchemaFormat { |
There was a problem hiding this comment.
I think some of these at least should have comments/links to specs
There was a problem hiding this comment.
Added. Taking another look at the MCAP spec, I realized it isn't a good idea to conflate the concept of message (frame in data track terminology) and schema encoding, so I have separated them here accordingly.
There was a problem hiding this comment.
It is not immediately clear to me what a schema encoding is (vs a frame encoding). Are both needed, and if so can we provide some info in the proto file?
ladvoc
force-pushed
the
ladvoc/schema-metadata
branch
from
f1a3f86 to
b16bce9
Compare
ladvoc
force-pushed
the
ladvoc/schema-metadata
branch
from
897ebab to
61116ab
Compare
boks1971
left a comment
boks1971
left a comment
There was a problem hiding this comment.
Probably hold this till we have some form of implementation?
Should AddDataTrack request fail if there is an existing schema id, but the definition bytes do not match? If so, need to add another error reason for this case.
Should schemas be per publisher? If that is the case, GetSchema should include participant.
If not per publisher, (i. e. per room), becomes trickier to coordinate in a distributed room, so would be good to avoid that.
| Encryption.Type encryption = 4; | ||
|
|
||
| // Type information for this track. If unset, the track is untyped and frame payloads | ||
| // are treated as opaque bytes. |
There was a problem hiding this comment.
Technically, there is no treatment in our system, right? Probably stop at the track is untyped?
There was a problem hiding this comment.
Can one of frame_encoding/schema_encoding be set and the other unset? Not sure if we want to elaborate the comment to say the typical encoding combinations and also maybe mention if some combination is prohibited. Maybe, a better place to add those might be AddDataTrack which is more user facing.
There was a problem hiding this comment.
oh you have
optional DataTrackSchemaId schema = 2;
Should that be optional or should it just be default unspecified?
| INVALID_NAME = 8; | ||
| DUPLICATE_HANDLE = 9; | ||
| DUPLICATE_NAME = 10; | ||
| INVALID_SCHEMA = 11; |
There was a problem hiding this comment.
maybe UNKNOWN_SCHEMA, there is no validation of schema in our system, so INVALID technically is not quite right. UNKNOWN feels better.
3 participants
No description provided.