Closed Bug 536996 Opened 15 years ago Closed 2 years ago

nsISound is broken (Linux)

Categories

(Core :: Widget, defect)

All
Linux
defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: u254340, Unassigned)

References

Details

(Keywords: regression, Whiteboard: [3.6.x])

Attachments

(1 file)

31.29 KB, application/x-xpinstall
Details
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.
Attached file Testcase addon.
(I made it very fast, hope it will work for everyone)
Yves, does it work with 3.5.6 or is that a regression in 3.6b5?
On linux 3.5.6:
The beep() method does nothing
The play() works
The playEventSound() does not exist (new in ff 3.6)
This sounds like a regression if the play() method has changed behaviour and is not consistent cross-platform, CCing Beltzner
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Version: unspecified → 3.6 Branch
Since it is a regression in the 3.6 branch, asking for that bug to be a blocker
Flags: blocking-firefox3.6?
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. :/
Flags: blocking-firefox3.6? → blocking-firefox3.6+
I'm planning on workin on the regression date in a few hour.

Do i need to check 3.7 and 4.0 too ?
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).
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.
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
Given that range, I suspect the fix for bug 479929 caused this, commit is:

http://hg.mozilla.org/mozilla-central/rev/5ac18226837e
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.
I agree, sound.beep not working for me is probably only for me, i don't remember ever hearing it.
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.
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.
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.
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.
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?
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.
Component: General → Widget
Flags: blocking-firefox3.6+
Product: Firefox → Core
QA Contact: general → general
Hardware: x86 → All
Version: 3.6 Branch → Trunk
-> core/widget, transferred blocking flag to blocking1.9.2+
Flags: blocking1.9.2+
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.
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
As an aside from comment 19, filed bug 537195.
Assignee: nobody → joshmoz
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.
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 ).)
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.
(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.
(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.
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.
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)."
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.)
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?
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.
Flags: blocking1.9.2+ → blocking1.9.2-
Whiteboard: [3.6.x]
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...
Flags: blocking1.9.2-
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)
Depends on: 520417
>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...
Assignee: joshmoz → nobody
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.
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
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.
Blocks: 576365
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.
Depends on: 579877
Here, in Gnome and with ALSA (no PulseAudio), it's fixed with FF 3.6.11 :)
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.
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); 
--------------------------
Keywords: relnote
Severity: normal → S3

The severity field for this bug is relatively low, S3. However, the bug has 16 votes.
:spohl, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Flags: needinfo?(spohl.mozilla.bugs)

This appears to be working now. Tentatively closing.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: