Closed Bug 1097849 Opened 8 years ago Closed 8 years ago

Verify sane audio sample rates in MediaEncoder


(Core :: Audio/Video, defect)

Not set





(Reporter: rillian, Assigned: rillian)



(1 file, 3 obsolete files)

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.
Attached patch Assert valid audio sample rates (obsolete) — Splinter Review
Per irc discussion.
Assignee: nobody → giles
Attachment #8521554 - Flags: review?(tterribe)
Attachment #8521554 - Flags: review?(tterribe) → review+
s/196/192/ carrying forward r=derf
Attachment #8521554 - Attachment is obsolete: true
Attachment #8521564 - Flags: review+
Actual updated patch. This is what landed. Sorry for the confusion.
Attachment #8521564 - Attachment is obsolete: true
Attachment #8521569 - Flags: review+
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 ;)
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 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+
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.