(no longer active) Reporter
	Description • 11 years ago

      No description provided.

	(no longer active) Reporter
	Comment 1 • 11 years ago

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.

	JW Wang [:jwwang] [:jw_wang] Assignee
	Comment 2 • 11 years ago

The idea is to use libspeex for up/down sampling per chunk. However, speex_resampler_process_float() gives weird outputs in Resampler::UpSample/DownSample. Is there anything missing to use libspeex correctly?

	Karl Tomlinson (:karlt)
	Comment 3 • 11 years ago

Pass 1 for channel count to _init and 0 for channel number in _process_float because the channels are already individual streams.

	Karl Tomlinson (:karlt)
	Comment 4 • 11 years ago

Actually, please ignore comment 3, sorry.

	(no longer active) Reporter
	Comment 8 • 11 years ago

Comment on attachment 782480 [details] [diff] [review]
Part2 - add up/down samplers from Blink

Review of attachment 782480 [details] [diff] [review]:
-----------------------------------------------------------------

Why should we not use libspeex here?  I don't think this is the right thing to do.

	(no longer active) Reporter
	Comment 10 • 11 years ago

Comment on attachment 782972 [details] [diff] [review]
Part2 - implement based on libspeex

Review of attachment 782972 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks!

::: content/media/webaudio/WaveShaperNode.cpp
@@ +67,5 @@
> +  void Reset(uint32_t aChannels, uint32_t aSampleRate, OverSampleType aType)
> +  {
> +    if (aChannels == mChannels
> +        && aSampleRate == mSampleRate
> +        && aType == mType)

Nit: please put the ands at the end of lines.  Also, please wrap if's in curly brackets.

@@ +90,5 @@
> +    mDownSampler = speex_resampler_init(aChannels,
> +                                        aSampleRate * ValueOf(aType),
> +                                        aSampleRate,
> +                                        SPEEX_RESAMPLER_QUALITY_DEFAULT,
> +                                        nullptr);

Hmm, I think it would be slightly better to lazily allocate the resamplers, since I believe in most cases we will only end up using one of them.

@@ +104,5 @@
> +    MOZ_ASSERT(mBuffer.Length() == outSamples);
> +
> +    speex_resampler_process_float(mUpSampler, aChannel,
> +                                  aInputData, &inSamples,
> +                                  outputData, &outSamples);

After this line, please MOZ_ASSERT(inSamples == WEBAUDIO_BLOCK_SIZE && outSamples == WEBAUDIO_BLOCK_SIZE * aBlocks);.  Same in DownSample() below.

@@ +139,5 @@
> +  OverSampleType mType;
> +  SpeexResamplerState* mUpSampler;
> +  SpeexResamplerState* mDownSampler;
> +  uint32_t mChannels;
> +  uint32_t mSampleRate;

Nit: please use TrackRate as the type of the sample rate here and in the above functions.  (TrackRate is int32_t.)

	JW Wang [:jwwang] [:jw_wang] Assignee
	Comment 11 • 11 years ago

Comment on attachment 782972 [details] [diff] [review]
Part2 - implement based on libspeex

Review of attachment 782972 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/WaveShaperNode.cpp
@@ +90,5 @@
> +    mDownSampler = speex_resampler_init(aChannels,
> +                                        aSampleRate * ValueOf(aType),
> +                                        aSampleRate,
> +                                        SPEEX_RESAMPLER_QUALITY_DEFAULT,
> +                                        nullptr);

I don't get it. Do you mean we will only use either upsampler or downsampler in most cases? I think we will need both of them to complete the process (upsample -> apply curve -> downsample).

	(no longer active) Reporter
	Comment 12 • 11 years ago

(In reply to jwwang from comment #11)
> Comment on attachment 782972 [details] [diff] [review]
> Part2 - implement based on libspeex
> 
> Review of attachment 782972 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/webaudio/WaveShaperNode.cpp
> @@ +90,5 @@
> > +    mDownSampler = speex_resampler_init(aChannels,
> > +                                        aSampleRate * ValueOf(aType),
> > +                                        aSampleRate,
> > +                                        SPEEX_RESAMPLER_QUALITY_DEFAULT,
> > +                                        nullptr);
> 
> I don't get it. Do you mean we will only use either upsampler or downsampler
> in most cases? I think we will need both of them to complete the process
> (upsample -> apply curve -> downsample).

No, sorry, this comment was a mistake of mine, and I meant to remove it before submitting the review!  Please ignore it.

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/a633e4d67d31
https://hg.mozilla.org/integration/mozilla-inbound/rev/aee6b1a6c400

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

Backed out for bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0dd80f800e2

https://tbpl.mozilla.org/php/getParsedLog.php?id=26184561&tree=Mozilla-Inbound

	JW Wang [:jwwang] [:jw_wang] Assignee
	Comment 16 • 11 years ago

"OverSampleType mType;" is missing from WaveShaperNode.h after merge and results in build fail. Does the merge fail?

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 11 years ago

Maybe. Please make sure your patches apply against m-c tip and rebase as necessary.

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/802ef003bb08
https://hg.mozilla.org/integration/mozilla-inbound/rev/1406e31167e2

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/802ef003bb08
https://hg.mozilla.org/mozilla-central/rev/1406e31167e2

	(no longer active) Reporter
	Comment 22 • 11 years ago

Comment on attachment 786152 [details] [diff] [review]
Part1 - add webidl enums

This is needed for Web Audio in Firefox 25.

	(no longer active) Reporter
	Comment 23 • 11 years ago

checkin-needed for Aurora, a=webaudio.

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/109bf9967b3d
https://hg.mozilla.org/releases/mozilla-aurora/rev/32aab56d0aa5

	Phil Ringnalda (:philor)
	Comment 25 • 11 years ago

Backed out in https://hg.mozilla.org/releases/mozilla-aurora/rev/4d652113720b because something in https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=2d110609f4d3 caused test_mediaStreamAudioSourceNodeResampling.html to time out (once https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=918a847c0018 let things get that far).

	Jean-Marc Valin
	Comment 26 • 11 years ago

Clear the needinfo

	Jean-Yves Perrier [:teoli]
	Comment 27 • 10 years ago

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

	Jean-Yves Perrier [:teoli]
	Comment 28 • 10 years ago

Ooops, last link should have been: 
https://developer.mozilla.org/en-US/Firefox/Releases/26