908144 - Terrible noise on OscillatorNoise demo, when no sound should be output

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Paul Adenot (:padenot)]

STR:
- Open http://plnkr.co/edit/dv5iEPuMun0EIdmt9Y6n?p=preview (warning, it output terrible loud noise)
- Press play
- Wait for the track to finish

Expected result:
- One should hear silence before Play is clicked, then the track, then silence again.

Actual result:
- Terrible noise is heard instead of silence, everything is just fine otherwise.

Comment 1

[Paul Adenot (:padenot)]

I've got half a patch written, taking.

Comment 2

[Ralph Giles (:rillian)]

Thanks Paul. The demo doesn't play anything at all for me. Controls seem to be broken.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Don't output sound before start() is called when using OscilatorNode r=

This fixes the noise. There are other problems with OscillatorNode, but I'll
open separate bugs.

Comment 4

[(no longer active)]

Comment on attachment 794678 [details] [diff] [review]
Don't output sound before start() is called when using OscilatorNode r=

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

Can you write a test for this, please?  Thanks!

::: content/media/webaudio/OscillatorNode.cpp
@@ +245,5 @@
>      MOZ_ASSERT(mSource == aStream, "Invalid source stream");
>  
>      TrackTicks ticks = aStream->GetCurrentPosition();
> +    if (mStart == -1) {
> +      return;

You should SetNull aOutput here...

Comment 5

[Paul Adenot (:padenot)]

Attachment: Don't output sound before start() is called when using OscilatorNode r=

Yes, we need a SetNull here, indeed.

I also added a test.

Comment 6

[Ralph Giles (:rillian)]

Comment on attachment 794697 [details] [diff] [review]
Don't output sound before start() is called when using OscilatorNode r=

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

LGTM.

::: content/media/webaudio/OscillatorNode.cpp
@@ +245,5 @@
>      MOZ_ASSERT(mSource == aStream, "Invalid source stream");
>  
>      TrackTicks ticks = aStream->GetCurrentPosition();
> +    if (mStart == -1) {
> +      aOutput->SetNull(WEBAUDIO_BLOCK_SIZE);

ComputeSilence(aOutput); ?

@@ +260,5 @@
>        ComputeSilence(aOutput);
>        *aFinished = true;
>        return;
>      }
> +

Please remove the trailing whitespace.

Comment 7

[Ralph Giles (:rillian)]

Should also remove the dump() statements from the test.

Comment 8

[(no longer active)]

Comment on attachment 794697 [details] [diff] [review]
Don't output sound before start() is called when using OscilatorNode r=

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

r=me with Ralph's comments addressed.

Comment 9

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19c5ee091594

Comment 10

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/mozilla-central/rev/19c5ee091594

Comment 11

[Paul Adenot (:padenot)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5ee1d298bc2d