In constructing tests for VP9 playback, we discovered Firefox will play badly muxed Opus tracks in webm. We should reject them instead. In particular, we should reject files whose CodecDelay track property doesn't match the value from the OpusHeader CodecPrivate.
Created attachment 8350500 [details] [diff] [review] 0001-Bug-951770-Reject-invalid-WebM-Opus-files-r-rillian.patch
Attachment #8350500 - Flags: review?(giles)
Comment on attachment 8350500 [details] [diff] [review] 0001-Bug-951770-Reject-invalid-WebM-Opus-files-r-rillian.patch Review of attachment 8350500 [details] [diff] [review]: ----------------------------------------------------------------- Can you add a test file for this as well?
Attachment #8350500 - Flags: review?(giles) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cddbc0d5183 Better to get this in even without a test.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4978efcdad9 The code generates a signed-compare warning because mCodecDelay is an unsigned long long, but CheckedInt64::value() returns an signed int64_t. Casting this warning away is safe. I built this locally, but without -Werror, so I didn't notice the problem until tbpl failed to build.
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2b6516c8713e for timeouts in test_playback.html.
Created attachment 8369293 [details] [diff] [review] reject invalid Opus pre-skip in WebM v2 Roll up warning and semicolon fixes from the previous landing attempt. Tests failed because detodos.webm had exactly the corrupt header we now reject. - move detodos.webm to invalid-preskip.webm - make a new detodos.webm. Also changed the debug message to use PR_LOG_WARNING and mention the file is invalid.
Thanks, Chris. Here we go again. https://hg.mozilla.org/integration/mozilla-inbound/rev/06e99d1896ce
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Flags: in-testsuite- → in-testsuite+
status-firefox27: --- → unaffected
status-firefox28: --- → affected
status-firefox29: --- → affected
tracking-firefox29: --- → ?
Comment on attachment 8369293 [details] [diff] [review] reject invalid Opus pre-skip in WebM v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): Feature Bug 938686. User impact if declined: There's some buggy encoder software generating invalid files. We want user agents to reject these as soon as possible to ensure fixed encoders get deployed. Testing completed (on m-c, etc.): Landed on m-c with tests, verified manually against wild files. Risk to taking this patch (and alternatives if risky): Risk is low. This is a new media format and use by content developers is still in the exploratory stage. The larger risk is that some bug in the patch affects playback of non-Opus WebM files, but we have tests. String or IDL/UUID changes made by this patch: none.
Attachment #8369293 - Flags: approval-mozilla-aurora?
Attachment #8369293 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-firefox29: affected → fixed
status-firefox30: --- → fixed
You need to log in before you can comment on or make changes to this bug.