Closed Bug 487504 Opened 15 years ago Closed 12 years ago

Audible changes to volume (and mute state) lag behind setting value

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- -

People

(Reporter: Dolske, Assigned: kinetik)

References

Details

Attachments

(2 files)

Patch: Win32 fix
9.02 KB, patch
kinetik
: review-
Details | Diff | Splinter Review
Chrome/Firefox audio fade in/out comparison
357.96 KB, application/octet-stream
Details 
Bug 475317 adds a volume-level slider, and I noticed that audio level I hear lags behind the value set by the controls. I'd guess it's a few hundred milliseconds. Fast enough that it's < 1 second, but slow enough that it feels wrong. Clicking mute/unmute exhibits the same behavior.

The mute case is interesting, because the icon changes immediately. That's driven by the volumechange event (not the actual click). So the video backend is immediately sending out the event, even when the level heard by the user hasn't yet changed.

[Thought this was already filed, but I was thinking of bug 469266.]
Flags: blocking1.9.1?
I suspect it's the already buffered audio data that has been queued at the old volume.
I'm reluctant to mark this blocking because I'm not sure how we can fix it.
The way to fix it is to use platform API's to control the volume rather than adjusting the sample values directly.
Hmm, for some reason I thought we had to adjust the sample values directly...
Because we didn't have independent control over the volume of each stream, or something?
We did it this way because libsydneyaudio didn't expose the api's fully for volume control across all platforms (It still doesn't iirc). And on Linux it controlled the master volume which was problematic as you mention in comment 5. I don't know the capabilities of all platforms as to whether per stream control is possible in all cases. So we took the route of modifying the sample values.
OK, I'm still reluctant to mark this blocking if it depends on the existence of platform APIs that may not exist :-)
I fully agree :-)
I'll make it wanted. It would be nice if we used platform volume control APIs where they exist, and/or turned down the audio buffer if we think we can without skipping, but this doesn't really need to block IMHO.
Flags: blocking1.9.1? → wanted1.9.1+
i can repro this too on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090413 Shiretoko/3.5b4pre.   The mute/unmute lags about .5 seconds off
This is trivial to fix on Mac.  The appropriate code already exists in sa_stream_set_volume_abs, which works per-stream.  All we'd need to do is call it from nsAudioStream::SetVolume and leave mVolume set at 1.0.

On Windows, some code exists in sa_stream_change_write_volume which could be used to implement sa_stream_set_volume_abs.  Unfortunately (despite being passed a per-stream handle) it affects the master "Wave Out" volume on pre-Vista, and the per-app "Wave Out" on Vista and later systems.  There's no way to control per-stream volume using the waveOut API.  We'd need to switch to using a more modern sound API to solve this.

sa_stream_set_volume_abs is implemented on Linux (in all of the backends), but it has similar limitations to the existing Windows code, except there's no obvious "modern API" to switch to to solve this, either (though I suppose that might eventually be PulseAudio once it's widespread enough).

I don't think we can afford to buffer less (unless we can make it adaptive), since we already have bug reports about audio glitches on slow or heavily loaded machines.

A more complicated way to fix this would be to immediately suspend audio playback when a volume change occurs, get the current playback position (ideally this would be sample accurate to be glitch-free, but maybe a "fairly" accurate value is fine if we can bear small glitches), purge the buffered data, remix it at the new volume, begin resubmitting the data, and resume the stream.  I don't think the existing sydneyaudio APIs are sufficient to permit this, though.
Or we could push the software volume control down into sydneyaudio, which would then perform volume adjustment on the samples just before passing them to the OS.  This would be easy to do in the existing Mac and Windows code, but the ALSA code would need some restructuring.
I noticed this in a test page I had open for Shiretoko on OSX.

