Last Comment Bug 536996 - nsISound is broken (Linux)
: nsISound is broken (Linux)
Status: NEW
[3.6.x]
: regression
Product: Core
Classification: Components
Component: Widget (show other bugs)
: Trunk
: All Linux
: -- normal with 16 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on: 520417 579877
Blocks: 576365
  Show dependency treegraph
 
Reported: 2009-12-28 11:26 PST by Yves
Modified: 2014-10-18 13:36 PDT (History)
29 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase addon. (31.29 KB, application/x-xpinstall)
2009-12-28 11:29 PST, Yves
no flags Details

Description Yves 2009-12-28 11:26:52 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b6pre) Gecko/20091225 Namoroka/3.6b6pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b6pre) Gecko/20091225 Namoroka/3.6b6pre

The nsISound interface is broken on linux.




https://developer.mozilla.org/en/nsISound

Reproducible: Always

Steps to Reproduce:
1.Install my test extention.
2.Go to it's preference pane.
3.Click the three buttons
Actual Results:  
On windows, the 3 sounds are played.
On linux:
The beep() method does nothing
The play() method raises an exception
The playEventSound() method does nothing

Expected Results:  
the 3 sounds are played.
Comment 1 Yves 2009-12-28 11:29:21 PST
Created attachment 419331 [details]
Testcase addon.

(I made it very fast, hope it will work for everyone)
Comment 2 Pascal Chevrel:pascalc(PTO until Sept 2) 2009-12-28 12:04:47 PST
Yves, does it work with 3.5.6 or is that a regression in 3.6b5?
Comment 3 Yves 2009-12-28 12:13:26 PST
On linux 3.5.6:
The beep() method does nothing
The play() works
The playEventSound() does not exist (new in ff 3.6)
Comment 4 Pascal Chevrel:pascalc(PTO until Sept 2) 2009-12-28 12:17:26 PST
This sounds like a regression if the play() method has changed behaviour and is not consistent cross-platform, CCing Beltzner
Comment 5 Dave Garrett 2009-12-28 13:21:46 PST
I can reproduce using the attached testcase here under Kubuntu 8.04. Latest 3.5.8pre plays the sound and latest 3.6b6pre throws NS_ERROR_FAILURE.
Comment 6 Pascal Chevrel:pascalc(PTO until Sept 2) 2009-12-28 13:32:53 PST
Since it is a regression in the 3.6 branch, asking for that bug to be a blocker
Comment 7 Mike Beltzner [:beltzner, not reading bugmail] 2009-12-29 10:08:31 PST
Three questions:
 - What regressed this? 
 - Who is working on a regression range?

Blocking for now, but it feels strange to me that nobody noticed this until just recently. :/
Comment 8 Yves 2009-12-29 10:10:43 PST
I'm planning on workin on the regression date in a few hour.

Do i need to check 3.7 and 4.0 too ?
Comment 9 Mike Beltzner [:beltzner, not reading bugmail] 2009-12-29 11:15:28 PST
It will probably be easier to isolate on 1.9.2, and from that we'll be able to determine a single bug that caused it on trunk (3.7).
Comment 10 Yves 2009-12-29 11:27:22 PST
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090225 Minefield/3.2a1pre WORKS


Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090226 Minefield/3.2a1pre Throws exception.


(it is the first time I do this kind of things so don't hésitate to doublecheck my result.
Comment 11 Dave Garrett 2009-12-29 11:41:41 PST
Tested builds in comment 10 and I get the same result.
The pushlog for that range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8eba35e62d92&tochange=a979ac23e17e
Comment 12 Josh Aas 2009-12-29 12:37:42 PST
Given that range, I suspect the fix for bug 479929 caused this, commit is:

http://hg.mozilla.org/mozilla-central/rev/5ac18226837e
Comment 13 Josh Aas 2009-12-29 13:06:14 PST
Using Ubuntu 9.10 in a VirtualBox VM and the standard Firefox 3.5.6 I get the following results with the test extension:

1. [works] sound.play
2. [works] sound.beep
3. [fails] sound.playEventSound

This is inconsistent with comment 3, which says that sound.beep does nothing. WFM.

I think this is the expected behavior so if we have any issues they are probably 1.9.2 branch and higher. I'll test the 1.9.2 branch and a backout of bug 479929 when my build finishes. Slow because I'm doing this in a VM.
Comment 14 Yves 2009-12-29 13:20:52 PST
I agree, sound.beep not working for me is probably only for me, i don't remember ever hearing it.
Comment 15 Josh Aas 2009-12-29 14:10:27 PST
Using current 1.9.3 (trunk) code I do not have any problems - all sound tests in the test extension WFM.

I'm using a trunk debug build on Ubuntu 9.10 in a VirtualBox VM with a Mac OS X 10.6 host.

I'll test 1.9.2 builds next.
Comment 16 Josh Aas 2009-12-29 14:14:15 PST
I have no problem with any of the sound tests on the latest 1.9.2 nightly build either. Same machine/OS setup as comment 15.

I'm not going to work on this any more today because I can't repro.
Comment 17 Yves 2009-12-29 14:21:37 PST
I am using kubutu 8.04 like Dave Garrett.

I checked, i have the bug in

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091229 Minefield/3.7a1pre

too.
Comment 18 Dave Garrett 2009-12-29 14:23:05 PST
Beep works fine for me here under both 3.5 and 3.6, assuming I unmute the awful thing in my mixer. Yves, you may have it muted or disabled on your end.

I also don't get playEventSound() here, but I think I just can't figure out where to enable that with my OS right now. Only part of this bug I'm sure I can reproduce is the exception on play() under 3.6/3.7.
Comment 19 Dave Garrett 2009-12-29 14:50:44 PST
Interesting. I just tested under another computer running Kubuntu 9.04 (sorry, no 9.10 on hand) and it throws the exception under Firefox 3.0, 3.5, and 3.6. This is beginning to sound like a KDE or Kubuntu specific issue based on this and comment 15. Anyone have Kubuntu 9.10 installed somewhere to test on, or should I install a VM?
Comment 20 Josh Aas 2009-12-29 16:12:33 PST
I can install Kubuntu 9.10 in a VM tonight and test.

I suspect we don't need to block on this, especially if it turns out not to be broken on Kubuntu 9.10, but we should stay on top of it and fix it ASAP if we can.
Comment 21 Josh Aas 2009-12-29 16:14:45 PST
-> core/widget, transferred blocking flag to blocking1.9.2+
Comment 22 Dave Garrett 2009-12-29 16:34:58 PST
Ok, I have Kubuntu 9.10 running in a VM now. The exception for 3.0 and 3.5 I noted in comment 19 happens there too, but it's an unrelated issue. Apparently I need to install esound. Once that's installed the play() test works under 3.5 and throws under 3.6.
Comment 23 aja+bugzilla 2009-12-29 16:41:52 PST
Just confirming comment 16, all three are OK on Ubuntu 9.10 with
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b6pre) Gecko/20091229 Namoroka/3.6b6pre ID:20091229033806
running native...i.e. no intervening VM
Comment 24 Dave Garrett 2009-12-29 19:27:31 PST
As an aside from comment 19, filed bug 537195.
Comment 25 Olaf Piesche 2009-12-30 15:23:43 PST
Debugged this with Josh for a while, and here's what we found out:

On Kubuntu 9.10 with a nightly build, all three buttons fail to work - the first one (play) raises an exception, the other two seem to do nothing at all. I'm not sure how the patch Josh posted can be causing this, but on 3.5.6 the sound.play button works fine; after looking at the esd code, the patch seems to be valid - esd_open_sound does return <0 in case of an error, so 0 seems to be a valid socket handle. So, without actually being able to step through the code, that patch *appears* to be doing the right thing.

With the nightly build running in a Kubuntu 9.10 VM, it seems to fail to spawn esd properly, which looks like the cause of the exception. Manually starting esd before running the firefox nightly build makes the sound.play button work properly. Again, looking at the esd code, there's a timeout value in /etc/esound/esd.conf that defaults to 100ms, and esd_open_sound waits for the specified timeout duration. If it can't connect to the daemon within that time, it'll return a negative value.  Increasing the timeout in the config file on my 9.10 VM didn't fix the problem, so this looks to me like esd simply not being spawned properly under Kubuntu/Ubuntu 9.10 when using the nightly build.
Comment 26 Yves 2009-12-30 15:31:23 PST
On my Kubuntu 8.04 , Manually starting esd before (or while) running  the firefox make sound.play work correctly! (the two other still does nothing)

