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

Issue 41229004: Clean up LappedTransform and Blocker. (Closed)

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

Description

Clean up LappedTransform and Blocker. - Remove unnecessary window member from lapped_transform. - Add comment indicated that Blocker does not take ownership of the window passed to its constructor. - Streamline LappedTransform constructor so members can be const. Also use a range-based for loop in audio_processing_impl.cc for clarity. R=aluebs@webrtc.org, andrew@webrtc.org Committed: https://code.google.com/p/webrtc/source/detail?r=8708

Patch Set 1 #

Patch Set 2 : Fix scoped_ptr type #

Total comments: 14

Patch Set 3 : Answer comments #

Total comments: 16

Patch Set 4 : Remove default window from LappedTransform #

Total comments: 3

Patch Set 5 : Fix nit #

Patch Set 6 : There is no std::sqrtf #

Messages

Total messages: 13 (0 generated)
mgraczyk
4 years, 5 months ago (2015-03-12 00:02:50 UTC) #1
ajm
https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc File webrtc/common_audio/lapped_transform.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc#newcode71 webrtc/common_audio/lapped_transform.cc:71: window == nullptr ? block_length_ : shift_amount, &blocker_callback_)), What ...
4 years, 5 months ago (2015-03-12 01:11:25 UTC) #2
mgraczyk
https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc File webrtc/common_audio/lapped_transform.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc#newcode71 webrtc/common_audio/lapped_transform.cc:71: window == nullptr ? block_length_ : shift_amount, &blocker_callback_)), On ...
4 years, 5 months ago (2015-03-12 01:43:26 UTC) #3
ajm
https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc File webrtc/common_audio/lapped_transform.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc#newcode71 webrtc/common_audio/lapped_transform.cc:71: window == nullptr ? block_length_ : shift_amount, &blocker_callback_)), On ...
4 years, 5 months ago (2015-03-12 01:46:53 UTC) #4
aluebs
https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc File webrtc/common_audio/blocker.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc#newcode122 webrtc/common_audio/blocker.cc:122: CHECK(window); Is this check necessary? It is going to ...
4 years, 5 months ago (2015-03-12 02:07:40 UTC) #5
mgraczyk
https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc File webrtc/common_audio/blocker.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc#newcode122 webrtc/common_audio/blocker.cc:122: CHECK(window); On 2015/03/12 02:07:40, aluebs wrote: > Is this ...
4 years, 5 months ago (2015-03-12 02:38:01 UTC) #6
mgraczyk
PTAL https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc File webrtc/common_audio/lapped_transform.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/20001/webrtc/common_audio/lapped_transform.cc#newcode71 webrtc/common_audio/lapped_transform.cc:71: window == nullptr ? block_length_ : shift_amount, &blocker_callback_)), ...
4 years, 5 months ago (2015-03-12 03:58:57 UTC) #7
ajm
lgtm Just in case you weren't aware, you can run "git cl try" to run ...
4 years, 5 months ago (2015-03-12 04:26:09 UTC) #8
mgraczyk
On 2015/03/12 04:26:09, ajm wrote: > Just in case you weren't aware, you can run ...
4 years, 5 months ago (2015-03-12 04:33:41 UTC) #9
mgraczyk
https://webrtc-codereview.appspot.com/41229004/diff/60001/webrtc/common_audio/lapped_transform_unittest.cc File webrtc/common_audio/lapped_transform_unittest.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/60001/webrtc/common_audio/lapped_transform_unittest.cc#newcode133 webrtc/common_audio/lapped_transform_unittest.cc:133: std::fill(window, &window[kBlockLength], sqrtf(0.5f)); On 2015/03/12 04:26:08, ajm wrote: > ...
4 years, 5 months ago (2015-03-12 05:38:27 UTC) #10
ajm
https://webrtc-codereview.appspot.com/41229004/diff/60001/webrtc/common_audio/lapped_transform_unittest.cc File webrtc/common_audio/lapped_transform_unittest.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/60001/webrtc/common_audio/lapped_transform_unittest.cc#newcode133 webrtc/common_audio/lapped_transform_unittest.cc:133: std::fill(window, &window[kBlockLength], sqrtf(0.5f)); On 2015/03/12 05:38:27, mgraczyk wrote: > ...
4 years, 5 months ago (2015-03-12 05:48:26 UTC) #11
aluebs
lgtm https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc File webrtc/common_audio/blocker.cc (right): https://webrtc-codereview.appspot.com/41229004/diff/40001/webrtc/common_audio/blocker.cc#newcode122 webrtc/common_audio/blocker.cc:122: CHECK(window); On 2015/03/12 02:38:01, mgraczyk wrote: > On ...
4 years, 5 months ago (2015-03-12 17:36:58 UTC) #12
mgraczyk
4 years, 5 months ago (2015-03-12 23:23:55 UTC) #13
Message was sent while issue was closed.
Committed patchset #6 (id:100001) manually as 8708 (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