WebRTC Code Reviews
Help | Chromium Project | Sign in
(1363)

Issue 36689004: Add WebRtcIsacfix_AllpassFilter2FixDec16Neon()'s intrinsics version. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by zhongwei.yao
Modified:
4 years, 3 months ago
Reviewers:
jridges, ajm
CC:
webrtc-reviews_webrtc.org, tterriberry, HL, kwiberg, Tina
Base URL:
https://chromium.googlesource.com/external/webrtc@master
Project:
webrtc
Visibility:
Public.

Description

Add WebRtcIsacfix_AllpassFilter2FixDec16Neon()'s intrinsics version. This intrinsics version gives bit-exact result as the current C code. And the performance is 14% better than current assembly neon version, 3.4 times faster than current C version. The test runs under Cortex-a53 aarch32 mode, other cpu should give similar performance result. Change-Id: Icce5eaf2e17790ce44513d52b53b9f600cc16f96 BUG=4002 R=andrew@webrtc.org, jridges@masque.com Committed: https://code.google.com/p/webrtc/source/detail?r=8070

Patch Set 1 #

Total comments: 1

Patch Set 2 : Update comments. #

Total comments: 5

Patch Set 3 : update according to comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+275 lines, -0 lines) Patch
A webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c View 1 2 1 chunk +275 lines, -0 lines 0 comments Download
Trybot results:
Project "webrtc" does not have a commit queue.

Messages

Total messages: 12 (0 generated)
zhongwei.yao
4 years, 3 months ago (2015-01-12 07:35:33 UTC) #1
ajm
https://webrtc-codereview.appspot.com/36689004/diff/1/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c (right): https://webrtc-codereview.appspot.com/36689004/diff/1/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode13 webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c:13: // WebRtcIsacfix_AllpassFilter2FixDec16Neon() in filterbanks.c. Prototype Dec16C, right? Can you ...
4 years, 3 months ago (2015-01-12 19:43:34 UTC) #2
zhongwei.yao
On 2015/01/12 19:43:34, ajm wrote: > https://webrtc-codereview.appspot.com/36689004/diff/1/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c > File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c > (right): > > https://webrtc-codereview.appspot.com/36689004/diff/1/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode13 ...
4 years, 3 months ago (2015-01-13 04:37:41 UTC) #3
ajm
lgtm, though I'd like jridges to review. On 2015/01/13 04:37:41, zhongwei.yao wrote: > I think ...
4 years, 3 months ago (2015-01-13 06:06:49 UTC) #4
jridges
https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c (right): https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode35 webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c:35: tmp = vld1_dup_s32((int32_t*)factor_ch1); Is there any potential problem (aside ...
4 years, 3 months ago (2015-01-13 17:37:31 UTC) #5
jridges
https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c (right): https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode54 webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c:54: a = vmull_s16(tmp1, factorv_neg); If you used vqdmull_s16 instead ...
4 years, 3 months ago (2015-01-13 23:00:52 UTC) #6
jridges
https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c (right): https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode51 webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c:51: a = vqdmull_s16(datav, factorv); Sorry about all the separate ...
4 years, 3 months ago (2015-01-14 00:20:50 UTC) #7
zhongwei.yao
https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c File webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c (right): https://webrtc-codereview.appspot.com/36689004/diff/20001/webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c#newcode35 webrtc/modules/audio_coding/codecs/isac/fix/source/filterbanks_neon.c:35: tmp = vld1_dup_s32((int32_t*)factor_ch1); On 2015/01/13 17:37:31, jridges wrote: > ...
4 years, 3 months ago (2015-01-14 12:34:44 UTC) #8
jridges
Very nice. Please update the description to reflect the new performance. Otherwise LGTM On 2015/01/14 ...
4 years, 3 months ago (2015-01-14 16:17:28 UTC) #9
ajm
Let me know when the description is updated Zhongwei, and I'll land this. Nice work.
4 years, 3 months ago (2015-01-14 18:18:41 UTC) #10
zhongwei.yao
On 2015/01/14 18:18:41, ajm wrote: > Let me know when the description is updated Zhongwei, ...
4 years, 3 months ago (2015-01-15 02:33:39 UTC) #11
ajm
4 years, 3 months ago (2015-01-15 02:56:16 UTC) #12
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 8070 (presubmit successful).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted