855990 - Implement AnalyserNode

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Frederic Bezies]

Attachment: Infos pasted from error console.

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.

Comment 1

[Frederic Bezies]

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

Comment 2

[Frederic Bezies]

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

[Mats Palmgren (inactive)]

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?

Comment 4

[Boris Zbarsky [:bzbarsky]]

This needs steps to reproduce, ideally..... ones that someone who has never done any music stuff with a Google account could follow.

Comment 5

[Frederic Bezies]

(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.

Comment 6

[Frederic Bezies]

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

[Boris Zbarsky [:bzbarsky]]

See comment 4?

Seriously, it's hard to say anything here without having some idea of what the page is doing.

Comment 8

[Frederic Bezies]

I saw it. So without anyone with a google music account, this bug will be blocked.

Comment 9

[Boris Zbarsky [:bzbarsky]]

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

[Mats Palmgren (inactive)]

DOM seems more likely than Style System given the regression range (comment 3).

Comment 11

[Frederic Bezies]

(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

[Boris Zbarsky [:bzbarsky]]

> 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!

Comment 13

[Frederic Bezies]

(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

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 15

[Frederic Bezies]

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 16

[Frederic Bezies]

Attachment: saved page as resquested.

saved page, hoping it could help.

Comment 17

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 18

[Frederic Bezies]

Verified on my homemade build (10 hours old)... And you got it ! Deactivating webaudio fixes the bug !

Comment 19

[Boris Zbarsky [:bzbarsky]]

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.

Comment 20

[Frederic Bezies]

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 !

Comment 21

[Boris Zbarsky [:bzbarsky]]

> the only fix is to implement createAnalyzer ?

And that might well not be enough, depending on what else they're using.

Comment 22

[(no longer active)]

I'll try to get to this soon.

Comment 23

[(no longer active)]

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.

Comment 24

[(no longer active)]

Attachment: Part 1: DOM bindings

Comment 25

[(no longer active)]

Attachment: Part 2: Import Kiss FFT

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.

Comment 26

[Boris Zbarsky [:bzbarsky]]

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.

Comment 27

[(no longer active)]

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...

Comment 28

[(no longer active)]

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

Comment 29

[(no longer active)]

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

Comment 30

[(no longer active)]

Attachment: Part 2: Import Kiss FFT

This incorporates some changes that I had to make to the build system to make things work.

Comment 31

[(no longer active)]

Attachment: Part 3: Send the buffer seen by AnalyserNodeEngine to AnalyserNode

Comment 32

[(no longer active)]

Attachment: Part 4: Implement the analysis routines of AnalyserNode

This mimics what WebKit does to implement the AnalyserNode, pending upcoming spec clarifications.

Testcase/demo: http://people.mozilla.com/~eakhgari/webaudio/analyser.html

Comment 33

[Paul Adenot (:padenot)]

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.

Comment 34

[Gervase Markham [:gerv]]

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

Comment 35

[Mike Hommey [:glandium]]

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?

Comment 36

[Mike Hommey [:glandium]]

BTW, a separation between the code import and the build bits would have been helpful for review.

Comment 37

[Paul Adenot (:padenot)]

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

[Mike Hommey [:glandium]]

(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

[Mike Hommey [:glandium]]

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_.

Comment 40

[(no longer active)]

(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

[Mike Hommey [:glandium]]

(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.

Comment 42

[(no longer active)]

(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.

Comment 43

[(no longer active)]

(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

[Paul Adenot (:padenot)]

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.

Comment 45

[(no longer active)]

(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)...

Comment 46

[(no longer active)]

Attachment: Part 4: Implement the analysis routines of AnalyserNode

Comment 47

[Paul Adenot (:padenot)]

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.

Comment 48

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/46e9d41acdeb

Comment 49

[(no longer active)]

Attachment: Part 2: Import Kiss FFT

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.

Comment 50

[(no longer active)]

Attachment: Part 3: Integrate Kiss FFT with the build system

This is just the build system changes.  glandium said he wants to have another look at this.

Comment 51

[Mike Hommey [:glandium]]

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)

Comment 52

[(no longer active)]

(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.

Comment 53

[(no longer active)]

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!

Comment 54

[Frederic Bezies]

(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.

Comment 55

[(no longer active)]

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

Comment 56

[(no longer active)]

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

[Ryan VanderMeulen [:RyanVM]]

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

Comment 58

[Frederic Bezies]

Thanks to everybody. Glad to get webaudio enabled again and a working google music account !

Comment 60

[Boris Zbarsky [:bzbarsky]]

Ehsan, do we need this on Aurora 22?

Comment 61

[(no longer active)]

(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?

Comment 62

[Boris Zbarsky [:bzbarsky]]

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...

Comment 63

[(no longer active)]

(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.

Comment 64

[Lukas Blakk [:lsblakk] use ?needinfo]

Given comment 63, we won't track or uplift this to 22.

Comment 65

[Jean-Yves Perrier [:teoli]]

Doc is up-to-date here: https://developer.mozilla.org/en-US/docs/Web/API/AnalyserNode