Update the semantics according to what is found in Draft 12 of the Matroska spec

[robUx4]

Some elements modified (should be harmless), so element added (shouldn't break API), some elements are now forbidden to write (should have never been used).

[mbunkus]

This breaks MKVToolNix, and it's likely due to a bug in the specs regarding CodecDelay.

CodecDelay is currently defined as mandatory without a default value (mandatory due to minOccurs="1" maxOccurs="1", and it's missing a default attribute). This translates to the following situation in libmatroska:

1. When you create a new KaxTrackEntry, it'll be populated with instances of all mandatory elements, which now includes CodecDelay.
2. If the application doesn't modify that existing CodecDelay child, its bValueIsSet will remain false.
3. As soon as the application tries to render the element, libebml will fail the following assertion:

lib/libebml/src/EbmlElement.cpp:596: filepos_t libebml::EbmlElement::Render(libebml::IOCallback &, bool, bool, bool): Assertion `bValueIsSet || (bWithDefault && DefaultISset())' failed.

Furthermore, the change to make CodecDelay mandatory but not have a default value means that suddenly all files that don't contain a CodecDelay element are now invalid.

[robUx4]

The default value was removed in https://github.com/ietf-wg-cellar/matroska-specification/commit/743317d06088e8a732a7b7f880386d8b0435142b to avoid people using it. But in fact it should have been added back when making it mandatory (or not make it mandatory).

Thanks for testing and nothing this! I'll post a fix to the specs.

[robUx4]

Removed the commit that changed the KaxCodecDelay default value.

[mbunkus]

Just FYI, this branch currently completely messes up MKVToolNix. I'm still in the middle of debugging and cannot say what's wrong exactly, just that bisecting shows that the commit KaxSemantic.cpp: make KaxInfo/KaxTracks elements unique causes very weird behavior — which doesn't make any sense at all.

Please don't merge yet. I'll try to get to the bottom of the issue(s) over the next couple of days & will report back.

[mbunkus]

I've done more testing, mostly on all the other commits. They're all fine. Haven't figured out why KaxSemantic.cpp: make KaxInfo/KaxTracks elements unique breaks MKVToolNix yet.

Here's my proposal:

• Redo the branch but leave out KaxSemantic.cpp: make KaxInfo/KaxTracks elements unique for the time being
• Let me test that branch
• Merge that branch
• Refile KaxSemantic.cpp: make KaxInfo/KaxTracks elements unique as a new, single-commit PR

I'm also not convinced that KaxSemantic.cpp: make KaxInfo/KaxTracks elements unique is actually the right thing to do anyway. However, I don't want to have that discussion block merging of all the other changes.

Does this sound good to you?

[robUx4]

Fine for me. I wonder is this is because you might be putting an extra copying of the Tracks somewhere else in the file to protect it ? The change in the specs come from https://github.com/ietf-wg-cellar/matroska-specification/pull/438. It is making recurring elements unique as in the content is unique. But they are also recurring, so they can actually be multiple times in the same parent (Segment). There's no differentiation in libmatroska, so technically they can be multiple times in the same parent.

So this commit should go away. I have to fix the generator to handle the recurring attribute.

[robUx4]

BTW excellent catch. You've got a very useful test suite :)

[robUx4]

And so it turns out that Chapters are also recurring and should be allowed multiples times. I added a corresponding commit, which might also trip on your tests.

[mbunkus]

Thanks! I'll test this new variant later today.