From 3bfafb5cab6889b230e194c347cd9b1fffee921d Mon Sep 17 00:00:00 2001
From: Sam Zackrisson <saza@webrtc.org>
Date: Mon, 1 Aug 2022 11:30:48 +0200
Subject: [PATCH] Reenable WebRTC PushResampler format checks on Windows clang
 debug builds

The referenced bug is fixed and issues like this haven't been observed for a long time.

Follow up CL to https://webrtc-review.googlesource.com/c/src/+/268769/comments/8777e26e_0356bffb

Bug: chromium:615050
Change-Id: I8f9e5db5a1b9c787867598e973a367a5c5e367df
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269761
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37699}
---
 common_audio/resampler/push_resampler.cc      | 50 ++++---------------
 .../resampler/push_resampler_unittest.cc      |  7 ---
 2 files changed, 11 insertions(+), 46 deletions(-)

diff --git a/common_audio/resampler/push_resampler.cc b/common_audio/resampler/push_resampler.cc
index 837fec624a..810d778993 100644
--- a/common_audio/resampler/push_resampler.cc
+++ b/common_audio/resampler/push_resampler.cc
@@ -20,42 +20,6 @@
 #include "rtc_base/checks.h"
 
 namespace webrtc {
-namespace {
-// These checks were factored out into a non-templatized function
-// due to problems with clang on Windows in debug builds.
-// For some reason having the DCHECKs inline in the template code
-// caused the compiler to generate code that threw off the linker.
-// TODO(tommi): Re-enable when we've figured out what the problem is.
-// http://crbug.com/615050
-void CheckValidInitParams(int src_sample_rate_hz,
-                          int dst_sample_rate_hz,
-                          size_t num_channels) {
-// The below checks are temporarily disabled on WEBRTC_WIN due to problems
-// with clang debug builds.
-#if !defined(WEBRTC_WIN) && defined(__clang__)
-  RTC_DCHECK_GT(src_sample_rate_hz, 0);
-  RTC_DCHECK_GT(dst_sample_rate_hz, 0);
-  RTC_DCHECK_GT(num_channels, 0);
-#endif
-}
-
-void CheckExpectedBufferSizes(size_t src_length,
-                              size_t dst_capacity,
-                              size_t num_channels,
-                              int src_sample_rate,
-                              int dst_sample_rate) {
-// The below checks are temporarily disabled on WEBRTC_WIN due to problems
-// with clang debug builds.
-// TODO(tommi): Re-enable when we've figured out what the problem is.
-// http://crbug.com/615050
-#if !defined(WEBRTC_WIN) && defined(__clang__)
-  const size_t src_size_10ms = (src_sample_rate / 100) * num_channels;
-  const size_t dst_size_10ms = (dst_sample_rate / 100) * num_channels;
-  RTC_DCHECK_EQ(src_length, src_size_10ms);
-  RTC_DCHECK_GE(dst_capacity, dst_size_10ms);
-#endif
-}
-}  // namespace
 
 template <typename T>
 PushResampler<T>::PushResampler()
@@ -68,7 +32,11 @@ template <typename T>
 int PushResampler<T>::InitializeIfNeeded(int src_sample_rate_hz,
                                          int dst_sample_rate_hz,
                                          size_t num_channels) {
-  CheckValidInitParams(src_sample_rate_hz, dst_sample_rate_hz, num_channels);
+  // These checks used to be factored out of this template function due to
+  // Windows debug build issues with clang. http://crbug.com/615050
+  RTC_DCHECK_GT(src_sample_rate_hz, 0);
+  RTC_DCHECK_GT(dst_sample_rate_hz, 0);
+  RTC_DCHECK_GT(num_channels, 0);
 
   if (src_sample_rate_hz == src_sample_rate_hz_ &&
       dst_sample_rate_hz == dst_sample_rate_hz_ &&
@@ -109,8 +77,12 @@ int PushResampler<T>::Resample(const T* src,
                                size_t src_length,
                                T* dst,
                                size_t dst_capacity) {
-  CheckExpectedBufferSizes(src_length, dst_capacity, num_channels_,
-                           src_sample_rate_hz_, dst_sample_rate_hz_);
+  // These checks used to be factored out of this template function due to
+  // Windows debug build issues with clang. http://crbug.com/615050
+  const size_t src_size_10ms = (src_sample_rate_hz_ / 100) * num_channels_;
+  const size_t dst_size_10ms = (dst_sample_rate_hz_ / 100) * num_channels_;
+  RTC_DCHECK_EQ(src_length, src_size_10ms);
+  RTC_DCHECK_GE(dst_capacity, dst_size_10ms);
 
   if (src_sample_rate_hz_ == dst_sample_rate_hz_) {
     // The old resampler provides this memcpy facility in the case of matching
diff --git a/common_audio/resampler/push_resampler_unittest.cc b/common_audio/resampler/push_resampler_unittest.cc
index f785ed92f0..91f2233aad 100644
--- a/common_audio/resampler/push_resampler_unittest.cc
+++ b/common_audio/resampler/push_resampler_unittest.cc
@@ -18,11 +18,6 @@
 
 namespace webrtc {
 
-// The below tests are temporarily disabled on WEBRTC_WIN due to problems
-// with clang debug builds.
-// TODO(tommi): Re-enable when we've figured out what the problem is.
-// http://crbug.com/615050
-#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
 TEST(PushResamplerTest, VerifiesInputParameters) {
   PushResampler<int16_t> resampler;
   EXPECT_EQ(0, resampler.InitializeIfNeeded(16000, 16000, 1));
@@ -48,8 +43,6 @@ TEST(PushResamplerDeathTest, VerifiesBadInputParameters3) {
   RTC_EXPECT_DEATH(resampler.InitializeIfNeeded(16000, 16000, 0),
                    "num_channels");
 }
-
-#endif
 #endif
 
 }  // namespace webrtc