hasenbanck / matroska-demuxer

A demuxer written in Rust that can demux Matroska and WebM container files.
Apache License 2.0
11 stars 6 forks source link

This file breaks the demuxer #10

Closed dwlsalmeida closed 1 year ago

dwlsalmeida commented 1 year ago

https://tempfile.io/en/mGU5snoiT6XkAs8/file

        if doc_type != "matroska" && doc_type != "webm" {
            return Err(DemuxError::InvalidEbmlHeader(format!(
                "unsupported DocType: {}",
                doc_type
            )));
        }
p doc_type
(alloc::string::String) doc_type = "webm {
  [0] = 'w'
  [1] = 'e'
  [2] = 'b'
  [3] = 'm'
  [4] = '\0'
}
dwlsalmeida commented 1 year ago

Researching a bit more here,

Null bytes are allowed by the spec:

https://github.com/ietf-wg-cellar/ebml-specification/blob/master/specification.markdown#string-element

String::from_utf8() does not trim

And lastly Strings in Rust are not null-terminated (I did not know that!) : https://github.com/rust-lang/rust/issues/39295

So the comparison fails (very confusingly btw)

hasenbanck commented 1 year ago

You could just trim the end of the str before we compare it:

https://doc.rust-lang.org/std/primitive.str.html#method.trim_end

PR is welcome.

hasenbanck commented 1 year ago

I updated the master branch to include the change. Please test if you have the time. Make sure to have currently current stable Rust (1.70), since this enables us to remove the once_cell dependency.

hasenbanck commented 1 year ago

Fix released with v0.5.0

dwlsalmeida commented 1 year ago

Hi @hasenbanck

Sorry, I've been OOO these past few days.

I just tested v0.5.0 and it works just fine. It also does not regress on any of the libvpx test files. Thank you for the fix.