Injectable output rate calculater for AudioMixer.

Injectable output rate calculater for AudioMixer.

This CL breaks out the output sample rate calculation from
webrtc::AudioMixerImpl. A new OutputRateCalculator interface is added
to make the sample rate configurable. There are at least three reasons
for this change:

  1. The mixer will be used for an internal project, in which no
     resampling is done after the mixing. There the sample rate should
     be static. Currently, it can differ across mix iterations and
     depends on the number of audio sources. If there are no sources,
     the WebRTC mixer behavior is to produce silence at 48 kHz.

  2. A planned change to WebRTC will make audio processing steps
     happen at constant sample rates. A configurable sample rate
     calculator will make the transition simpler for the mixer.

  3. The current mixer design is a single large file. Behavior is not
     always simple to change (e.g. as in this case to mix at a
     constant rate), unrelated behavior can be broken, reusing the
     mixer in internal projects is tricky. Using DI for the sample
     rate calculation solves parts of these issues.

Changes:

The protected mixer c-tor now takes
unique_ptr<OutputRateCalculator>. The current output rate calculation
is moved to DefaultOutputRateCalculator. A new factory method
AudioMixerImpl::CreateWithOutputRateCalculator is added. The old
factory method passes the default rate calculator.

BUG=webrtc:6346
Committed: https://crrev.com/623427c5228050fa158671430a2949202a0d7514
Cr-Commit-Position: refs/heads/master@{#15472}

[aleloi, 2016-12-07 12:46:21]

The CQ bit was checked by aleloi@webrtc.org to run a CQ dry run

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

Dry run: 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-12-07 12:52:15]

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

[commit-bot: I haz the power, 2016-12-07 12:52:16]

Dry run: Try jobs failed on following builders:
  win_x64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/20476)

[aleloi, 2016-12-07 13:41:43]

The CQ bit was checked by aleloi@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 13:41:50]

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

[aleloi, 2016-12-07 13:43:14]

Patchset #1 (id:1) has been deleted

[aleloi, 2016-12-07 13:44:17]

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

[aleloi, 2016-12-07 13:44:18]

Hi, can you please take a look?

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
File webrtc/modules/audio_mixer/audio_mixer_impl.cc (left):

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
webrtc/modules/audio_mixer/audio_mixer_impl.cc:227: }
Moved into private method, because two nested lambdas is one lambda too many...

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
File webrtc/modules/audio_mixer/audio_mixer_impl.cc (right):

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
webrtc/modules/audio_mixer/audio_mixer_impl.cc:142:
std::unique_ptr<AudioProcessing> CreateLimiter() {
Constructing a limiter is common for the two factory methods.

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
webrtc/modules/audio_mixer/audio_mixer_impl.cc:189: 
Changes: invalid dummy values for the frequency and sample size. These will not
affect anything, because they are over-written at the start of each Mix
iteration during the SetOutputFrequency() call.

[commit-bot: I haz the power, 2016-12-07 13:58:20]

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

[commit-bot: I haz the power, 2016-12-07 13:58:21]

Dry run: This issue passed the CQ dry run.

[ivoc, 2016-12-08 10:09:47]

LGTM, but please have a look at my comments below.

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
File webrtc/modules/audio_mixer/audio_mixer_impl.h (right):

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
webrtc/modules/audio_mixer/audio_mixer_impl.h:74: void SetOutputFrequency();
I think this function should be renamed, the current name implies that it is a
setter, but it's confusing because it doesn't take an argument. Something like
CalculateOutputFrequency() would be better IMO.

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
File webrtc/modules/audio_mixer/default_output_rate_calculator.cc (right):

https://codereview.webrtc.org/2557713006/diff/20001/webrtc/modules/audio_mixe...
webrtc/modules/audio_mixer/default_output_rate_calculator.cc:25: int
maximal_frequency = 0;
How about this:
int maximal_frequency = *std::max_element(preferred_sample_rates.begin(),
preferred_sample_rates.end());

[aleloi, 2016-12-08 10:17:03]

The CQ bit was checked by aleloi@webrtc.org

[aleloi, 2016-12-08 10:17:03]

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/2557713006/#ps60001 (title:
"stl instead of loop, renamed SetOutputFrequency.")

[commit-bot: I haz the power, 2016-12-08 10:17:05]

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

[aleloi, 2016-12-08 10:17:14]

Thanks for the comments!

[commit-bot: I haz the power, 2016-12-08 10:37:58]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481192223575620,
"parent_rev": "dc90d63af81562364e227c0aee92501379126a70", "commit_rev":
"e93d7409df1e1650b10d11cc8749327592958d81"}

[commit-bot: I haz the power, 2016-12-08 10:38:01]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-12-08 10:38:12]

Description was changed from

==========
Injectable output rate calculater for AudioMixer.

This CL breaks out the output sample rate calculation from
webrtc::AudioMixerImpl. A new OutputRateCalculator interface is added
to make the sample rate configurable. There are at least three reasons
for this change:

  1. The mixer will be used for an internal project, in which no
     resampling is done after the mixing. There the sample rate should
     be static. Currently, it can differ across mix iterations and
     depends on the number of audio sources. If there are no sources,
     the WebRTC mixer behavior is to produce silence at 48 kHz.

  2. A planned change to WebRTC will make audio processing steps
     happen at constant sample rates. A configurable sample rate
     calculator will make the transition simpler for the mixer.

  3. The current mixer design is a single large file. Behavior is not
     always simple to change (e.g. as in this case to mix at a
     constant rate), unrelated behavior can be broken, reusing the
     mixer in internal projects is tricky. Using DI for the sample
     rate calculation solves parts of these issues.

Changes:

The protected mixer c-tor now takes
unique_ptr<OutputRateCalculator>. The current output rate calculation
is moved to DefaultOutputRateCalculator. A new factory method
AudioMixerImpl::CreateWithOutputRateCalculator is added. The old
factory method passes the default rate calculator.

BUG=webrtc:6346
==========

to

==========
Injectable output rate calculater for AudioMixer.

This CL breaks out the output sample rate calculation from
webrtc::AudioMixerImpl. A new OutputRateCalculator interface is added
to make the sample rate configurable. There are at least three reasons
for this change:

  1. The mixer will be used for an internal project, in which no
     resampling is done after the mixing. There the sample rate should
     be static. Currently, it can differ across mix iterations and
     depends on the number of audio sources. If there are no sources,
     the WebRTC mixer behavior is to produce silence at 48 kHz.

  2. A planned change to WebRTC will make audio processing steps
     happen at constant sample rates. A configurable sample rate
     calculator will make the transition simpler for the mixer.

  3. The current mixer design is a single large file. Behavior is not
     always simple to change (e.g. as in this case to mix at a
     constant rate), unrelated behavior can be broken, reusing the
     mixer in internal projects is tricky. Using DI for the sample
     rate calculation solves parts of these issues.

Changes:

The protected mixer c-tor now takes
unique_ptr<OutputRateCalculator>. The current output rate calculation
is moved to DefaultOutputRateCalculator. A new factory method
AudioMixerImpl::CreateWithOutputRateCalculator is added. The old
factory method passes the default rate calculator.

BUG=webrtc:6346

Committed: https://crrev.com/623427c5228050fa158671430a2949202a0d7514
Cr-Commit-Position: refs/heads/master@{#15472}
==========

[commit-bot: I haz the power, 2016-12-08 10:38:13]

Patchset 3 (id:??) landed as
https://crrev.com/623427c5228050fa158671430a2949202a0d7514
Cr-Commit-Position: refs/heads/master@{#15472}