AudioMixer interface cleanup suggestions

webrtc/modules/audio_mixer/audio_frame_manipulator.cc

Index: webrtc/modules/audio_mixer/audio_frame_manipulator.cc
diff --git a/webrtc/modules/audio_mixer/audio_frame_manipulator.cc b/webrtc/modules/audio_mixer/audio_frame_manipulator.cc
index aafeedaeda9748100fe98b49dba72c516234ba19..649007dd5d10e91d4322f7a7ff2a97e04601324e 100644
--- a/webrtc/modules/audio_mixer/audio_frame_manipulator.cc
+++ b/webrtc/modules/audio_mixer/audio_frame_manipulator.cc
@@ -14,6 +14,9 @@
 
 namespace webrtc {
 namespace {
+// !!!: This array is likely a pessimisation. It's a linear ramp, so a simple
+// add with a constant would do in the ramp loop. Here we instead touch 320
+// bytes of memory, or 10 cache lines for 32B/each.
 // Linear ramping over 80 samples.
 // TODO(hellner): ramp using fix point?
 const float kRampArray[] = {
@@ -40,7 +43,10 @@ uint32_t NewMixerCalculateEnergy(const AudioFrame& audio_frame) {
   return energy;
 }
 
+// !!!: "New" in function names, is rarely a good idea. Things tend to not stay
+// new for very long...
 void NewMixerRampIn(AudioFrame* audio_frame) {
+  // !!!: It is also a prerequisite that there is just one channel.
   assert(kRampSize <= audio_frame->samples_per_channel_);
   for (size_t i = 0; i < kRampSize; i++) {
     audio_frame->data_[i] =
@@ -52,9 +58,11 @@ void NewMixerRampOut(AudioFrame* audio_frame) {
   assert(kRampSize <= audio_frame->samples_per_channel_);
   for (size_t i = 0; i < kRampSize; i++) {
     const size_t kRampPos = kRampSize - 1 - i;
+    // !!!: Reading data backwards? I wonder how hw prefetchers handle that?
     audio_frame->data_[i] =
         static_cast<int16_t>(kRampArray[kRampPos] * audio_frame->data_[i]);
   }
+  // !!!: Or ramp the end of the frame...
   memset(&audio_frame->data_[kRampSize], 0,
          (audio_frame->samples_per_channel_ - kRampSize) *
              sizeof(audio_frame->data_[0]));