Closed
Bug 1097849
Opened 11 years ago
Closed 11 years ago
Verify sane audio sample rates in MediaEncoder
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: rillian, Assigned: rillian)
Details
Attachments
(1 file, 3 obsolete files)
5.20 KB,
patch
|
derf
:
review+
|
Details | Diff | Splinter Review |
We think the encoder sample rate is controlled but the audio driver, but we should verify this assumption to avoid passing unreasonable sample rates to the encoding implementation.
Assignee | ||
Comment 1•11 years ago
|
||
Per irc discussion.
Assignee: nobody → giles
Attachment #8521554 -
Flags: review?(tterribe)
Updated•11 years ago
|
Attachment #8521554 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 2•11 years ago
|
||
s/196/192/ carrying forward r=derf
Attachment #8521554 -
Attachment is obsolete: true
Attachment #8521564 -
Flags: review+
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Comment 4•11 years ago
|
||
Actual updated patch. This is what landed. Sorry for the confusion.
Attachment #8521564 -
Attachment is obsolete: true
Attachment #8521569 -
Flags: review+
Comment 5•11 years ago
|
||
Turns out we have unit tests that hit this assert. Backed out. Please make sure this is green on Try before attempting to re-land ;)
https://hg.mozilla.org/integration/mozilla-inbound/rev/cff5f009a657
https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3824640&repo=mozilla-inbound
Assignee | ||
Comment 6•11 years ago
|
||
Updated patch to address gtest failures.
- Return an error rather than asserting.
- Update unit tests to verify the valid sample rate boundaries.
MOZ_ASSERT isn't caught by the gtest framework and so is always fatal. Since we have unit tests verifying that invalid values are rejected, this doesn't pass automated tests. Returning an error instead is reasonable for an Init method, and we already do this for other parameters.
Attachment #8521569 -
Attachment is obsolete: true
Attachment #8521896 -
Flags: review?(tterribe)
Comment 7•11 years ago
|
||
Comment on attachment 8521896 [details] [diff] [review]
Assert valid audio sample rates v4
Review of attachment 8521896 [details] [diff] [review]:
-----------------------------------------------------------------
Hooray tests!
Attachment #8521896 -
Flags: review?(tterribe) → review+
Assignee | ||
Comment 8•11 years ago
|
||
This one builds on try. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7e51fe546d1d
Keywords: checkin-needed
Comment 9•11 years ago
|
||
Keywords: checkin-needed
Comment 10•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•