Move functionality out from AudioFrame and into AudioFrameOperations.

webrtc/modules/utility/source/audio_frame_operations.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include <algorithm>
+
 #include "webrtc/modules/include/module_common_types.h"
 #include "webrtc/modules/utility/include/audio_frame_operations.h"
 #include "webrtc/base/checks.h"
 
 namespace webrtc {
 namespace  {
 
 // 2.7ms @ 48kHz, 4ms @ 32kHz, 8ms @ 16kHz.
 const size_t kMuteFadeFrames = 128;
 const float kMuteFadeInc = 1.0f / kMuteFadeFrames;
 
 }  // namespace {
 

[aleloi, 2016/10/18 11:38:04]

All implementations are copied from AudioFrame with the following exceptions:

All asserts are replaced with RTC_DCHECKs.
memset and memcpy are changed into std::fill and std::copy.
NULL is changed into nullptr.

operator>>=(int) is renamed into ShiftDown(). We only downshifts by 1, so
ShiftDown() right-shifts by 1 and doesn't take parameters.

operator+=(frame) is renamed into AddFrames(frame_to_add, result_frame).

[the sun, 2016/10/19 09:23:17]

Can we just make it "Add()" or "AddTo()"? Or you could add a "Frames" suffix for
all of the functions...

[aleloi, 2016/10/20 08:27:06]

Done.

+void AudioFrameOperations::AddFrames(const AudioFrame& frame_to_add,
+                                     AudioFrame* result_frame) {
+  // Sanity check
+  RTC_DCHECK_GT(result_frame->num_channels_, 0u);
+  RTC_DCHECK_LT(result_frame->num_channels_, 3u);
+  if ((result_frame->num_channels_ > 2) || (result_frame->num_channels_ < 1))
+    return;
+  if (result_frame->num_channels_ != frame_to_add.num_channels_)
+    return;
+
+  bool noPrevData = false;
+  if (result_frame->samples_per_channel_ != frame_to_add.samples_per_channel_) {
+    if (result_frame->samples_per_channel_ == 0) {
+      // special case we have no data to start with
+      result_frame->samples_per_channel_ = frame_to_add.samples_per_channel_;
+      noPrevData = true;
+    } else {
+      return;
+    }
+  }
+
+  if ((result_frame->vad_activity_ == AudioFrame::kVadActive) ||
+      frame_to_add.vad_activity_ == result_frame->kVadActive) {
+    result_frame->vad_activity_ = AudioFrame::kVadActive;
+  } else if (result_frame->vad_activity_ == AudioFrame::kVadUnknown ||
+             frame_to_add.vad_activity_ == AudioFrame::kVadUnknown) {
+    result_frame->vad_activity_ = AudioFrame::kVadUnknown;
+  }
+
+  if (result_frame->speech_type_ != frame_to_add.speech_type_)
+    result_frame->speech_type_ = AudioFrame::kUndefined;
+
+  if (noPrevData) {
+    std::copy(frame_to_add.data_, frame_to_add.data_ +
+                                      frame_to_add.samples_per_channel_ *
+                                          result_frame->num_channels_,
+              result_frame->data_);
+  } else {
+    // IMPROVEMENT this can be done very fast in assembly

[the sun, 2016/10/19 09:23:17]

I think you can remove the comment

[aleloi, 2016/10/20 08:27:06]

Done.

+    for (size_t i = 0;
+         i < result_frame->samples_per_channel_ * result_frame->num_channels_;
+         i++) {
+      int32_t wrap_guard = static_cast<int32_t>(result_frame->data_[i]) +
+                           static_cast<int32_t>(frame_to_add.data_[i]);
+      result_frame->data_[i] = ClampToInt16(wrap_guard);
+    }
+  }
+  return;
+}
+
 void AudioFrameOperations::MonoToStereo(const int16_t* src_audio,
                                         size_t samples_per_channel,
                                         int16_t* dst_audio) {
   for (size_t i = 0; i < samples_per_channel; i++) {
     dst_audio[2 * i] = src_audio[i];
     dst_audio[2 * i + 1] = src_audio[i];
   }
 }
 
 int AudioFrameOperations::MonoToStereo(AudioFrame* frame) {
@@ skipping 84 matching lines @@
     for (size_t j = 0; j < channels; ++j) {
       float g = start_g;
       for (size_t i = start * channels; i < end * channels; i += channels) {
         g += inc;
         frame->data_[i + j] *= g;
       }
     }
   }
 }
 
