Remove redundant definition of Spatializer abstract class

Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)

Committed: https://crrev.com/6a7e84782f979dd2720761025881ac62ddf41b1c
Cr-Commit-Position: refs/heads/master@{#414502}

[hongchan, 2016-08-23 23:08:07]

hongchan@chromium.org changed reviewers:
+ rtoy@chromium.org

[hongchan, 2016-08-23 23:08:08]

PTAL.

[Raymond Toy, 2016-08-24 15:57:58]

I'm all for removing Spatializer.*, but I wonder if this is really the right
abstraction.  It seems odd to smash in the stereo panner (2D) into the 3D
(mostly) panner.  Would it be bette rto extract out the common parts to create a
BasePanner class?

[hongchan, 2016-08-24 16:02:53]

On 2016/08/24 15:57:58, Raymond Toy wrote:
> I'm all for removing Spatializer.*, but I wonder if this is really the right
> abstraction.  It seems odd to smash in the stereo panner (2D) into the 3D
> (mostly) panner.  Would it be bette rto extract out the common parts to create
a
> BasePanner class?

Yes, I agree. One thing bothers me is what actually EQP does is not 3D panning -
it's more of 2D. Are we fine with that?

[hongchan, 2016-08-24 18:16:59]

Patchset #2 (id:20001) has been deleted

[hongchan, 2016-08-24 18:18:13]

The 2nd attempt - PTAL.

[Raymond Toy, 2016-08-24 18:30:29]

Just a bunch of nits.  Please fix the description to match what you're doing
now.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:66:
m_stereoPanner->panWithSampleAccurateValues(
It looks like nothing changed here other than indentation. If so, please revert.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:71: inputBus,
outputBus, m_pan->value(), framesToProcess);
Please revert this indentation change.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/Panner.cpp (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/Panner.cpp:47: NOTREACHED();
Does the presubmit complain about this? If not, please revert.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/StereoPanner.cpp (left):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/StereoPanner.cpp:24:
m_smoothingConstant =
AudioUtilities::discreteTimeConstantForSampleRate(SmoothingTimeConstant,
sampleRate);
I'd prefer not changing these lines.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/StereoPanner.h (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/StereoPanner.h:11: #include <memory>
Is including memory needed here?  Is it for USING_FAST_MALLOC?

[hongchan, 2016-08-24 18:39:37]

Description was changed from

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to move
StereoPanner class into the platform/audio/Panner.h and deprecate
Spatializer class.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)
==========

to

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)
==========

[hongchan, 2016-08-24 20:41:47]

Patchset #3 (id:60001) has been deleted

[hongchan, 2016-08-24 21:45:36]

Patchset #3 (id:80001) has been deleted

[hongchan, 2016-08-24 21:51:49]

Patchset #3 (id:100001) has been deleted

[hongchan, 2016-08-24 21:52:33]

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:66:
m_stereoPanner->panWithSampleAccurateValues(
On 2016/08/24 18:30:29, Raymond Toy wrote:
> It looks like nothing changed here other than indentation. If so, please
revert.

Done.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/StereoPannerNode.cpp:71: inputBus,
outputBus, m_pan->value(), framesToProcess);
On 2016/08/24 18:30:29, Raymond Toy wrote:
> Please revert this indentation change.

Done.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/Panner.cpp (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/Panner.cpp:47: NOTREACHED();
On 2016/08/24 18:30:29, Raymond Toy wrote:
> Does the presubmit complain about this? If not, please revert.

Oh, actually NOTREACHED is the DCHECK versions of ASSERT_NOT_REACHED. Still, I
am reverting this.

Done.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/StereoPanner.cpp (left):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/StereoPanner.cpp:24:
m_smoothingConstant =
AudioUtilities::discreteTimeConstantForSampleRate(SmoothingTimeConstant,
sampleRate);
On 2016/08/24 18:30:29, Raymond Toy wrote:
> I'd prefer not changing these lines.

Done.

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/audio/StereoPanner.h (right):

https://codereview.chromium.org/2272683003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/audio/StereoPanner.h:11: #include <memory>
On 2016/08/24 18:30:29, Raymond Toy wrote:
> Is including memory needed here?  Is it for USING_FAST_MALLOC?

Done.

[Raymond Toy, 2016-08-24 22:21:25]

lgtm with nit.

https://codereview.chromium.org/2272683003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/audio/StereoPanner.h (right):

https://codereview.chromium.org/2272683003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/audio/StereoPanner.h:17: //
http://webaudio.github.io/web-audio-api/#panning-algorithm
Nit: this link is broken. I think you want
https://webaudio.github.io/web-audio-api/#Spatialzation-equal-power-panning

[hongchan, 2016-08-24 23:01:20]

hongchan@chromium.org changed reviewers:
+ haraken@chromium.org

[hongchan, 2016-08-24 23:01:21]

haraken@ PTAL. I've removed unnecessary files from blink_platform.gypi.

[hongchan, 2016-08-24 23:01:53]

The CQ bit was checked by hongchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 23:03:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[haraken, 2016-08-24 23:44:59]

LGTM

[commit-bot: I haz the power, 2016-08-25 01:08:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 01:08:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hongchan, 2016-08-25 03:44:23]

The CQ bit was checked by hongchan@chromium.org

[hongchan, 2016-08-25 03:44:23]

The patchset sent to the CQ was uploaded after l-g-t-m from rtoy@chromium.org
Link to the patchset: https://codereview.chromium.org/2272683003/#ps140001
(title: "Fix reference URL")

[commit-bot: I haz the power, 2016-08-25 03:44:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 07:12:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 07:12:36]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hongchan, 2016-08-25 13:42:34]

The CQ bit was checked by hongchan@chromium.org

[commit-bot: I haz the power, 2016-08-25 13:42:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 16:56:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 16:56:03]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hongchan, 2016-08-25 17:06:01]

The CQ bit was checked by hongchan@chromium.org

[commit-bot: I haz the power, 2016-08-25 17:06:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 19:14:28]

Description was changed from

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)
==========

to

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)
==========

[commit-bot: I haz the power, 2016-08-25 19:14:30]

Committed patchset #4 (id:140001)

[commit-bot: I haz the power, 2016-08-25 19:18:27]

Description was changed from

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)
==========

to

==========
Remove redundant definition of Spatializer abstract class

platform/audio/Spatializer.h. was originally designed to own various type of
panners including equal power, HRTF, stereo, sound-field and more.
However, the direction has been changed and now it is reasonable to remove
Spatializer class and make the StereoPanner standalone.

BUG=612299
TEST=NONE (the CL build passes all the panner-related tests.)

Committed: https://crrev.com/6a7e84782f979dd2720761025881ac62ddf41b1c
Cr-Commit-Position: refs/heads/master@{#414502}
==========

[commit-bot: I haz the power, 2016-08-25 19:18:28]

Patchset 4 (id:??) landed as
https://crrev.com/6a7e84782f979dd2720761025881ac62ddf41b1c
Cr-Commit-Position: refs/heads/master@{#414502}