[Chromoting] Add AudioVolumeApplier to reduce the complexity and the dependency of kChannels

remoting/host/audio_volume_applier.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_HOST_AUDIO_VOLUME_APPLIER_H_
+#define REMOTING_HOST_AUDIO_VOLUME_APPLIER_H_
+
+#include "remoting/host/audio_silence_detector.h"
+
+namespace remoting {
+
+// A component to modify input audio sample to apply the audio level. This class
+// is used on platforms which returns non-adjusted audio samples, e.g. Windows.
+// This class supports frames with 16 bits per sample only.
+class AudioVolumeApplier {

[Sergey Ulanov, 2017/04/26 19:57:35]

AudioVolumeFilter?

[Sergey Ulanov, 2017/04/26 19:57:36]

What's the plan to integrate this class with downmixer? IMO it would be the best
to structure it as follows:
 SilenceDetector -> Downmixer -> VolumeFilter

This will allow to avoid downmixing when stream is inactive, while volume would
be applied at the end when it's cheaper. But then it doesn't make sense to put
silence detection and volume in this class. Also the interface doesn't makes it
easy to connect these parts. Potentially we could implement it as a chain of
filters, similar to how we connect InputFilter's for input pipeline - that would
allow to make each part independent and easy to connect with each other. Encoder
would be just another filter. 

[Hzj_jie, 2017/04/28 03:53:34]

Done.

[Hzj_jie, 2017/04/28 03:53:34]

IMO, downmixer should be placed out of AudioCapturer itself, but volume filter
should be part of AudioCapturer. Silence detector can be part of AudioCapturer
(as what we have now), or after downmixer (placing it in AudioPump should be a
good choice AFAICT). Because applying volume level to the audio packet captured
can make the behaviors of AudioCapturerWin and AudioCapturerLinux consistent.
And downmixing and silence detector are platform independent.

But considering current implementation, and the cost of applying volume level in
AudioCapturerWin, leaving AudioSilenceDetector in AudioCapturer (or
AudioVolumeApplier) is still a reasonable choice to trade-off between sharing
logic across platforms and consuming less processor power.

I tend to agree your opinion that checking silence after applying volume is not
necessary in most of the cases. But the downmixer looks like an O(n*m) algorithm
instead of O(n),
https://cs.chromium.org/chromium/src/media/base/channel_mixer.cc?q=media/base....
So checking for the silence after applying volume level should not be a bad
choice.

Due to the design of AudioSilenceDetector::Reset() and the
AudioSilenceDetector::silence_length_, chaining filters may not be able to work
smoothly. AudioSilenceDetector can only work if all packets go through it, and
any parameters after initialization are passed to it.

My plan is to modify AudioSource to return AudioBus instead of AudioPacket, and
down-mix it in AudioPump.

+ public:
+  // See AudioSilenceDetector for the meaning of |silence_threshold|.
+  explicit AudioVolumeApplier(int silence_threshold);
+  virtual ~AudioVolumeApplier();
+
+  // Adjusts audio samples in |data|. If the samples are silent before or after
+  // applying the volume level, this function returns false. If |frames| is 0,
+  // this function also returns false.
+  bool Apply(int16_t* data, size_t frames);
+
+  // Updates the sampling rate and channels.
+  void Initialize(int sampling_rate, int channels);
+
+ protected:
+  // Returns the volume level in [0, 1].
+  virtual float GetAudioLevel() = 0;
+
+ private:
+  bool ApplyVolumeLevel(int16_t* data, size_t frames);
+
+  AudioSilenceDetector silence_detector_before_;
+  AudioSilenceDetector silence_detector_after_;
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_AUDIO_VOLUME_APPLIER_H_