+void AudioFrameOperations::Mute(AudioFrame* frame) {
+  Mute(frame, true, true);
+}
+
+void AudioFrameOperations::Reset(AudioFrame* frame) {
+  frame->id_ = -1;

[the sun, 2016/10/19 09:23:17]

It is dangerous to have the default values in multiple places. Is this
performance critical? (and that is why we're not zeroing data_)? If not,
default-construct an AudioFrame on the stack and assign the values from there.

[aleloi, 2016/10/20 08:27:06]

I think it might lead to a (barely noticeable) performance regression. Reset is
called in SyncBuffer::GetNextAudioInterleaved (the only non-test code that uses
::Reset), which runs every 10ms for every input channel. The values seems to
never be read anywhere, just updated with new ones. Probably everything should
still work if that Reset call is gone. I don't think it should be removed, since
we'd get a frame full of unknown values passed through a long chain of methods.

I moved Reset back to AudioFrame. Now at least the two sets of default values
are in the same file.

[the sun, 2016/10/20 08:37:57]

Acknowledged.

+  // TODO(wu): Zero is a valid value for |timestamp_|. We should initialize
+  // to an invalid value, or add a new member to indicate invalidity.
+  frame->timestamp_ = 0;
+  frame->elapsed_time_ms_ = -1;
+  frame->ntp_time_ms_ = -1;
+  frame->samples_per_channel_ = 0;
+  frame->sample_rate_hz_ = 0;
+  frame->num_channels_ = 0;
+  frame->speech_type_ = AudioFrame::kUndefined;
+  frame->vad_activity_ = AudioFrame::kVadUnknown;
+}
+
+void AudioFrameOperations::ShiftDown(AudioFrame* frame) {
+  RTC_DCHECK_GT(frame->num_channels_, 0u);
+  RTC_DCHECK_LT(frame->num_channels_, 3u);
+  if ((frame->num_channels_ > 2) || (frame->num_channels_ < 1))
+    return;
+
+  for (size_t i = 0; i < frame->samples_per_channel_ * frame->num_channels_;
+       i++) {
+    frame->data_[i] = static_cast<int16_t>(frame->data_[i] >> 1);
+  }
+}
+
 int AudioFrameOperations::Scale(float left, float right, AudioFrame& frame) {
   if (frame.num_channels_ != 2) {
     return -1;
   }
 
   for (size_t i = 0; i < frame.samples_per_channel_; i++) {
     frame.data_[2 * i] =
         static_cast<int16_t>(left * frame.data_[2 * i]);
     frame.data_[2 * i + 1] =
         static_cast<int16_t>(right * frame.data_[2 * i + 1]);
@@ skipping 12 matching lines @@
       frame.data_[i] = -32768;
     } else if (temp_data > 32767) {
       frame.data_[i] = 32767;
     } else {
       frame.data_[i] = static_cast<int16_t>(temp_data);
     }
   }
   return 0;
 }
 
+void AudioFrameOperations::UpdateFrame(int id,
+                                       uint32_t timestamp,
+                                       const int16_t* data,
+                                       size_t samples_per_channel,
+                                       int sample_rate_hz,
+                                       AudioFrame::SpeechType speech_type,
+                                       AudioFrame::VADActivity vad_activity,
+                                       size_t num_channels,
+                                       AudioFrame* frame) {
+  frame->id_ = id;
+  frame->timestamp_ = timestamp;
+  frame->samples_per_channel_ = samples_per_channel;
+  frame->sample_rate_hz_ = sample_rate_hz;
+  frame->speech_type_ = speech_type;
+  frame->vad_activity_ = vad_activity;
+  frame->num_channels_ = num_channels;
+
+  const size_t length = samples_per_channel * num_channels;
+  RTC_DCHECK_LT(length, AudioFrame::kMaxDataSizeSamples);
+  if (data != nullptr) {
+    std::copy(data, data + length, frame->data_);
+  } else {
+    std::fill(frame->data_, frame->data_ + length, 0);
+  }
+}
+
+int16_t ClampToInt16(int32_t input) {
+  if (input < -0x00008000) {
+    return -0x8000;
+  } else if (input > 0x00007FFF) {
+    return 0x7FFF;
+  } else {
+    return static_cast<int16_t>(input);
+  }
+}
+
 }  // namespace webrtc