Sound notification for new mail always starts with a blop

This is not a mozilla problem, but a linux sound driver problem in general. On
my system, every time the system sound device is opened, there is a crackle.
OSS and ALSA do that; maybe other drivers (eg commercial OSS) change that.

No, XMMS (WinAMP clone), which uses OSS, doesn't exhibit this behaviour.

Well, there are some cracks but
- rarely (only sometimes during skipping tracks)
- minimal (much less volume; shorter)

*** Bug 217803 has been marked as a duplicate of this bug. ***

*** Bug 290953 has been marked as a duplicate of this bug. ***

This is a mozilla problem, not a driver problem.
The GTK(2) widget's nsSound::Play function (actually, its helper function nsSound::OnStreamComplete) sends the whole WAV file - including header and trailing garbage - to ESD, while ESD expects just the samples.

See
  http://lxr.mozilla.org/mozilla/source/widget/src/gtk/nsSound.cpp#130
or
  http://lxr.mozilla.org/mozilla/source/widget/src/gtk2/nsSound.cpp#134

Changing component, owner, milestone, to reflect today

Created attachment 215756
Send only sample data, not WAV file headers to ESD

Created attachment 233004
Do proper WAV file parsing

This works similarly to the previous patch, except it cleans up the code a bit, does more sanity checking and isn't bit rotted. Thanks to Uwe Dannowski for the analysis and initial patch.

+ if (i + audio_len >= dataLen)
+ audio_len = (dataLen - i) - 1;

Why are you dropping the last byte of the file?

+ for (PRUint32 j = 0; j < audio_len - 1; j += 2) {

I'd prefer "j + 2 <= audio_len"

+ } else
+ i++;

I'd prefer braces around the i++. Also, is it really correct to scan through the WAV file looking for the string "fmt "? Aren't there other chunks that could have binary data in them that happens to contain the string "fmt "?

(In reply to comment #10)
> + if (i + audio_len >= dataLen)
> + audio_len = (dataLen - i) - 1;
>
> Why are you dropping the last byte of the file?
>
Opps. I knew something was a bit off about this code.. Thanks

> Also, is it really correct to scan through
> the WAV file looking for the string "fmt "? Aren't there other chunks that
> could have binary data in them that happens to contain the string "fmt "?
>
I've added a check which makes sure we only parse one fmt chunk. It is fairly unlikely that fmt wouldn't the first chunk, so this should work out.

Created attachment 233589
Do proper WAV file parsing, v2

Updated w/ roc's comments.

Is there no way to do a proper "skip to the next chunk" operation as we search for the "fmt " chunk?

Created attachment 233669
Do proper WAV file parsing, v3

Properly skip unknown chunks, eliminate unused block_align variable.

Comment on attachment 233669
Do proper WAV file parsing, v3

excellent!

Checking in widget/src/gtk2/nsSound.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsSound.cpp,v <-- nsSound.cpp
new revision: 1.15; previous revision: 1.14
done

Comment on attachment 233669
Do proper WAV file parsing, v3

This patch has been baking for a while and there has been no regressions AFAIK. It makes the gtk2 nsSound code actually parse WAV files instead of sending the whole file to the sound card.

Comment on attachment 233669
Do proper WAV file parsing, v3

a=mconnor on behalf of drivers for checkin to the 1.8 branch

Not going to block release on this ... but we did approve the patch.

Checking in widget/src/gtk2/nsSound.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsSound.cpp,v <-- nsSound.cpp
new revision: 1.12.20.2; previous revision: 1.12.20.1
done

*** Bug 354743 has been marked as a duplicate of this bug. ***

*** Bug 377145 has been marked as a duplicate of this bug. ***

With Thunderbird 2.0.0.6 in Ubuntu 7.10 b1, the bug still there.

Sorry, that was for another bug.