Closed
Bug 855990
Opened 12 years ago
Closed 12 years ago
Implement AnalyserNode
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
Tracking | Status | |
---|---|---|
firefox22 | - | --- |
People
(Reporter: fredbezies, Assigned: ehsan.akhgari)
References
Details
(Keywords: dev-doc-complete, regression)
Attachments
(7 files, 3 obsolete files)
324.17 KB,
text/x-log
|
Details | |
3.06 MB,
application/x-zip
|
Details | |
14.18 KB,
patch
|
bzbarsky
:
review+
ehsan.akhgari
:
checkin+
|
Details | Diff | Splinter Review |
6.26 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
7.24 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
50.16 KB,
patch
|
Details | Diff | Splinter Review | |
2.81 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:22.0) Gecko/20130329 Firefox/22.0 Build ID: 20130329083344 Steps to reproduce: Logged into my google account. Actual results: After I logged in on my google account, I wanted to manage my musical collection. But, it doesn't display anything. Progress bar is not display, and page remains empty. Expected results: Displaying all my music. Looking at error console, I saw a lot of warning starting with "Warning: Unknown pseudo-class or pseudo-element '-webkit-scrollbar-track'. Ruleset ignored due to bad selector." I don't know for how much display had been broken.
Reporter | ||
Comment 1•12 years ago
|
||
Forget to add : I can see that on both homemade builds and "official" nightlies. My last homemade build is based on http://hg.mozilla.org/mozilla-central/rev/8693d1d4c86d
Reporter | ||
Comment 2•12 years ago
|
||
After doing some research, using linux 64 bits builds, I found the last working build and the first broken one. 17 march : http://hg.mozilla.org/mozilla-central/rev/0b052daa913c : ok 18 march : http://hg.mozilla.org/mozilla-central/rev/b03bb3ce8cee : broken
Comment 3•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b052daa913c&tochange=b03bb3ce8cee I don't see any style system changes in that range. There's a lot of DOM changes though. I think the "Unknown pseudo-class or pseudo-element" is unrelated to the problem. Do you see those warnings also in the build that works?
Updated•12 years ago
|
Flags: needinfo?(fredbezies)
Keywords: regression
![]() |
||
Comment 4•12 years ago
|
||
This needs steps to reproduce, ideally..... ones that someone who has never done any music stuff with a Google account could follow.
Reporter | ||
Comment 5•12 years ago
|
||
(In reply to Mats Palmgren [:mats] from comment #3) > http://hg.mozilla.org/mozilla-central/ > pushloghtml?fromchange=0b052daa913c&tochange=b03bb3ce8cee > > I don't see any style system changes in that range. There's a lot of DOM > changes though. > > I think the "Unknown pseudo-class or pseudo-element" is unrelated to the > problem. Do you see those warnings also in the build that works? Same output indeed :( I will look at error console with a newer build asap and see if there is anything else.
Flags: needinfo?(fredbezies)
Reporter | ||
Comment 6•12 years ago
|
||
By the way, I only get css error in Error console, like : Warning: Expected color but found 'top'. Error in parsing value for 'background-image'. Declaration dropped. Source File: https://plus.google.com/_/scs/apps-static/_/ss/k=oz.sbw.1h295td2rco7g.L.F4.O/am=AAACAM5BgAAgJAAAgMQh5n4BAAAAAwg/rs=AItRSTMOBrsfZ0J-6qL9aoTRz60C_dklOg Or Timestamp: 29/03/2013 17:29:42 Warning: Unknown property '-moz-box-shadow'. Declaration dropped. Source File: https://plus.google.com/_/scs/apps-static/_/ss/k=oz.sbw.1h295td2rco7g.L.F4.O/am=AAACAM5BgAAgJAAAgMQh5n4BAAAAAwg/rs=AItRSTMOBrsfZ0J-6qL9aoTRz60C_dklOg A lot of declaration dropped also. Any idea ? No error, only warning related to css file :(
![]() |
||
Comment 7•12 years ago
|
||
See comment 4? Seriously, it's hard to say anything here without having some idea of what the page is doing.
Flags: needinfo?(fredbezies)
Reporter | ||
Comment 8•12 years ago
|
||
I saw it. So without anyone with a google music account, this bug will be blocked.
Flags: needinfo?(fredbezies)
![]() |
||
Comment 9•12 years ago
|
||
Hmm. Well, if you save the page as "web page, complete", does it happen to work locally? If not, are you willing to bisect on tinderbox builds?
Comment 10•12 years ago
|
||
DOM seems more likely than Style System given the regression range (comment 3).
Component: Style System (CSS) → DOM
Keywords: testcase-wanted
Keywords: regressionwindow-wanted
Reporter | ||
Comment 11•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #9) > Hmm. Well, if you save the page as "web page, complete", does it happen to > work locally? Nothing. Same problem. > > If not, are you willing to bisect on tinderbox builds? Just tell me which tinderbox builds to get and how to bisect. I want to see this bug fixed.
![]() |
||
Comment 12•12 years ago
|
||
> Nothing. Same problem. What I meant was, does the saved version work in the old nightly but fail in the new one? If so, you could attach the saved version here and I could debug. ;) > Just tell me which tinderbox builds to get and how to bisect. Bisect the same way as you do on nightlies. As for which builds, http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux/ or http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64/ depending on which one you want, I assume. And then look for the relevant dates. Note that you might have to go back another day or so because things might have landed on inbound a good bit before the merge to mozilla-central. If you do have time to do that, that would be great!
Reporter | ||
Comment 13•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #12) > > Nothing. Same problem. > > What I meant was, does the saved version work in the old nightly but fail in > the new one? If so, you could attach the saved version here and I could > debug. ;) I will try to see if there is no personal datas and upload it. > > > Just tell me which tinderbox builds to get and how to bisect. > > Bisect the same way as you do on nightlies. > > As for which builds, > http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla- > inbound-linux/ or > http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla- > inbound-linux64/ depending on which one you want, I assume. And then look > for the relevant dates. Note that you might have to go back another day or > so because things might have landed on inbound a good bit before the merge > to mozilla-central. > > If you do have time to do that, that would be great! Will do this asap tonight (european time).
![]() |
||
Comment 14•12 years ago
|
||
> I will try to see if there is no personal datas and upload it.
Makes sense. Please feel free to email it to me directly or something if you're more comfortable with that.
Reporter | ||
Comment 15•12 years ago
|
||
First of all, builds :) Following publishing date and hours, looks like last working build was based on : http://hg.mozilla.org/integration/mozilla-inbound/rev/3a5f73a4f816 First broken one : http://hg.mozilla.org/integration/mozilla-inbound/rev/a8b4ad85b576 Will add asap datas, as it looks like there is no sensitive personal datas in it ;)
![]() |
||
Comment 17•12 years ago
|
||
> First of all, builds :) Thanks. The range there is http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3a5f73a4f816&tochange=a8b4ad85b576 If you take a nightly that's broken and set the "media.webaudio.enabled" preference to false, does that fix things?
![]() |
||
Updated•12 years ago
|
Attachment #731273 -
Attachment mime type: application/octet-stream → application/x-zip
Reporter | ||
Comment 18•12 years ago
|
||
Verified on my homemade build (10 hours old)... And you got it ! Deactivating webaudio fixes the bug !
Reporter | ||
Updated•12 years ago
|
Summary: Google Play Music doesn't load completely "Warning: Unknown pseudo-class or pseudo-element '-webkit-scrollbar-track'. Ruleset ignored due to bad selector." → Google Play Music doesn't load completely if webaudio is enabled.
![]() |
||
Comment 19•12 years ago
|
||
And thank you for the files. So listen_fr.js has this bit: this.sh = l; try { typeof webkitAudioContext == CH ? this.sh = new webkitAudioContext : typeof AudioContext == CH && (this.sh = new AudioContext) } catch (b) {} this.XA = l; this.sh && (this.XA = this.sh.createAnalyser()) Pretty sure we don't implement createAnalyzer, so this is going to throw... and pretty sure the site just ends up hiding the exception or something; that sort of thing is all over that script.
Reporter | ||
Comment 20•12 years ago
|
||
You're welcome for the files. So, besides deactivating webaudio (which is annoying), the only fix is to implement createAnalyzer ? Anyway, happy to know why google music wasn't working correctly !
![]() |
||
Updated•12 years ago
|
![]() |
||
Comment 21•12 years ago
|
||
> the only fix is to implement createAnalyzer ?
And that might well not be enough, depending on what else they're using.
Assignee | ||
Comment 22•12 years ago
|
||
I'll try to get to this soon.
Assignee: nobody → ehsan
Flags: needinfo?(ehsan)
Keywords: testcase-wanted
Summary: Google Play Music doesn't load completely if webaudio is enabled. → Implement DOM bindings for AnalyserNode
Assignee | ||
Comment 23•12 years ago
|
||
Looks like Google Music is using getFloatFrequencyData from AnalyserNode. I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=21446 to get this node properly spec'ed.
Assignee | ||
Updated•12 years ago
|
Summary: Implement DOM bindings for AnalyserNode → Implement AnalyserNode
Assignee | ||
Comment 25•12 years ago
|
||
Kiss FFT is a simple and cross platform FFT library which should be good enough for our FFT needs in Web Audio for now. This patch just imports the library and adds it to the build. Not a lot to review here... Note that Kiss FFT is also part of opus, but I'd rather have the library as a clear dependency which we can update later as opposed to hacking the libopus build system to let us use its Kiss FFT.
Attachment #731760 -
Flags: review?(paul)
![]() |
||
Comment 26•12 years ago
|
||
Comment on attachment 731753 [details] [diff] [review] Part 1: DOM bindings >+#include "AnalyserNode.h" "mozilla/dom/AnalyserNode.h", please. r=me, though you could make these attributes [Pure] if you wanted to, I think. Not sure whether it matters.
Attachment #731753 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 27•12 years ago
|
||
Comment on attachment 731753 [details] [diff] [review] Part 1: DOM bindings https://hg.mozilla.org/integration/mozilla-inbound/rev/81bad314826a With this patch, Google Music should in theory begin to work, just with no visualizations. I don't have an account yet so I can't test it myself...
Attachment #731753 -
Flags: checkin+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [leave open]
Assignee | ||
Comment 28•12 years ago
|
||
Comment on attachment 731753 [details] [diff] [review] Part 1: DOM bindings Backed out because of build bustage: http://hg.mozilla.org/integration/mozilla-inbound/rev/d03af4bf5681
Attachment #731753 -
Flags: checkin+
Assignee | ||
Comment 29•12 years ago
|
||
Comment on attachment 731753 [details] [diff] [review] Part 1: DOM bindings This is what I get for building patches on top of the ones that I have applied on my tree! Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/46e9d41acdeb
Attachment #731753 -
Flags: checkin+
Assignee | ||
Comment 30•12 years ago
|
||
This incorporates some changes that I had to make to the build system to make things work.
Attachment #731760 -
Attachment is obsolete: true
Attachment #731760 -
Flags: review?(paul)
Attachment #732179 -
Flags: review?(paul)
Assignee | ||
Comment 32•12 years ago
|
||
This mimics what WebKit does to implement the AnalyserNode, pending upcoming spec clarifications. Testcase/demo: http://people.mozilla.com/~eakhgari/webaudio/analyser.html
Attachment #732182 -
Flags: review?(paul)
Comment 33•12 years ago
|
||
Comment on attachment 732179 [details] [diff] [review] Part 2: Import Kiss FFT Review of attachment 732179 [details] [diff] [review]: ----------------------------------------------------------------- Hm, this is a build system patch, it should be reviewed by a build system person I guess. Bouncing off to glandium. Also, did you check that the licensing is okay? There should not be a problem considering this code is already present in Opus, and because Opus is already in the tree, but gerv could have a look. ::: media/kiss_fft/.hg_archival.txt @@ +1,4 @@ > +repo: d844c818f599aea64fe86745cdd2ef9b3d1910dc > +node: b354a59534b0a77c43c67deb1eb1bc39eb99b487 > +branch: default > +tag: v130 Are those .hg* files intended to be in the import? ::: media/kiss_fft/README.mozilla @@ +1,2 @@ > +This is the Kiss FFT library, version 1.3.0, retrieved from > +http://hivelocity.dl.sourceforge.net/project/kissfft/kissfft/v1_3_0/kiss_fft130.zip. Maybe you could document the process one should use to update the library? It seems to be a matter of simply doing `hg update`, but I'm not sure. Usually, we just have a script called `update.sh` that takes an upstream repo as $1, and pull in the new version, applying any patch that is needed in the process. Considering you Makefile.in references only one .c file, it could be cool to copy just the needed files. ::: media/kiss_fft/test/benchfftw.c @@ +32,5 @@ > + fprintf(stderr,"Datatype not available in FFTW\n" ); > + return 0; > +} > +#else > +int main(int argc,char ** argv) We probably don't want to compile that, and maybe we should not include it at all in the library version we have in-tree, that and all the files we don't use, such as benchmarks, python examples, matlab files, and other stuff. It might make the update process simpler, but the library is updated very rarely, so it should not matter.
Attachment #732179 -
Flags: review?(paul)
Attachment #732179 -
Flags: review?(mh+mozilla)
Attachment #732179 -
Flags: review?(gerv)
Comment 34•12 years ago
|
||
Comment on attachment 732179 [details] [diff] [review] Part 2: Import Kiss FFT This is fine; but simply call it the "Kiss FFT License" in about:license. Gerv
Attachment #732179 -
Flags: review?(gerv) → review+
Comment 35•12 years ago
|
||
Comment on attachment 732179 [details] [diff] [review] Part 2: Import Kiss FFT Review of attachment 732179 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, but... > "There should not be a problem considering this code is already present in Opus, and because Opus is already in the tree" So, why are we importing this code exactly?
Attachment #732179 -
Flags: review?(mh+mozilla) → review+
Comment 36•12 years ago
|
||
BTW, a separation between the code import and the build bits would have been helpful for review.
Comment 37•12 years ago
|
||
glandium, Comment 25 says: > Note that Kiss FFT is also part of opus, but I'd rather have the library as a clear > dependency which we can update later as opposed to hacking the libopus build system to let > us use its Kiss FFT.
Comment 38•12 years ago
|
||
(In reply to Paul Adenot (:padenot) from comment #37) > glandium, Comment 25 says: > > > Note that Kiss FFT is also part of opus, but I'd rather have the library as a clear > > dependency which we can update later as opposed to hacking the libopus build system to let > > us use its Kiss FFT. Then can we make opus use this one? Because with two copies of the same thing, you can be sure that at some point, one will have fixes the other one won't.
Comment 39•12 years ago
|
||
Actually, it should be possible to use the opus kiss fft without modifying anything to the build system. Just include kiss_fft.h from $(topsrcdir)/media/libopus/celt, and prefix the function calls with opus_.
Assignee | ||
Comment 40•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #39) > Actually, it should be possible to use the opus kiss fft without modifying > anything to the build system. Just include kiss_fft.h from > $(topsrcdir)/media/libopus/celt, and prefix the function calls with opus_. The kiss_fft inside libopus lacks the tools directory, where kiss_fftr which I use in this code comes from. I don't really want to implement my own kiss_fftr on top of libopus' kiss_fft. Also, I'm not sure if the kiss_fft symbols in libopus are exported or not, since libopus lives in libgkmedias and the code using kiss_fftr lives in libxul.
Comment 41•12 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #40) > (In reply to Mike Hommey [:glandium] from comment #39) > > Actually, it should be possible to use the opus kiss fft without modifying > > anything to the build system. Just include kiss_fft.h from > > $(topsrcdir)/media/libopus/celt, and prefix the function calls with opus_. > > The kiss_fft inside libopus lacks the tools directory, where kiss_fftr which > I use in this code comes from. I don't really want to implement my own > kiss_fftr on top of libopus' kiss_fft. Also, I'm not sure if the kiss_fft > symbols in libopus are exported or not, since libopus lives in libgkmedias > and the code using kiss_fftr lives in libxul. You could import kiss_fftr, make it use libopus's kiss_fft, and put it in libgkmedias.
Assignee | ||
Comment 42•12 years ago
|
||
(In reply to Paul Adenot (:padenot) from comment #33) > ::: media/kiss_fft/.hg_archival.txt > @@ +1,4 @@ > > +repo: d844c818f599aea64fe86745cdd2ef9b3d1910dc > > +node: b354a59534b0a77c43c67deb1eb1bc39eb99b487 > > +branch: default > > +tag: v130 > > Are those .hg* files intended to be in the import? I just imported the entire redistributable package. These files are not _needed_ and I can clean them up. > ::: media/kiss_fft/README.mozilla > @@ +1,2 @@ > > +This is the Kiss FFT library, version 1.3.0, retrieved from > > +http://hivelocity.dl.sourceforge.net/project/kissfft/kissfft/v1_3_0/kiss_fft130.zip. > > Maybe you could document the process one should use to update the library? > It seems to be a matter of simply doing `hg update`, but I'm not sure. Oh, I just downloaded the zip file, so the update process for future versions is to download the new zip archive and put its contents in this directory... > Usually, we just have a script called `update.sh` that takes an upstream > repo as $1, and pull in the new version, applying any patch that is needed > in the process. Considering you Makefile.in references only one .c file, it > could be cool to copy just the needed files. OK I'll add that. > ::: media/kiss_fft/test/benchfftw.c > @@ +32,5 @@ > > + fprintf(stderr,"Datatype not available in FFTW\n" ); > > + return 0; > > +} > > +#else > > +int main(int argc,char ** argv) > > We probably don't want to compile that, and maybe we should not include it > at all in the library version we have in-tree, that and all the files we > don't use, such as benchmarks, python examples, matlab files, and other > stuff. It might make the update process simpler, but the library is updated > very rarely, so it should not matter. Alright. These files were not built in my version of the patch, but I'll create another import which just copies the required files and nothing more.
Assignee | ||
Comment 43•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #41) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #40) > > (In reply to Mike Hommey [:glandium] from comment #39) > > > Actually, it should be possible to use the opus kiss fft without modifying > > > anything to the build system. Just include kiss_fft.h from > > > $(topsrcdir)/media/libopus/celt, and prefix the function calls with opus_. > > > > The kiss_fft inside libopus lacks the tools directory, where kiss_fftr which > > I use in this code comes from. I don't really want to implement my own > > kiss_fftr on top of libopus' kiss_fft. Also, I'm not sure if the kiss_fft > > symbols in libopus are exported or not, since libopus lives in libgkmedias > > and the code using kiss_fftr lives in libxul. > > You could import kiss_fftr, make it use libopus's kiss_fft, and put it in > libgkmedias. What would be the point of this exercise? You're assuming that these two libraries can be used interchangeably, that's not the case. The libopus version has diverged from upstream heavily and I'm not sure what the changes involved are. Also, it's compiled with -DFIXED_POINT=1 on mobile, but I do need the floating point version on mobile too since my input and output data are floats...
Comment 44•12 years ago
|
||
Comment on attachment 732182 [details] [diff] [review] Part 4: Implement the analysis routines of AnalyserNode Review of attachment 732182 [details] [diff] [review]: ----------------------------------------------------------------- This fails to build (with the four patches applied on an updated central tree). http://www.pastebin.mozilla.org/2267323 ::: content/media/webaudio/AnalyserNode.cpp @@ +188,5 @@ > + buffer[i] = static_cast<unsigned char>(scaled); > + } > +} > + > +bool You don't use the return value on the call site, maybe you could make this function void. @@ +196,5 @@ > + bool allocated = false; > + if (mWriteIndex == 0) { > + inputBuffer = mBuffer.Elements(); > + } else { > + inputBuffer = static_cast<float*>(moz_malloc(mFFTSize * sizeof(float))); Why |moz_malloc| instead of |new|? @@ +206,5 @@ > + allocated = true; > + } > + nsAutoArrayPtr<kiss_fft_cpx> outputBuffer(new kiss_fft_cpx[FrequencyBinCount() + 1]); > + > + ApplyBlackmanWindow(inputBuffer, mFFTSize); I could not find the window to use in the spec. Time to raise another spec issue, I believe. @@ +224,5 @@ > + (1.0 - mSmoothingTimeConstant) * scalarMagnitude; > + } > + > + if (allocated) { > + moz_free(inputBuffer); |delete []| instead of |moz_free| if you make the change above.
Assignee | ||
Comment 45•12 years ago
|
||
(In reply to Paul Adenot (:padenot) from comment #44) > Comment on attachment 732182 [details] [diff] [review] > Part 4: Implement the analysis routines of AnalyserNode > > Review of attachment 732182 [details] [diff] [review]: > ----------------------------------------------------------------- > > This fails to build (with the four patches applied on an updated central > tree). > > http://www.pastebin.mozilla.org/2267323 Yeah, sorry, I had a local patch applied which broke this. Please use the version 1 patch that I pushed to inbound yesterday and just apply parts 2-4. (They should apply and build fine on inbound.) > ::: content/media/webaudio/AnalyserNode.cpp > @@ +188,5 @@ > > + buffer[i] = static_cast<unsigned char>(scaled); > > + } > > +} > > + > > +bool > > You don't use the return value on the call site, maybe you could make this > function void. Oh that's my mistake, I'll fix it. This function can fail to allocate memory, hence the need for returning a bool. > @@ +196,5 @@ > > + bool allocated = false; > > + if (mWriteIndex == 0) { > > + inputBuffer = mBuffer.Elements(); > > + } else { > > + inputBuffer = static_cast<float*>(moz_malloc(mFFTSize * sizeof(float))); > > Why |moz_malloc| instead of |new|? To make the allocation fallible, since the allocation size comes from web content. operator new is infallible by default. If you want, I can use new(fallible) similar to here: <http://mxr.mozilla.org/mozilla-central/source/content/media/webaudio/MediaBufferDecoder.cpp#527> but I don't like that style for things which don't need to get constructed. > @@ +206,5 @@ > > + allocated = true; > > + } > > + nsAutoArrayPtr<kiss_fft_cpx> outputBuffer(new kiss_fft_cpx[FrequencyBinCount() + 1]); > > + > > + ApplyBlackmanWindow(inputBuffer, mFFTSize); > > I could not find the window to use in the spec. Time to raise another spec > issue, I believe. https://www.w3.org/Bugs/Public/show_bug.cgi?id=21446 covers that. :-) > @@ +224,5 @@ > > + (1.0 - mSmoothingTimeConstant) * scalarMagnitude; > > + } > > + > > + if (allocated) { > > + moz_free(inputBuffer); > > |delete []| instead of |moz_free| if you make the change above. Let me know if you want me to use operator new(fallible)...
Assignee | ||
Comment 46•12 years ago
|
||
Attachment #732182 -
Attachment is obsolete: true
Attachment #732182 -
Flags: review?(paul)
Attachment #732308 -
Flags: review?(paul)
Comment 47•12 years ago
|
||
Comment on attachment 732180 [details] [diff] [review] Part 3: Send the buffer seen by AnalyserNodeEngine to AnalyserNode Review of attachment 732180 [details] [diff] [review]: ----------------------------------------------------------------- I'll go and talk about what we should spec in the bug on the mailing list.
Attachment #732180 -
Flags: review?(paul) → review+
Updated•12 years ago
|
Attachment #732308 -
Flags: review?(paul) → review+
Comment 48•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/46e9d41acdeb
Flags: in-testsuite+
Assignee | ||
Comment 49•12 years ago
|
||
This version imports the minimum of Kiss FFT that we need into the tree, and it also adds an update.sh script for future updates.
Attachment #732179 -
Attachment is obsolete: true
Assignee | ||
Comment 50•12 years ago
|
||
This is just the build system changes. glandium said he wants to have another look at this.
Attachment #732457 -
Flags: review?(mh+mozilla)
Comment 51•12 years ago
|
||
Comment on attachment 732457 [details] [diff] [review] Part 3: Integrate Kiss FFT with the build system Review of attachment 732457 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/kiss_fft/Makefile.in @@ +9,5 @@ > + > +include $(DEPTH)/config/autoconf.mk > + > +LIBRARY_NAME = kiss_fft > +SHORT_LIBNAME = kiss_fft Don't set SHORT_LIBNAME. @@ +10,5 @@ > +include $(DEPTH)/config/autoconf.mk > + > +LIBRARY_NAME = kiss_fft > +SHORT_LIBNAME = kiss_fft > +VISIBILITY_FLAGS = You probably want to enclose this with ifeq (WINNT,$(OS_TARGET)) @@ +13,5 @@ > +SHORT_LIBNAME = kiss_fft > +VISIBILITY_FLAGS = > +EXPORTS_NAMESPACES = kiss_fft > + > +EXTRA_DSO_LDOPTS += $(MOZALLOC_LIB) Unnecessary. @@ +15,5 @@ > +EXPORTS_NAMESPACES = kiss_fft > + > +EXTRA_DSO_LDOPTS += $(MOZALLOC_LIB) > + > +EXPORTS_kiss_fft = kiss_fft.h kiss_fftr.h I assume you made the directory flat. (as opposed to the previous tools/ subdirectory)
Attachment #732457 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 52•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #51) > @@ +15,5 @@ > > +EXPORTS_NAMESPACES = kiss_fft > > + > > +EXTRA_DSO_LDOPTS += $(MOZALLOC_LIB) > > + > > +EXPORTS_kiss_fft = kiss_fft.h kiss_fftr.h > > I assume you made the directory flat. (as opposed to the previous tools/ > subdirectory) Yes.
Assignee | ||
Comment 53•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f7d9ca669a8 https://hg.mozilla.org/integration/mozilla-inbound/rev/93e66438fb71 https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c0b77f3022 https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e56c43e359 Frederick, these patches fix Google Music for me. Would you mind retesting when this stuff hits Nightly? Thanks!
Whiteboard: [leave open]
Reporter | ||
Comment 54•12 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #53) > https://hg.mozilla.org/integration/mozilla-inbound/rev/3f7d9ca669a8 > https://hg.mozilla.org/integration/mozilla-inbound/rev/93e66438fb71 > https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c0b77f3022 > https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e56c43e359 > > Frederick, these patches fix Google Music for me. Would you mind retesting > when this stuff hits Nightly? > > Thanks! Of course, I will test this ! Just waiting to see these patches to be integrated in main trunk code.
Assignee | ||
Comment 55•12 years ago
|
||
gcc cannot deal with template arguments that are local classes. Somehow that doesn't surprise me! Fix: https://hg.mozilla.org/integration/mozilla-inbound/rev/581b88b27b2d
Assignee | ||
Comment 56•12 years ago
|
||
FWIW, we discussed the possibility of merging the kissfft used here with the copy in libopus, and given our constraints sharing them seems impractical at this point. Also it seems like the libopus copy has diverged heavily from upstream, so we might not be duplicating much code after all.
Comment 57•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3f7d9ca669a8 https://hg.mozilla.org/mozilla-central/rev/93e66438fb71 https://hg.mozilla.org/mozilla-central/rev/a0c0b77f3022 https://hg.mozilla.org/mozilla-central/rev/e5e56c43e359 https://hg.mozilla.org/mozilla-central/rev/581b88b27b2d
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Reporter | ||
Comment 58•12 years ago
|
||
Thanks to everybody. Glad to get webaudio enabled again and a working google music account !
Updated•12 years ago
|
Keywords: dev-doc-needed
![]() |
||
Comment 60•12 years ago
|
||
Ehsan, do we need this on Aurora 22?
tracking-firefox22:
--- → ?
Flags: needinfo?(ehsan)
Assignee | ||
Comment 61•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #60) > Ehsan, do we need this on Aurora 22? Hmm, I'm undecided on whether we should backport this or just disable Web Audio on Aurora 22... This fixes Google Music, but Google Music doesn't actually seem to use Web Audio for playback anyways, otherwise it would probably be broken on Aurora for other things we don't yet implement. On the other hand, other content which uses Web Audio should be broken to a good extent on Aurora 22 anyways... What do you think, Boris?
Flags: needinfo?(ehsan)
![]() |
||
Comment 62•12 years ago
|
||
If we're planning to disable web audio on beta 22 no matter what (I assume we are?) then I'm not sure it's worth worrying about doing anything with this bug on Aurora...
Assignee | ||
Comment 63•12 years ago
|
||
(In reply to comment #62) > If we're planning to disable web audio on beta 22 no matter what (I assume we > are?) then I'm not sure it's worth worrying about doing anything with this bug > on Aurora... We're definitely not planning to enable Web Audio on Beta 22. The pref is currently enabled behind RELEASE_BUILD, which means Nightly and Aurora.
Updated•11 years ago
|
Comment 65•10 years ago
|
||
Doc is up-to-date here: https://developer.mozilla.org/en-US/docs/Web/API/AnalyserNode
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•