Skip to content

fix incorrect VINT encoding for VINT numbers 2^(7*k)-1 - #10

Open
brunoxylo wants to merge 1 commit into
b01o:masterfrom
brunoxylo:VInt_encoding_fix
Open

brunoxylo wants to merge 1 commit into
b01o:masterfrom
brunoxylo:VInt_encoding_fix

Conversation

@brunoxylo

Copy link
Copy Markdown

fix incorrect VINT encoding for VINT numbers 2^(7*k)-1

The BUG

For Element IDs the VInt64 must use the lowest possible encoding length. However this constraint is not mandatory for Element Data Sizes. This is because the final Element Data Size might not be known when the header of this element is written (see rfc8794 section-6.1).
rfc8794 section-6.2 describes how unknown sizes of a ebml elements data are encoded: -> The VINT_data part contains only ones. This conflicts with encoding element lengths of size 2^(7k) − 1 for k ∈ N and 1 ≤ k ≤ 7 (127, 16383, 2097151, …). For example 127 encoded as minimal VINT would result in 0xFF, which would be considered 'unknown size' (VINT_data all ones). Hence we cant encode it minimally here and need a two byte VINT encoding ( 0x407F).
The previous implementation only considered this for encoding 127 . But as explained above a unknown Element Data Size does not have to be minimal. Unkown size might also be encoded for example as 0x7FFF (2 byte version). This makes sense, because an encoder might write the actual size of the Element Data later and wants the required bytes for it to be reserved.

The fix

This fix ensures proper encoding and decoding for all 2^(7k) − 1 numbers and not just 2^(7) − 1 a.k.a 127 .

Impact of the bug

Chromium's WebM/MSE parser is strictly adhering to the rule "If all VINT_data are ones, then its unknown size". When remuxing, it happens form time to time that the simple block data sizes are exactly 2^(7k) − 1. Chromium treats those as unknown size , which is illegal for simple blocks. Hence playback stops and the MSE pipline errors with PipelineStatus::CHUNK_DEMUXER_ERROR_APPEND_FAILED.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant