WebAudio divide-by-zero crash [mozilla::dom::AudioBufferSourceNodeEngine::Resampler]

RESOLVED FIXED in mozilla23

Status

()

defect
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: posidron, Assigned: padenot)

Tracking

(Blocks 1 bug, {crash, testcase})

Trunk
mozilla23
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

6 years ago
Posted file testcase
content/media/webaudio/AudioBufferSourceNode.cpp:129

      mResampler = speex_resampler_init(mChannels, mSampleRate,
*                                       ComputeFinalOutSampleRate(),
                                        SPEEX_RESAMPLER_QUALITY_DEFAULT,
                                        nullptr);

Tested with m-i changeset: 130174:ea5490a3bca7
Reporter

Comment 1

6 years ago
Posted file callstack
Assignee: nobody → paul
On Windows: bp-6d206d97-c7b5-4ffd-ae17-2ae212130430.
Assignee: paul → nobody
Crash Signature: [@ mozalloc_abort(char const*) | NS_DebugBreak | mozilla::dom::AudioBufferSourceNodeEngine::Resampler(unsigned int)] [@ update_filter ]
OS: Mac OS X → All
Hardware: x86_64 → All

Updated

6 years ago
Assignee: nobody → paul
Status: NEW → ASSIGNED
Posted patch Test. (obsolete) — Splinter Review
and a crashtest.
Attachment #743577 - Flags: review?(ehsan)
Posted patch Test.Splinter Review
Sorry, forgot to `hg add`.
Attachment #743585 - Flags: review?(ehsan)
Attachment #743577 - Attachment is obsolete: true
Attachment #743577 - Flags: review?(ehsan)

Comment 6

6 years ago
Comment on attachment 743562 [details] [diff] [review]
Validate the playbackRate before using it.

Review of attachment 743562 [details] [diff] [review]:
-----------------------------------------------------------------

r=me minus the cubeb changes which don't seem intentional.

::: content/media/webaudio/AudioBufferSourceNode.cpp
@@ +11,5 @@
>  #include "AudioNodeStream.h"
>  #include "AudioDestinationNode.h"
>  #include "PannerNode.h"
>  #include "speex/speex_resampler.h"
> +#include "WebAudioUtils.h"

You shouldn't need this #include here.
Attachment #743562 - Flags: review?(ehsan) → review+

Comment 7

6 years ago
Comment on attachment 743585 [details] [diff] [review]
Test.

Review of attachment 743585 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/test/test_bug867089.html
@@ +27,5 @@
> +  source2.playbackRate.value = -1.0;
> +  source2.connect(ctx.destination);
> +  source2.start(0);
> +
> +  var source2 = ctx.createBufferSource();

Nit: source3.
Attachment #743585 - Flags: review?(ehsan) → review+

Comment 8

6 years ago
Actually, you're not waiting for the graph to play back in the test, so it may never get enough time to render before being paused when we go and run the next test.  Does this test crash without your patch?
Yes, consistently. Should I put a |setTimeout| before finishing, for good measure?

Comment 10

6 years ago
(In reply to comment #9)
> Yes, consistently. Should I put a |setTimeout| before finishing, for good
> measure?

Hmm, no, if the test crashes without your patches, then it's good!

Comment 13

6 years ago
That's just a typo in the test, will reland now.

Updated

6 years ago
Blocks: webaudio
https://hg.mozilla.org/mozilla-central/rev/79220f949406
https://hg.mozilla.org/mozilla-central/rev/a0d7d78facd4
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Reporter

Updated

6 years ago
Blocks: 875414

Comment 16

6 years ago
Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.
Component: Video/Audio → Web Audio
You need to log in before you can comment on or make changes to this bug.