[FFmpeg-devel] [PATCH] avformat/matroskaenc: remove MAX_TRACKS limit

[Andreas Rheinhardt]

Jan Chren (rindeal):
> RnJvbSA0M2U0Yjk2OTVhNDM0MjExMmIxMTE3ZGI2NTI2YmFmYTFmOWRhYjNlIE1vbiBTZXAgMTcg
> MDA6MDA6MDAgMjAwMQpGcm9tOiBKYW4gQ2hyZW4gKHJpbmRlYWwpIDxkZXYucmluZGVhbEBnbWFp
> bC5jb20+CkRhdGU6IFRodSwgMSBBcHIgMjAyMCAwMDowMDowMCArMDAwMApTdWJqZWN0OiBbUEFU
> Q0hdIGF2Zm9ybWF0L21hdHJvc2thZW5jOiByZW1vdmUgTUFYX1RSQUNLUyBsaW1pdAoKSXQgd2Fz
> IGludHJvZHVjZWQgaW4gN2JlMGY0OGEzMjE1NWVmOWY0NzFmZmM1YTFiNDFkNjYyZWEzMzdmMQp0
> byBzZXQgc2l6ZSBvZiBhbiBhcnJheSBzdHJ1Y3QgZmllbGQsIGJ1dCB0aGF0IGJhZCBkZXNpZ24g
> d2FzIGZpeGVkCmluIDY1ZWY3NGY3NDkwMDU5MGYxMzRiNGExMzBlOGY1NmU1MjcyZDE5MjUuCkFz
> IHN1Y2ggdGhpcyBhcnRpZmljaWFsIGxpbWl0IHNlcnZlcyBubyBwdXJwb3NlIGFueW1vcmUuCgpT
> aWduZWQtb2ZmLWJ5OiBKYW4gQ2hyZW4gKHJpbmRlYWwpIDxkZXYucmluZGVhbEBnbWFpbC5jb20+
> Ci0tLQogbGliYXZmb3JtYXQvbWF0cm9za2FlbmMuYyB8IDExIC0tLS0tLS0tLS0tCiAxIGZpbGUg
> Y2hhbmdlZCwgMTEgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvbGliYXZmb3JtYXQvbWF0cm9z
> a2FlbmMuYyBiL2xpYmF2Zm9ybWF0L21hdHJvc2thZW5jLmMKaW5kZXggNWRhZTUzMDI2ZDguLjNh
> ZjA1NDAzZDFkIDEwMDY0NAotLS0gYS9saWJhdmZvcm1hdC9tYXRyb3NrYWVuYy5jCisrKyBiL2xp
> YmF2Zm9ybWF0L21hdHJvc2thZW5jLmMKQEAgLTExNywxMCArMTE3LDYgQEAgdHlwZWRlZiBzdHJ1
> Y3QgbWt2X2F0dGFjaG1lbnRzIHsKICNkZWZpbmUgTU9ERV9NQVRST1NLQXYyIDB4MDEKICNkZWZp
> bmUgTU9ERV9XRUJNICAgICAgIDB4MDIKIAotLyoqIE1heGltdW0gbnVtYmVyIG9mIHRyYWNrcyBh
> bGxvd2VkIGluIGEgTWF0cm9za2EgZmlsZSAod2l0aCB0cmFjayBudW1iZXJzIGluCi0gKiByYW5n
> ZSAxIHRvIDEyNiAoaW5jbHVzaXZlKSAqLwotI2RlZmluZSBNQVhfVFJBQ0tTIDEyNgotCiB0eXBl
> ZGVmIHN0cnVjdCBNYXRyb3NrYU11eENvbnRleHQgewogICAgIGNvbnN0IEFWQ2xhc3MgICAqY2xh
> c3M7CiAgICAgaW50ICAgICAgICAgICAgIG1vZGU7CkBAIC0yNjA0LDEzICsyNjAwLDYgQEAgc3Rh
> dGljIGludCBta3ZfaW5pdChzdHJ1Y3QgQVZGb3JtYXRDb250ZXh0ICpzKQogewogICAgIGludCBp
> OwogCi0gICAgaWYgKHMtPm5iX3N0cmVhbXMgPiBNQVhfVFJBQ0tTKSB7Ci0gICAgICAgIGF2X2xv
> ZyhzLCBBVl9MT0dfRVJST1IsCi0gICAgICAgICAgICAgICAiQXQgbW9zdCAlZCBzdHJlYW1zIGFy
> ZSBzdXBwb3J0ZWQgZm9yIG11eGluZyBpbiBNYXRyb3NrYVxuIiwKLSAgICAgICAgICAgICAgIE1B
> WF9UUkFDS1MpOwotICAgICAgICByZXR1cm4gQVZFUlJPUihFSU5WQUwpOwotICAgIH0KLQogICAg
> IGZvciAoaSA9IDA7IGkgPCBzLT5uYl9zdHJlYW1zOyBpKyspIHsKICAgICAgICAgaWYgKHMtPnN0
> cmVhbXNbaV0tPmNvZGVjcGFyLT5jb2RlY19pZCA9PSBBVl9DT0RFQ19JRF9BVFJBQzMgfHwKICAg
> ICAgICAgICAgIHMtPnN0cmVhbXNbaV0tPmNvZGVjcGFyLT5jb2RlY19pZCA9PSBBVl9DT0RFQ19J
> RF9DT09LIHx8Cg==
First, the content of your mail is somehow base64 encoded.

Are you running into this limitation? If so, do the files that you
create with this patch that have more than 127 tracks work? They
shouldn't. The reason for this (and the reason that I didn't remove this
limit altogether in 65ef74f74900590f134b4a130e8f56e5272d1925) lies in
the way the TrackNumber is encoded in the (Simple)Blocks: They are
encoded as variable-length numbers, so that encoding small TrackNumbers
takes up less bytes than encoding bigger TrackNumbers. More precisely,
TrackNumbers in the range 1..127 are encodable on one byte. And the way
they are written in mkv_write_block() and mkv_write_vtt_blocks() relies
on this. If you simply remove said limit, the tracks with TrackNumbers >
127 will not have any (Simple)Blocks associated with them; instead the
encoded TrackNumber will be the desired TrackNumber modulo 128 and the
(Simple)Block will appear to belong to the track with the encoded
TrackNumber (if one exists).** The results will of course be catastrophic.

Notice that I have sent a patchset that slightly increases the number of
allowed tracks (without fixing the root cause though, because I didn't
know if there are really people who run into this): [1] makes
attachments no longer count towards the limit; [2] increases the limit
to 127. I will resend said patchset soon.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/253452.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/252563.html

*: The reason for setting MAX_TRACKS to 126 instead of 127 is probably
that the encoding corresponding to the number 127 is special when
encoding lengths (which use the same variable-length number scheme):
Here it denotes an unknown length instead of a length of 127. But there
are no such values with a special meaning for encoded TrackNumbers.
**: Notice that a TrackNumber of 0 is invalid, so any (Simple)Block that
ought to belong to the tracks with TrackNumber 128, 256, 384, ... will
have no matching track at all. FFmpeg's Matroska demuxer treats
encountering such (Simple)Blocks as a sign of invalid data and it will
try to resync to the next level 1 element (typically the next Cluster).
Similar things can happen if you use attachments (in this case there may
be gaps in the used TrackNumbers).