Closed Bug 908144 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Web Audio, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox25 --- fixed
firefox26 --- fixed

People

(Reporter: padenot, Assigned: padenot)

Details

(Whiteboard: [blocking-webaudio+])

Attachments

(1 file, 1 obsolete file)

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.
Whiteboard: [blocking-webaudio+]
I've got half a patch written, taking.
Assignee: nobody → paul
Whiteboard: [blocking-webaudio+]
Whiteboard: [blocking-webaudio+]
Thanks Paul. The demo doesn't play anything at all for me. Controls seem to be broken.
This fixes the noise. There are other problems with OscillatorNode, but I'll
open separate bugs.
Attachment #794678 - Flags: review?(ehsan)
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...
Attachment #794678 - Flags: review?(ehsan) → review+
Yes, we need a SetNull here, indeed.

I also added a test.
Attachment #794697 - Flags: review?(ehsan)
Attachment #794678 - Attachment is obsolete: true
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.
Attachment #794697 - Flags: feedback+
Should also remove the dump() statements from the test.
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.
Attachment #794697 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/19c5ee091594
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Whiteboard: [blocking-webaudio+] → [blocking-webaudio+][checkin-needed-aurora]
https://hg.mozilla.org/releases/mozilla-aurora/rev/5ee1d298bc2d
Whiteboard: [blocking-webaudio+][checkin-needed-aurora] → [blocking-webaudio+]
You need to log in before you can comment on or make changes to this bug.