Adding full initial version of delay estimation functionality in echo canceller 3

webrtc/modules/audio_processing/aec3/correlator.h

+/*
+ *  Copyright (c) 2017 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.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_PROCESSING_AEC3_CORRELATOR_H_
+#define WEBRTC_MODULES_AUDIO_PROCESSING_AEC3_CORRELATOR_H_
+
+#include <memory>
+#include <vector>
+
+#include "webrtc/base/constructormagic.h"
+#include "webrtc/base/optional.h"
+#include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"

[hlundin-webrtc, 2017/02/01 08:30:00]

Fwd declare instead.

[peah-webrtc, 2017/02/02 14:04:45]

Done.

+
+namespace webrtc {
+
+// Produces recursively updated cross-correlation estimates for several signal
+// shifts where the intra-shift spacing is uniform.
+class Correlator {
+ public:
+  // Stores properties for the lag estimate corresponding to a particular signal
+  // shift.
+  struct LagEstimate {
+    LagEstimate() {}

[hlundin-webrtc, 2017/02/01 08:30:00]

Is this needed? If so, = default.

[peah-webrtc, 2017/02/02 14:04:45]

Done.

+    void Set(float accuracy, bool reliable, size_t lag, bool updated) {

[hlundin-webrtc, 2017/02/01 08:30:00]

You could probably just as well have a ctor that sets the members according to
input parameters. And then instead of my_struct.Set(a, b, c, d), use my_struct =
LagEstimate(a, b, c, d).

[peah-webrtc, 2017/02/02 14:04:45]

Done.

+      this->accuracy = accuracy;
+      this->reliable = reliable;
+      this->lag = lag;
+      this->updated = updated;
+    }
+
+    float accuracy = 0.f;
+    bool reliable = false;
+    size_t lag = 0;
+    bool updated = false;
+  };
+
+  Correlator(ApmDataDumper* data_dumper,
+             size_t window_size_sub_blocks,
+             size_t num_alignment_shifts,
+             size_t alignment_shift_sub_blocks);
+
+  ~Correlator();
+
+  // Updates the correlation with the values in x and y.

[hlundin-webrtc, 2017/02/01 08:30:00]

No x and y in parameter list.

[peah-webrtc, 2017/02/02 14:04:45]

Done.

+  void Update(rtc::ArrayView<const float> render,

[hlundin-webrtc, 2017/02/01 08:30:00]

This class seems to be agnostic of the concepts render and capture. Stick to
something more generic, like x and y :)

[peah-webrtc, 2017/02/02 14:04:45]

It is not in that it requires a certain causality between x and y. I'd therefore
rather stick with render and capture, as that quite well describes that
relationship, and instead in the implementation replace these with x and y.

WDYT?

[hlundin-webrtc, 2017/02/06 09:13:18]

Acknowledged.

[peah-webrtc, 2017/02/06 11:25:38]

Acknowledged.

+              rtc::ArrayView<const float> capture);
+
+  // Returns the current lag estimates.
+  rtc::ArrayView<const Correlator::LagEstimate> GetLagEstimates() const {
+    return lag_estimates_;
+  }
+
+  // Returns the number of lag estimates produced using the shifted signals.
+  size_t NumLagEstimates() const { return correlations_.size(); }
+
+ private:
+  // Provides buffer with a related index.
+  struct IndexedBuffer {
+   public:

[hlundin-webrtc, 2017/02/01 08:30:01]

public not needed in struct.

[peah-webrtc, 2017/02/02 14:04:45]

Done.

+    explicit IndexedBuffer(size_t size);
+    ~IndexedBuffer();
+
+    std::vector<float> data;
+    int index = 0;
+    RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(IndexedBuffer);
+  };
+
+  ApmDataDumper* const data_dumper_;
+  const size_t correlator_intra_lag_shift_;
+  std::vector<std::vector<float>> correlations_;
+  std::vector<LagEstimate> lag_estimates_;
+  IndexedBuffer render_buffer_;
+
+  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(Correlator);
+};
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_PROCESSING_AEC3_CORRELATOR_H_