(and thanks for the workaround ( I will now hear my new mails :D ).)
Comment 27 Josh Aas 2009-12-30 15:49:33 PST
Olaf and I have done some more debugging...

Backing out the patch from bug 479929 fixes sound.play().

sound.beep() uses ::gdk_beep() and doesn't work at all on kubuntu, neither does the "beep" command line util, so this is probably a kubuntu bug.

sound.playEventSound does not work with or without the patch from bug 479929 and we don't know what is going on there yet.
Comment 28 Dave Garrett 2009-12-30 15:54:03 PST
(In reply to comment #27)
> sound.beep() uses ::gdk_beep() and doesn't work at all on kubuntu, neither does
> the "beep" command line util, so this is probably a kubuntu bug.

See comment 18. It works fine if you re-enable it. All I had to do here was open KMix and unmute and turn up the volume on the PC Speaker.

> sound.playEventSound does not work with or without the patch from bug 479929
> and we don't know what is going on there yet.

Yeah, as I said above I think that's just that it'd need GTK/GNOME sounds configured and they aren't under KDE. No clue where to set them. Could just not work at all here, though.
Comment 29 Dave Garrett 2009-12-30 16:07:46 PST
(In reply to comment #28)
> See comment 18. It works fine if you re-enable it. All I had to do here was
> open KMix and unmute and turn up the volume on the PC Speaker.

Caveat: Works fine for me on my native installs of 8.04 and 9.04. I can't get it to work in the 9.10 VM. I think I just have no virtualized PC speaker.
Comment 30 Olaf Piesche 2009-12-30 17:05:42 PST
As for the problem with sound.play(), my take on it is that it works without the patch, because in that case the code ignores a return value of -1 from esd_open_sound. That way, the library does not end up being unloaded (and indeed, it shows up in the shared library list in gdb in the unpatched build), and by the time we play the sound, the esd daemon has spawned and is usable.
With the patch, the -1 return value is not ignored, the lib is unloaded, and playing the sound doesn't work. 

So, despite the fact that the patch does in fact do the right thing (esd_open_sound does return a negative value on error, and 0 is a valid handle), it breaks it because it doesn't ignore the fact that esd doesn't spawn quickly enough. This also explains why manually starting esd in the patched build makes the problem go away.
Comment 31 Karl Tomlinson (:karlt) 2009-12-30 21:45:53 PST
bug 469635 comment 29:

"I'm not sure why we call esd_open_sound anyway--the esdref we get back is
never
used again (except to close ESD again), and the message about it being needed
for streams but not playing sounds is curious.

... we'll eventually call
esd_play_stream (which calls esd_open_sound internally)."
Comment 32 Karl Tomlinson (:karlt) 2009-12-30 21:55:49 PST
If this is only an issue for KDE users, then it's not clear to me that we should block on this.  How many KDE users would have esound installed?
(IIUC KDE users without esound won't get sound whatever we do here.)
Comment 33 Olaf Piesche 2009-12-30 23:42:42 PST
I'm poking a bit in the dark here, but I think the esd_open_sound is mainly used to make sure esd is actually running so we can play sound through it.
This is a bit of a philosophical question, and one that's speculative at best since I don't know the code well, but maybe, shouldn't this all be using gstreamer to begin with?
Comment 34 Mike Beltzner [:beltzner, not reading bugmail] 2009-12-31 14:38:59 PST
If this is a KDE only issue I agree that we shouldn't block, though we'll want to take a fix on a security and stability branch.
Comment 35 Olaf Piesche 2009-12-31 14:47:45 PST
Isn't this really three separate issues? sound.play is not working because of the esd weirdness, sound.beep likely because of the PC speaker volume and/or missing Gnome sound configs, (so it doesn't seem to be a problem on the code side) - and sound.playEventSound appears to be using libcanberra exclusively, so that looks like something entirely different from the other two...
Comment 36 Wolfgang Rosenauer [:wolfiR] 2010-01-02 01:29:30 PST
What I found while testing on openSUSE 11.2:
- beep is working nowhere for me (not sure why)
- play() is working if libesd and ( esound-daemon or pulseaudio with esound 
  compat interface) are available (and running).
  esd used to autospawn esound-daemon in the past which is not the case anymore 
  apparently (in openSUSE)
- playEventSound() is more or less Gnome only by using canberra as already 
  suggested

So basically everyone not using Gnome and pulseaudio is very likely not getting any sound out of Firefox (excluding media tags) by default which is a bit annoying.
Also nothing to block here but the sound mess in Firefox should probably be cleaned up given that 
- parts are relying on esound (which is pretty much deprecated)
- parts (media support) are using ALSA directly
- parts are using Gnome only interfaces/canberra  (what probably can make sense 
  in that case for sound themes)
Comment 37 Olaf Piesche 2010-01-03 17:49:50 PST
>Also nothing to block here but the sound mess in Firefox should probably be
cleaned up

Agreed, however it doesn't seem like a my-hair-is-on-fire-fix-it-now issue.
Not to sound like a broken record, but since gstreamer is part of the LSB, should that be the API of choice, or should Firefox rely on something else? What about mobile platforms and BSD? 
I'd strongly recommend relying on a single API as much as possible and only use others where absolutely necessary for specific platforms. It means rewriting pretty much the entirety of this code, but it would likely save a lot of headaches later on. Just my $0.02 :)

Are these sorts of things handled through different abstraction layers on Windows and MacOS, by the way? If so, it may be worth unifying the different interfaces...
Comment 38 Hartmut Figge 2010-01-11 07:36:03 PST
For me on a current Gentoo SM 2.1 doesn't play a sound when selecting Preview from Notifications for mail. I have even tested some older TB 3.1 and a recent TB 3.2 which fail also.

Backing out the aforementioned patch from my SM 2.1 fixes it. Well, in some way. *g* The sound is somewhat distorted but a least i am now notified again when mail arrives. :)

No distortion if i open the .wav in the browser part. I am only compiling SM 2.1, so i cannot test if the backout also fixes the problem for TB 3.2.
Comment 39 Hartmut Figge 2010-01-13 11:55:01 PST
I was just reminded of this bug by a mail caused by a new CC. Well, the distortion is gone with my current build. :)

