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)

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
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
Assignee: nobody → paul
Status: NEW → ASSIGNED
oops.
Attachment #743562 - Flags: review?(ehsan)
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 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 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+
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?
(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!
That's just a typo in the test, will reland now.
Blocks: webaudio
https://hg.mozilla.org/mozilla-central/rev/79220f949406
https://hg.mozilla.org/mozilla-central/rev/a0d7d78facd4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Blocks: 875414
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.