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

Issue 13049004: Refactor audio conversion functions. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 3 months ago by ajm
Modified:
4 years, 11 months ago
Reviewers:
kwiberg, aluebs, Bjorn
CC:
webrtc-reviews_webrtc.org, rillian-moz, tterriberry, leozwang1
Base URL:
https://webrtc.googlecode.com/svn/trunk
Visibility:
Public.

Description

Refactor audio conversion functions. Use a consistent naming scheme that can be understood at the callsite without having to refer to documentation. Remove hacks in AudioBuffer intended to maintain bit-exactness with the float path. The conversions etc. are now all natural, and instead we enforce close but not bit-exact output between the two paths. Output of ApmTest.Process: https://paste.googleplex.com/5931055831842816 R=aluebs@webrtc.org, bjornv@webrtc.org, kwiberg@webrtc.org Committed: https://code.google.com/p/webrtc/source/detail?r=7561

Patch Set 1 #

Total comments: 11

Patch Set 2 : Big changes. #

Patch Set 3 : Cleanups. #

Patch Set 4 : Change reference data. #

Total comments: 9

Patch Set 5 : Rename. #

Patch Set 6 : Fix comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+155 lines, -96 lines) Patch
M data/audio_processing/output_data_fixed.pb View 1 2 3 Binary file 0 comments Download
M data/audio_processing/output_data_float.pb View 1 2 3 Binary file 0 comments Download
M webrtc/common_audio/audio_util.cc View 1 2 3 4 1 chunk +16 lines, -6 lines 0 comments Download
M webrtc/common_audio/audio_util_unittest.cc View 1 2 3 4 1 chunk +36 lines, -12 lines 0 comments Download
M webrtc/common_audio/include/audio_util.h View 1 2 3 4 5 2 chunks +31 lines, -23 lines 0 comments Download
M webrtc/common_audio/resampler/push_sinc_resampler.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M webrtc/common_audio/resampler/push_sinc_resampler_unittest.cc View 1 2 3 4 1 chunk +4 lines, -5 lines 0 comments Download
M webrtc/common_audio/wav_writer.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/audio_processing/audio_buffer.cc View 1 2 3 4 5 chunks +10 lines, -28 lines 0 comments Download
M webrtc/modules/audio_processing/test/audio_processing_unittest.cc View 1 2 3 4 7 chunks +33 lines, -20 lines 0 comments Download
M webrtc/modules/audio_processing/test/test_utils.h View 1 2 2 chunks +23 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
ajm
https://webrtc-codereview.appspot.com/13049004/diff/20002/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://webrtc-codereview.appspot.com/13049004/diff/20002/webrtc/common_audio/include/audio_util.h#newcode60 webrtc/common_audio/include/audio_util.h:60: void ScaleToInt16Range(const float* src, size_t size, float* dest); This ...
5 years, 3 months ago (2014-07-19 01:59:01 UTC) #1
kwiberg
Looks worthwhile, but I wonder if it wouldn't make sense to just take care of ...
5 years, 2 months ago (2014-07-21 20:36:22 UTC) #2
ajm
Karl: Coming back to this at long last, because I actually need it now. I'm ...
4 years, 11 months ago (2014-10-27 23:54:53 UTC) #3
aluebs
lgtm % one question https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode65 webrtc/common_audio/include/audio_util.h:65: void F32Q15ToS16(const float* src, size_t ...
4 years, 11 months ago (2014-10-28 00:22:26 UTC) #4
ajm
https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode65 webrtc/common_audio/include/audio_util.h:65: void F32Q15ToS16(const float* src, size_t size, int16_t* dest); On ...
4 years, 11 months ago (2014-10-28 00:38:16 UTC) #5
kwiberg
lgtm, though I'm not a huge fan of either your names or the replacements I ...
4 years, 11 months ago (2014-10-28 00:55:23 UTC) #6
ajm
https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode26 webrtc/common_audio/include/audio_util.h:26: // F32Q15: float, [-32768, 32767] On 2014/10/28 00:55:23, kwiberg ...
4 years, 11 months ago (2014-10-28 05:39:43 UTC) #7
Bjorn
naming lgtm https://webrtc-codereview.appspot.com/13049004/diff/150001/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://webrtc-codereview.appspot.com/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode26 webrtc/common_audio/include/audio_util.h:26: // F32Q15: float, [-32768, 32767] On 2014/10/28 ...
4 years, 11 months ago (2014-10-28 14:26:26 UTC) #8
ajm
https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h File webrtc/common_audio/include/audio_util.h (right): https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode26 webrtc/common_audio/include/audio_util.h:26: // F32Q15: float, [-32768, 32767] On 2014/10/28 14:26:26, Bjorn ...
4 years, 11 months ago (2014-10-28 18:14:07 UTC) #9
kwiberg
On 2014/10/28 18:14:07, ajm wrote: > https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h > File webrtc/common_audio/include/audio_util.h (right): > > https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode26 > ...
4 years, 11 months ago (2014-10-29 12:09:21 UTC) #10
Bjorn
On 2014/10/28 18:14:07, ajm wrote: > https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h > File webrtc/common_audio/include/audio_util.h (right): > > https://review.webrtc.org/13049004/diff/150001/webrtc/common_audio/include/audio_util.h#newcode26 > ...
4 years, 11 months ago (2014-10-29 12:31:51 UTC) #11
ajm
On 2014/10/29 12:31:51, Bjorn wrote: > Alternative 1 is my vote. I like s16 is ...
4 years, 11 months ago (2014-10-30 03:37:40 UTC) #12
ajm
4 years, 11 months ago (2014-10-30 03:40:17 UTC) #13
Message was sent while issue was closed.
Committed patchset #6 (id:190001) manually as 7561 (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