I'd hit play on the <video> - the sound blasted out rather loudly.  I quickly paused, then toggled mute.  On hitting play again, the sound blasted out once more for a moment.
The delay is at least a full second here on a Mac build from 04/11, FWIW.
When viewing air mozilla on Windows the delay can be 3+ seconds hitting the Mute button. It's really annoying!
blocking2.0: --- → ?
blocking2.0: ? → beta1+
Per comment 11, it's not easy to fix this properly on all platforms, so it's unlikely to be fixed for beta1.
Assuming this affects WebM video as well as ogg, this is going to be a pretty major black eye for us: having audio continue for seconds after you close a youtube tab or hit the mute button is very noticeable. What can we accomplish in the beta timeframe (and for 3.6.x, if we decide to backport WebM)?
Does audio actually continue after you close a tab? If so, that's a different bug.
Yes, at least in air mozilla it has. I can file that separately.
Audio continuing after a tab closes is a different bug--we can stop audio immediately by destroying the audio stream.
Bumping this to beta2 per beta re-triage with dbaron.  If this should indeed block beta1, please re-nom.  Is this still looking like it's not going to make beta1, regardless?
blocking2.0: beta1+ → beta2+
Was the bug for the 'close tab' issue in comment 19 filed?
Moving this to betaN to eventually be triaged into a specific beta milestone.
blocking2.0: beta2+ → betaN+
Depends on: 586535
Assignee: kinetik → nobody
The bug exists in Windows 7 too.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101007 Firefox/4.0b8pre
This has been fixed on Mac, still needs fixing on Linux and Windows.
OS: Mac OS X → All
Attached patch Patch: Win32 fix — — Splinter Review 
Uses WaveAPI to set volume on Windows. This is immediately setting the volume for me on native Windows 7 and in my WinXP VM.

Linux still needs to be fixed separately.
Assignee: nobody → chris
Attachment #485225 - Flags: review?(kinetik)
Comment on attachment 485225 [details] [diff] [review]
Patch: Win32 fix

This isn't per-stream volume control, it's either global (pre-Vista) or per-application (Vista onwards).  See comment 11 for a few more details.
Attachment #485225 - Flags: review?(kinetik) → review-
This is already fixed on Mac, but to fix on other platforms we'd need to rewrite the libsydney audio backend, so we shouldn't block 2.0 on this; it's a large and risky change to be making this late in the cycle.
Assignee: chris → nobody
Sucks, but I guess you're right.
blocking2.0: betaN+ → -
Depends on: cubeb
With Firefox 9, the lag of mute/unmute/volume change is about 1 second on Solaris.
With Firefox 10, the lag is about 9 seconds on Solaris.
There is a regression.

Similarly, pause is almost immediately with Firefox 9.
With Firefox 10, there is a 1-2 seconds lag with pause.
Thanks Ginn. Does this happen for you on Nightly (Firefox 12)? If so, would you be willing to track down a specific regression range using the Mozregression tool?

http://harthur.github.com/mozregression/
Thanks, the regression doesn't happen on Linux.
I found it is caused by Bug 669556, apply the same patch to sydney_audio_sunaudio.c will solve it.
Marking this as a regression as per comment 34.
Keywords: regression
Anthony, I think I should file a separate bug for the regression on Solaris.
The lag is not regressed on tier 1 platforms.
Keywords: regression
(In reply to Ginn Chen from comment #36)
> Anthony, I think I should file a separate bug for the regression on Solaris.
> The lag is not regressed on tier 1 platforms.

I think I agree. Please do so and link it back here. Thanks.
What's the status of this bug? Now that youtube is rolling out html5 videos for users not having flash installed , i think this regression should be fixed...
It's going to be fixed by bug 623444.
Seems like its fixed now with latest nightly. Any confirms ?
Yes, this has been fixed on Windows by the landing of patches for bug 623444.  It's not yet fixed on Linux, so I'll leave the bug open until those patches land.
Fixed by bug 623444.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee: nobody → kinetik
This is still buggy for me. Please see this fiddle: http://jsfiddle.net/2eG6s/ It belongs to the following question on Stack Overflow: http://stackoverflow.com/q/17408967/1392379

While the audio fades very smooth in for example Chrome, it's still very choppy in Firefox, not as bad as in previous versions, but still anything but smooth. I'm testing this on Windows 7 x64 with an EMU-0404 PCIe, using Chrome 27.0.1453.116m and Firefox 22.0 as well as 25.0a1.

The attachment demonstrates exactly what it sounds for me. The first sample is using Chrome, the second one is using Firefox, where you should clearly hear the choppy transition.
I can't repro on Linux/Mac, but I can repro on Windows, so this is backend specific.

I'll try with the new WASAPI backend, because I have the fear that the winmm buffer size is simply too bad to be able to have a smooth fade-in/fade-out effect using the volume attribute the way we do it (i.e. having a single possible value for the callback).
So, did you have any luck using WASAPI?
I haven't tried and I don't have a windows machine at the moment. I'll NEEDINFO myself as a reminder.
Flags: needinfo?(paul)
Flags: needinfo?(paul)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: