Removals and renamings in the new audio mixer.

Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True

Committed: https://crrev.com/5d167d6829673aeb6e7e6eaeee2dd57ccc06dc17
Cr-Commit-Position: refs/heads/master@{#13883}

[aleloi, 2016-08-18 12:14:13]

aleloi@webrtc.org changed reviewers:
+ ivoc@webrtc.org

[aleloi, 2016-08-18 12:14:14]

ivoc@, this is hopefully the last audio mixer CL with large structural changes.
'git diff' seems not to cooperate, so I'll write here what I've actually done:

DELETED audio_mixer.h
DELETED audio_mixer.cc
MOVED new_audio_conference_mixer.h -> audio_mixer.h
MOVED new_audio_conference_mixer_impl.h -> audio_mixer_impl.h
MOVED new_audio_conference_mixer_impl.cc -> audio_mixer_impl.cc


I also did matching renamings inside the mixer and unit tests and updated the
build files.

PTAL!

[ivoc, 2016-08-18 15:47:23]

Looks mostly good, see some minor comments below.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:2: *  Copyright (c) 2011 The WebRTC
project authors. All Rights Reserved.
This looks a bit odd, is the new mixer older than the old mixer? :-)

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:23: enum {
kMaximumAmountOfMixedAudioSources = 3 };
I think this should be a static const int.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:34: static AudioMixer* Create(int id);
You can consider to change this to return a unique_ptr<AudioMixer>.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer_impl.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer_impl.h:38: // Cheshire cat implementation
of MixerAudioSource's non virtual functions.
Not sure what "Cheshire cat" means here.

[aleloi, 2016-08-19 12:06:09]

New patch set and responses to comments.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:2: *  Copyright (c) 2011 The WebRTC
project authors. All Rights Reserved.
On 2016/08/18 15:47:23, ivoc wrote:
> This looks a bit odd, is the new mixer older than the old mixer? :-)

The old mixer is a different file. I think we had a discussion with tommi@
earlier and the conclusion was to not update headers unless it's a completely
new file.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:23: enum {
kMaximumAmountOfMixedAudioSources = 3 };
On 2016/08/18 15:47:23, ivoc wrote:
> I think this should be a static const int.
There are apparently some differences between anonymous enum and static const:
http://stackoverflow.com/a/1377865

But I think static const int makes more sense here.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:34: static AudioMixer* Create(int id);
On 2016/08/18 15:47:23, ivoc wrote:
> You can consider to change this to return a unique_ptr<AudioMixer>.

That makes sense! Done.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer_impl.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer_impl.h:38: // Cheshire cat implementation
of MixerAudioSource's non virtual functions.
On 2016/08/18 15:47:23, ivoc wrote:
> Not sure what "Cheshire cat" means here.

I looked it up when reading the mixer code for the first time. It means that the
declaration & definition of MixHistory is only visible in this file, but the
fact that AudioMixerSource has a MixHistory* is visible publicly.

Ref: https://en.wikipedia.org/wiki/Opaque_pointer

[ivoc, 2016-08-19 12:37:37]

LGTM

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:2: *  Copyright (c) 2011 The WebRTC
project authors. All Rights Reserved.
On 2016/08/19 12:06:08, aleloi wrote:
> On 2016/08/18 15:47:23, ivoc wrote:
> > This looks a bit odd, is the new mixer older than the old mixer? :-)
> 
> The old mixer is a different file. I think we had a discussion with tommi@
> earlier and the conclusion was to not update headers unless it's a completely
> new file.

Acknowledged.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer.h:23: enum {
kMaximumAmountOfMixedAudioSources = 3 };
On 2016/08/19 12:06:08, aleloi wrote:
> On 2016/08/18 15:47:23, ivoc wrote:
> > I think this should be a static const int.
> There are apparently some differences between anonymous enum and static const:
> http://stackoverflow.com/a/1377865
> 
> But I think static const int makes more sense here.

Interesting, but I agree that I prefer static const int, mostly because it looks
less confusing.

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
File webrtc/modules/audio_mixer/audio_mixer_impl.h (right):

https://codereview.webrtc.org/2249213005/diff/1/webrtc/modules/audio_mixer/au...
webrtc/modules/audio_mixer/audio_mixer_impl.h:38: // Cheshire cat implementation
of MixerAudioSource's non virtual functions.
On 2016/08/19 12:06:09, aleloi wrote:
> On 2016/08/18 15:47:23, ivoc wrote:
> > Not sure what "Cheshire cat" means here.
> 
> I looked it up when reading the mixer code for the first time. It means that
the
> declaration & definition of MixHistory is only visible in this file, but the
> fact that AudioMixerSource has a MixHistory* is visible publicly.
> 
> Ref: https://en.wikipedia.org/wiki/Opaque_pointer

Okay, I'm familiar with opaque pointers, but I've just never heard them being
called "cheshire cat" before, but if that's a common term for it then that's
fine.

[aleloi, 2016-08-24 08:51:03]

Description was changed from

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.
==========

to

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True
==========

[aleloi, 2016-08-24 08:51:08]

The CQ bit was checked by aleloi@webrtc.org

[aleloi, 2016-08-24 08:51:08]

The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2249213005/#ps80001 (title:
"Set local vars const in tests.")

[commit-bot: I haz the power, 2016-08-24 08:51:12]

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

[commit-bot: I haz the power, 2016-08-24 09:20:57]

Description was changed from

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True
==========

to

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True
==========

[commit-bot: I haz the power, 2016-08-24 09:20:58]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-24 09:21:08]

Description was changed from

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True
==========

to

==========
Removals and renamings in the new audio mixer.

Removed the OutputMixer part of the new mixer and renamed the new
mixer from NewAudioConferenceMixer to AudioMixer.

NOTRY=True

Committed: https://crrev.com/5d167d6829673aeb6e7e6eaeee2dd57ccc06dc17
Cr-Commit-Position: refs/heads/master@{#13883}
==========

[commit-bot: I haz the power, 2016-08-24 09:21:09]

Patchset 5 (id:??) landed as
https://crrev.com/5d167d6829673aeb6e7e6eaeee2dd57ccc06dc17
Cr-Commit-Position: refs/heads/master@{#13883}