It was deprecated in bug 855570 (shipped in Firefox 22). Removing it would be nice: 48 files changed, 8 insertions(+), 1913 deletions(-) Two mochitests (test_wave_data_*) need to be updated to use Web Audio, the video controls will lose their "sample rate" and "channels" info (unless these values are exposed another way, perhaps via mozGetMetadata()?), and Gaia's dialer and emergency call code needs to be migrated to Web Audio.

Maybe the code path the ported test would exercise are already covered by the test suite in content/media/webaudio/test. I'll have a look. I know someone is working on porting the dialer (Etienne, CC-ed), not sure about porting the emergency call code.

(In reply to Paul Adenot (:padenot) from comment #1) > Maybe the code path the ported test would exercise are already covered by > the test suite in content/media/webaudio/test. I'll have a look. Couldn't see anything obvious, so I wrote a test, but either my test code is broken or I've found a new bug. I've attached the test; I get 14 samples returned for each instead of 3, and the sample values are wonky.

Slight updated patch, with the above test (expecting it to fail for now) and the video controls fixed: https://tbpl.mozilla.org/?tree=Try&rev=89dbb6214a94

(In reply to Paul Adenot (:padenot) from comment #1) > Maybe the code path the ported test would exercise are already covered by > the test suite in content/media/webaudio/test. I'll have a look. > > I know someone is working on porting the dialer (Etienne, CC-ed), not sure > about porting the emergency call code. Very similar code, I'll probably do both.

I think removing the code whole-sale is a mistake initially. My plan was to hide the API points behind a pref which is set to false is non-release builds initially and let that bake for one or two cycles, and if nobody screams, flip the pref to false in all release builds, and remove the code shortly afterwards. Doing that will make it easier for us to back off one release or two if we find out that this breaks some web content out there in a bad way, and seems like better risk mitigation to me.

CCing some Emscripten folks to keep them in the loop.

Emscripten currently has two audio API implementations available for C/C++ code: - The OpenAL library utilizes Web Audio API as backend: https://github.com/kripken/emscripten/blob/master/src/library_openal.js - The SDL library primarily utilizes the Audio Data API if available, and secondarily Web Audio API if that is present. The detection is made dynamically, so if/when Audio Data API is killed, existing Emscripten-built applications should gracefully start falling back to Web Audio API, see https://github.com/kripken/emscripten/blob/master/src/library_sdl.js#L1449 The reason for currently favoring Audio Data API if available over Web Audio API is that we have not been able to implement a smooth continous audio playback path for Web Audio API, the following bug is related https://bugzilla.mozilla.org/show_bug.cgi?id=913854 . The issue is most notable in non-48kHz/32-bit float audio sources. I am ok with Audio Data API going down, and feeling hopeful that the above bug can be resolved without changes to the spec(?), the discussion there makes it feel like the problem would be at implementation level and not the spec level. Hopefully there will not be a time gap between killing Audio Data API, and the above bug being resolved, or the audio experience in running existing SDL audio -based applications like http://clb.demon.fi/html5scummvm/ could be impacted.

(In reply to comment #8) > Emscripten currently has two audio API implementations available for C/C++ > code: > - The OpenAL library utilizes Web Audio API as backend: > https://github.com/kripken/emscripten/blob/master/src/library_openal.js > - The SDL library primarily utilizes the Audio Data API if available, and > secondarily Web Audio API if that is present. The detection is made > dynamically, so if/when Audio Data API is killed, existing Emscripten-built > applications should gracefully start falling back to Web Audio API, see > https://github.com/kripken/emscripten/blob/master/src/library_sdl.js#L1449 That's great! > The reason for currently favoring Audio Data API if available over Web Audio > API is that we have not been able to implement a smooth continous audio > playback path for Web Audio API, the following bug is related > https://bugzilla.mozilla.org/show_bug.cgi?id=913854 . The issue is most notable > in non-48kHz/32-bit float audio sources. Yes, that use case is not really supported by Web Audio. > I am ok with Audio Data API going down, and feeling hopeful that the above bug > can be resolved without changes to the spec(?) I don't think that it can unfortunately, since it's not a bug in our implementation per se. > the discussion there makes it > feel like the problem would be at implementation level and not the spec level. The resampling algorithm does make a difference, and that is not mandated in the spec, but lower quality resampling algorithms may "hide" this problem in some implementations. > Hopefully there will not be a time gap between killing Audio Data API, and the > above bug being resolved, or the audio experience in running existing SDL audio > -based applications like http://clb.demon.fi/html5scummvm/ could be impacted. How many Emscripten games are affected by this? Off the top of my head I remember scumvm and perhaps one other game I can't remember the name of right now. In my experience this has been a very rare use case for Emscripten games, and in most cases should be possible to work around by just playing the background audio using an <audio> element.

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6) > I think removing the code whole-sale is a mistake initially. My plan was to > hide the API points behind a pref which is set to false is non-release > builds initially and let that bake for one or two cycles, and if nobody > screams, flip the pref to false in all release builds, and remove the code > shortly afterwards. Okay, we can use the existing media.audio_data.enabled for that. (See http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLAudioElement.cpp#24). It'll need to be extended slightly to throw for mozChannels/mozSampleRate too.

(In reply to Matthew Gregan [:kinetik] from comment #3) > Couldn't see anything obvious, so I wrote a test, but either my test code is > broken or I've found a new bug. I've attached the test; I get 14 samples > returned for each instead of 3, and the sample values are wonky. Turns out it's just resampling. The test media is 3 samples at 11025Hz, the AudioContext is running at 48000Hz, 48/11 * 3 ends up with 14 samples. Simple fix was to rewrite the test media's sample rate with whatever the AudioContext.sampleRate reports to avoid resampling. Try push for the pref-off approach: https://tbpl.mozilla.org/?tree=Try&rev=717c1f655cef

Removal patch, includes the fixed test (which I'll move to the pref-off patch before review/landing): https://tbpl.mozilla.org/?tree=Try&rev=744d37fb5bfe

(In reply to Matthew Gregan [:kinetik] from comment #10) > Okay, we can use the existing media.audio_data.enabled for that. (See > http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/ > HTMLAudioElement.cpp#24). It'll need to be extended slightly to throw for > mozChannels/mozSampleRate too. It would be sufficient to add [Pref="media.audio_data.enabled"] in .webidl files.

(In reply to Matthew Gregan [:kinetik] from comment #11) > Try push for the pref-off approach: > https://tbpl.mozilla.org/?tree=Try&rev=717c1f655cef Er, with the pref actually off (missed it in all.js): https://tbpl.mozilla.org/?tree=Try&rev=5300a9d7a335 (In reply to Masatoshi Kimura [:emk] from comment #13) > It would be sufficient to add [Pref="media.audio_data.enabled"] in .webidl > files. Thanks, that's what I did.

Comment on attachment 819513 [details] [diff] [review] bug927245_pref_off_v0.patch Review of attachment 819513 [details] [diff] [review]: ----------------------------------------------------------------- Looks mostly good, except that I don't agree that we should remove the tests using the Audio Data APIs. Instead, we should turn this pref on for those mochitests I think, and get rid of the tests when we actually remove the code that they're testing. ::: content/media/webaudio/test/test_waveDecoder.html @@ +1,1 @@ > +<!DOCTYPE HTML> Hmm, is there something in this test which <http://mxr.mozilla.org/mozilla-central/source/content/media/webaudio/test/test_mediaDecoding.html?force=1> doesn't cover?

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #9) > How many Emscripten games are affected by this? Off the top of my head I > remember scumvm and perhaps one other game I can't remember the name of > right now. In my experience this has been a very rare use case for > Emscripten games, and in most cases should be possible to work around by > just playing the background audio using an <audio> element. I'm not sure how many released emscripten sites there are that have been released, but this is not at all rare for Emscripten games - any C/C++ project that uses OpenAL or SDL audio is affected by this, since both of these backends push audio through circular buffers. All audio playback using SDL audio is affected, since SDL mixes its streams manually even if you do short audio clip playback in the code. Streaming audio in OpenAL side is affected. Manually writing a path that uses <audio> elements is a workaround, but naturally not feasible for all cases.

(In reply to comment #17) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #9) > > How many Emscripten games are affected by this? Off the top of my head I > > remember scumvm and perhaps one other game I can't remember the name of > > right now. In my experience this has been a very rare use case for > > Emscripten games, and in most cases should be possible to work around by > > just playing the background audio using an <audio> element. > > I'm not sure how many released emscripten sites there are that have been > released, but this is not at all rare for Emscripten games - any C/C++ project > that uses OpenAL or SDL audio is affected by this, since both of these backends > push audio through circular buffers. All audio playback using SDL audio is > affected, since SDL mixes its streams manually even if you do short audio clip > playback in the code. Streaming audio in OpenAL side is affected. Well, those projects are only affected "in theory", since not all games use those APIs for streaming audio tracks. > Manually writing a path that uses <audio> elements is a workaround, but > naturally not feasible for all cases. Yes, I understand that. Ideally we would enable Web Audio to handle this use case, but honestly very few people have asked for it so far.

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #16) > Looks mostly good, except that I don't agree that we should remove the tests > using the Audio Data APIs. Instead, we should turn this pref on for those > mochitests I think, and get rid of the tests when we actually remove the > code that they're testing. Okay, I've fixed the tests where necessary. > Hmm, is there something in this test which > <http://mxr.mozilla.org/mozilla-central/source/content/media/webaudio/test/ > test_mediaDecoding.html?force=1> doesn't cover? 8 and 16 bit sample formats without resampling.

Comment on attachment 820121 [details] [diff] [review] bug927245_pref_off_v1.patch Review of attachment 820121 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! ::: content/media/webaudio/test/mochitest.ini @@ +11,5 @@ > noaudio.webm > small-shot-expected.wav > small-shot-mono-expected.wav > small-shot.ogg > + ting-11k-1ch-8b.wav This file seems to be unused, and I can't find it in the patch.

(In reply to :Ehsan Akhgari (needinfo? me!) [Away 10/29-11/6] from comment #20) > > + ting-11k-1ch-8b.wav > > This file seems to be unused, and I can't find it in the patch. Gah, sorry, that's from an attempt to integrate the waveDecoder tests into mediaDecoding that ended up being too complicated. I'll remove it before landing.

With ting-11k-1ch-8b.wav removed. Carrying forward Ehsan's review. I have also removed the all.js change (that actually disables the API) from this version, I'm going to land it now to avoid having to rebase, then all we need to do once bug 927852 and bug 929089 have been fixed is land the pref change itself.

Landed (with a better commit message): https://hg.mozilla.org/integration/mozilla-inbound/rev/9a82f8dbb719

Actually pref off. Need Shumway and Gaia fixed before we can land this. Carrying forward Ehsan's review, since this is extracted from the previously reviewed patch.

And actually remove it. This can't land for a couple of cycles, and will almost certainly bitrot somewhat in the meantime, but it'll at least serve as a record of all the places that need to be touched to remove this.

Comment on attachment 820754 [details] [diff] [review] bug927245_pref_off_only_v0.patch https://hg.mozilla.org/integration/mozilla-inbound/rev/6f5a37dd7a4e

This will hit release in around three weeks when beta (28) becomes release. Once that happens, I plan to commit the code removal patch to nightly (31).

Rebased on top of current tree. Builds locally, try run started: https://tbpl.mozilla.org/?tree=Try&rev=897dc0a16b31 cajbir for the media bits, smaug for the dom bits.

(In reply to comment #31) > https://hg.mozilla.org/integration/mozilla-inbound/rev/1d5155bd974c \o/

Somehow I have missed this. Added to the docs: https://developer.mozilla.org/en-US/Firefox/Releases/28/Site_Compatibility https://developer.mozilla.org/en-US/Firefox/Releases/31/Site_Compatibility

And updated https://developer.mozilla.org/en-US/docs/Introducing_the_Audio_API_Extension