From bc46bf22e776d2f1372c6e79e1f9b4bdb7edd17b Mon Sep 17 00:00:00 2001 From: Bjorn Volcker Date: Mon, 30 Mar 2015 23:38:28 +0200 Subject: [PATCH] common_audio: Explicit cast in WebRtcSpl_NormW16 on ARM We currently hit asserts in AECM where the output of WebRtcSpl_NormW16() on armv7 is incorrect. I've verified that it outputs -17 for negative values. Internally that means that clz returns 0 after a two's complement operation on a int16_t. There is a mismatch between the int16_t input and otherwise 32 bit assumptions. Explicitly casting to int32_t makes the two's complement do the correct thing. The CL also extends the unit tests by running through a larger set of values. BUG=4486 TESTED=locally on Android Nexus 7 and trybots R=aluebs@webrtc.org, kwiberg@webrtc.org Review URL: https://webrtc-codereview.appspot.com/49549004 Cr-Commit-Position: refs/heads/master@{#8897} --- .../signal_processing/include/spl_inl_armv7.h | 9 +++++---- .../signal_processing/signal_processing_unittest.cc | 5 +++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/webrtc/common_audio/signal_processing/include/spl_inl_armv7.h b/webrtc/common_audio/signal_processing/include/spl_inl_armv7.h index 0d10129a22..2718801159 100644 --- a/webrtc/common_audio/signal_processing/include/spl_inl_armv7.h +++ b/webrtc/common_audio/signal_processing/include/spl_inl_armv7.h @@ -110,15 +110,16 @@ static __inline int16_t WebRtcSpl_NormU32(uint32_t a) { static __inline int16_t WebRtcSpl_NormW16(int16_t a) { int32_t tmp = 0; + int32_t a_32 = a; - if (a == 0) { + if (a_32 == 0) { return 0; } - else if (a < 0) { - a ^= 0xFFFFFFFF; + else if (a_32 < 0) { + a_32 ^= 0xFFFFFFFF; } - __asm __volatile ("clz %0, %1":"=r"(tmp):"r"(a)); + __asm __volatile ("clz %0, %1":"=r"(tmp):"r"(a_32)); return (int16_t)(tmp - 17); } diff --git a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc index 305789e2aa..12b2fffbee 100644 --- a/webrtc/common_audio/signal_processing/signal_processing_unittest.cc +++ b/webrtc/common_audio/signal_processing/signal_processing_unittest.cc @@ -106,6 +106,11 @@ TEST_F(SplTest, InlineTest) { EXPECT_EQ(15, WebRtcSpl_NormW16(-1)); EXPECT_EQ(0, WebRtcSpl_NormW16(WEBRTC_SPL_WORD16_MIN)); EXPECT_EQ(4, WebRtcSpl_NormW16(b32)); + for (int ii = 0; ii < 15; ++ii) { + int16_t value = 1 << ii; + EXPECT_EQ(14 - ii, WebRtcSpl_NormW16(value)); + EXPECT_EQ(15 - ii, WebRtcSpl_NormW16(-value)); + } EXPECT_EQ(0, WebRtcSpl_NormU32(0u)); EXPECT_EQ(0, WebRtcSpl_NormU32(0xffffffff));