 Chromium Code Reviews
 Chromium Code Reviews Issue 1908623002:
  Avoiding overflow in cross correlation in NetEq.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/webrtc.git@master
    
  
    Issue 1908623002:
  Avoiding overflow in cross correlation in NetEq.  (Closed) 
  Base URL: https://chromium.googlesource.com/external/webrtc.git@master| OLD | NEW | 
|---|---|
| (Empty) | |
| 1 /* | |
| 2 * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. | |
| 3 * | |
| 4 * Use of this source code is governed by a BSD-style license | |
| 5 * that can be found in the LICENSE file in the root of the source | |
| 6 * tree. An additional intellectual property rights grant can be found | |
| 7 * in the file PATENTS. All contributing project authors may | |
| 8 * be found in the AUTHORS file in the root of the source tree. | |
| 9 */ | |
| 10 | |
| 11 #include "webrtc/modules/audio_coding/neteq/cross_correlation.h" | |
| 12 | |
| 13 #include <assert.h> | |
| 
hlundin-webrtc
2016/04/25 07:53:57
Use DCHECK instead of assert, and include base/web
 
minyue-webrtc
2016/04/27 10:38:03
will remove
 | |
| 14 #include <limits> | |
| 
hlundin-webrtc
2016/04/25 07:53:57
Order of includes is wrong.
 
minyue-webrtc
2016/04/27 10:38:03
Done.
 | |
| 15 #include <cmath> | |
| 16 | |
| 17 namespace webrtc { | |
| 18 | |
| 19 // This function decides the overflow-protecting scaling and calls | |
| 20 // WebRtcSpl_CrossCorrelation. | |
| 21 int CrossCorrelationWithAutoShift(int32_t* cross_correlation, | |
| 22 const int16_t* sequence_1, | |
| 23 const int16_t* sequence_2, | |
| 24 size_t sequence_1_length, | |
| 25 size_t cross_correlation_length, | |
| 26 int cross_correlation_step) { | |
| 27 // Find the maximum absolute value of sequence_1 and 2. | |
| 28 const int16_t max_1 = WebRtcSpl_MaxAbsValueW16(sequence_1, sequence_1_length); | |
| 29 const int sequence_2_shift = | |
| 30 cross_correlation_step * (cross_correlation_length - 1); | |
| 31 const int16_t* sequence_2_start = | |
| 32 sequence_2_shift >= 0 ? sequence_2 : sequence_2 + sequence_2_shift; | |
| 33 const size_t sequence_2_length = | |
| 34 sequence_1_length + std::abs(sequence_2_shift); | |
| 35 const int16_t max_2 = | |
| 36 WebRtcSpl_MaxAbsValueW16(sequence_2_start, sequence_2_length); | |
| 37 | |
| 38 // In order to avoid overflow when computing the sum we should scale the | |
| 39 // samples so that (in_vector_length * max_1 * max_2) will not overflow. | |
| 40 // Expected scaling fulfills | |
| 41 // 1) sufficient: | |
| 42 // sequence_1_length * (max_1 * max_2 >> scaling) <= 0x7fffffff; | |
| 43 // 2) necessary: | |
| 44 // if (scaling > 0) | |
| 45 // sequence_1_length * (max_1 * max_2 >> (scaling - 1)) > 0x7fffffff; | |
| 46 // The following calculation fulfills 1) and almost fulfills 2). | |
| 47 // There are some corner cases that 2) is not satisfied, e.g., | |
| 48 // max_1 = 17, max_2 = 30848, sequence_1_length = 4095, in such case, | |
| 49 // optimal scaling is 0, while the following calculation results in 1. | |
| 50 const int32_t factor = (max_1 * max_2) / (std::numeric_limits<int32_t>::max() | |
| 51 / static_cast<int32_t>(sequence_1_length)); | |
| 52 const int scaling = factor == 0 ? 0 : 31 - WebRtcSpl_NormW32(factor); | |
| 53 | |
| 54 assert((double)max_1 * max_2 * sequence_1_length / (1 << scaling) <= | |
| 
hlundin-webrtc
2016/04/25 07:53:57
RTC_DCHECK_LE
 
minyue-webrtc
2016/04/27 10:38:03
I plan to remove these two lines. I've done an exh
 | |
| 55 std::numeric_limits<int32_t>::max()); | |
| 56 assert(scaling == 0 || | |
| 
hlundin-webrtc
2016/04/25 07:53:57
RTC_DCHECK
 
minyue-webrtc
2016/04/27 10:38:03
also plan to remove
 | |
| 57 (double)max_1 * max_2 * sequence_1_length /(1 << scaling) * 2 > | |
| 58 std::numeric_limits<int32_t>::max()); | |
| 59 | |
| 60 WebRtcSpl_CrossCorrelation(cross_correlation, sequence_1, sequence_2, | |
| 61 sequence_1_length, cross_correlation_length, | |
| 62 scaling, cross_correlation_step); | |
| 63 | |
| 64 return scaling; | |
| 65 } | |
| 66 | |
| 67 } // namespace webrtc | |
| OLD | NEW |