Status

()

defect
RESOLVED FIXED
6 years ago
2 months ago

People

(Reporter: fredbezies, Assigned: Ehsan)

Tracking

({dev-doc-complete, regression})

Trunk
mozilla23
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22-)

Details

Attachments

(7 attachments, 3 obsolete attachments)

Reporter

Description

6 years ago
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

6 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

6 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
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?
Flags: needinfo?(fredbezies)
Keywords: regression
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

6 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

6 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 :(
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

6 years ago
I saw it. So without anyone with a google music account, this bug will be blocked.
Flags: needinfo?(fredbezies)
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?
DOM seems more likely than Style System given the regression range (comment 3).
Component: Style System (CSS) → DOM
Keywords: testcase-wanted
Reporter

Comment 11

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

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

6 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 ;)
Reporter

Comment 16

6 years ago
saved page, hoping it could help.
> 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?
Attachment #731273 - Attachment mime type: application/octet-stream → application/x-zip
Reporter

Comment 18

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

Updated

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

6 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 !
Blocks: 851603
Flags: needinfo?(ehsan)
> the only fix is to implement createAnalyzer ?

And that might well not be enough, depending on what else they're using.
I'll try to get to this soon.
Assignee: nobody → ehsan
Blocks: webaudio
No longer blocks: 851603
Flags: needinfo?(ehsan)
Keywords: testcase-wanted
Summary: Google Play Music doesn't load completely if webaudio is enabled. → Implement DOM bindings for AnalyserNode
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.
Summary: Implement DOM bindings for AnalyserNode → Implement AnalyserNode
Attachment #731753 - Flags: review?(bzbarsky)
Posted patch Part 2: Import Kiss FFT (obsolete) — Splinter Review
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 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+
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+
Whiteboard: [leave open]
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+
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+
Posted patch Part 2: Import Kiss FFT (obsolete) — Splinter Review
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)
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 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 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 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+
BTW, a separation between the code import and the build bits would have been helpful for review.
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.
(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.
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_.
(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.
(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.
(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.
(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 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.
(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)...
Attachment #732182 - Attachment is obsolete: true
Attachment #732182 - Flags: review?(paul)
Attachment #732308 - Flags: review?(paul)
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+
Attachment #732308 - Flags: review?(paul) → review+
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
This is just the build system changes.  glandium said he wants to have another look at this.
Attachment #732457 - Flags: review?(mh+mozilla)
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+
(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.
Reporter

Comment 54

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

Comment 58

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

Updated

6 years ago
Duplicate of this bug: 861588
Ehsan, do we need this on Aurora 22?
Flags: needinfo?(ehsan)
(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)
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...
(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.
Given comment 63, we won't track or uplift this to 22.

Updated

6 years ago
Blocks: 894848
Blocks: 894856
No longer blocks: 894848
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.