Build identifier: Mozilla/5.0 (X11; U; Linux i686; en; rv:1.9.3a1pre) Gecko/20100113 Mnenhy/0.8.0pre6.h1 SeaMonkey/2.1a1pre
Comment 40 Robert Kaiser 2010-04-03 05:37:45 PDT
IMHO, we should use the same backend as for the media tags everywhere in our code, as comment #36 says, we are currently using 3 different backends, which is just a pain. The media tags work fine for everybody, I think, so we should converge to using that backend everywhere. At least that's my thinking.
Comment 41 Wolfgang Rosenauer [:wolfiR] 2010-07-19 04:51:38 PDT
FWIW, I've debugged the thing with nsISound.play and the "fix" for bug 479929 really broke it. I just needed some time to find out why (not knowing much about esd). I've attached a patch there to backout the change and recover the important comment.
Comment 42 Tsu Jan 2010-10-26 07:37:49 PDT
Here, in Gnome and with ALSA (no PulseAudio), it's fixed with FF 3.6.11 :)
Comment 43 Wolfgang Rosenauer [:wolfiR] 2010-10-26 07:41:42 PDT
Yes, I'm actually wondering if there is anything in this bug report which is not fixed now?
Yes, the nsISound redesign but that's another story I think.
Comment 44 Sasha (retired) 2011-11-13 06:42:30 PST
Seems to be broken on XP as well. Cannot generate a simple beep() with nsISound. The Audio API does work. Paste the following into error console to compare:

Does not work:
--------------------------
Components.classes["@mozilla.org/sound;1"].createInstance(Components.interfaces.nsISound).beep();
--------------------------
Does work:
--------------------------
var o = new Audio();   
c=10000;     
o.mozSetup(1, c);           
var s = new Float32Array(c);                  
for (var i = 0; i < (c/10) ; i++) {              
	s[i] = Math.sin( i / (2* Math.PI * 440/44100));          
}                  
o.mozWriteAudio(s); 
--------------------------

Note You need to log in before you can comment on or make changes to this bug.