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

Issue 57579004: Allow intelligibility to compile in apm

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by ekm
Modified:
4 years, 2 months ago
Reviewers:
Turaj, aluebs, ajm
CC:
webrtc-reviews_webrtc.org, ajm, tterriberry, HL, kwiberg, aluebs, Bjorn
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Allow intelligibility to compile in apm - Added files to gyp and BUILD - Made minor fixes to get everything to compile and intelligibility_proc to run - Added comments - Auto-reformatting

Patch Set 1 #

Total comments: 50

Patch Set 2 : Addressed comments #

Total comments: 4

Patch Set 3 : Added TODO #

Total comments: 4

Patch Set 4 : Fixed whitespace #

Unified diffs Side-by-side diffs Delta from patch set Stats (+337 lines, -206 lines) Patch
M webrtc/modules/audio_processing/BUILD.gn View 1 chunk +4 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/audio_processing.gypi View 1 1 chunk +4 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/audio_processing_tests.gypi View 1 chunk +13 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h View 1 2 3 4 chunks +75 lines, -34 lines 0 comments Download
M webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc View 1 17 chunks +102 lines, -80 lines 0 comments Download
M webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc View 1 7 chunks +73 lines, -39 lines 0 comments Download
M webrtc/modules/audio_processing/intelligibility/intelligibility_utils.h View 1 2 5 chunks +22 lines, -22 lines 0 comments Download
M webrtc/modules/audio_processing/intelligibility/intelligibility_utils.cc View 1 10 chunks +44 lines, -31 lines 0 comments Download
Trybot results: Sign in to try more bots
Project "webrtc" does not have a commit queue.

Messages

Total messages: 12 (2 generated)
ekm
4 years, 2 months ago (2015-06-16 01:25:21 UTC) #2
aluebs
Awesome documentation! :D lgtm after comments are addressed. https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/audio_processing.gypi File webrtc/modules/audio_processing/audio_processing.gypi (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/audio_processing.gypi#newcode110 webrtc/modules/audio_processing/audio_processing.gypi:110: 'intelligibility/intelligibility_enhancer.cc', ...
4 years, 2 months ago (2015-06-16 01:50:25 UTC) #3
ajm
Minor stuff. https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc File webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc#newcode46 webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc:46: // TODO(ekmeyerson) add option to disable gain ...
4 years, 2 months ago (2015-06-16 05:44:35 UTC) #4
aluebs
https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc File webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc#newcode41 webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc:41: using rtc::scoped_ptr; On 2015/06/16 05:44:34, ajm wrote: > On ...
4 years, 2 months ago (2015-06-16 15:26:42 UTC) #5
ekm
https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/audio_processing.gypi File webrtc/modules/audio_processing/audio_processing.gypi (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/audio_processing.gypi#newcode110 webrtc/modules/audio_processing/audio_processing.gypi:110: 'intelligibility/intelligibility_enhancer.cc', On 2015/06/16 01:50:24, aluebs wrote: > Alphabetical order. ...
4 years, 2 months ago (2015-06-16 22:31:38 UTC) #6
ajm
lgtm with minor comments. https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h File webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h#newcode13 webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h:13: */ On 2015/06/16 22:31:37, ekm ...
4 years, 2 months ago (2015-06-16 22:45:30 UTC) #7
ekm
https://webrtc-codereview.appspot.com/57579004/diff/20001/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc File webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc (right): https://webrtc-codereview.appspot.com/57579004/diff/20001/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc#newcode175 webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.cc:175: // no matter what the aggressiveness, so it was ...
4 years, 2 months ago (2015-06-16 23:10:08 UTC) #9
aluebs
lgtm https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc File webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc (right): https://webrtc-codereview.appspot.com/57579004/diff/1/webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc#newcode41 webrtc/modules/audio_processing/intelligibility/intelligibility_proc.cc:41: using rtc::scoped_ptr; On 2015/06/16 22:31:37, ekm wrote: > ...
4 years, 2 months ago (2015-06-16 23:23:39 UTC) #10
Turaj
LGTM with minor nits. https://webrtc-codereview.appspot.com/57579004/diff/60001/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h File webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h (right): https://webrtc-codereview.appspot.com/57579004/diff/60001/webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h#newcode65 webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h:65: kCaptureStream, // Noise stream. two ...
4 years, 2 months ago (2015-06-16 23:31:17 UTC) #11
ekm
4 years, 2 months ago (2015-06-17 00:10:54 UTC) #12
https://webrtc-codereview.appspot.com/57579004/diff/60001/webrtc/modules/audi...
File webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h
(right):

https://webrtc-codereview.appspot.com/57579004/diff/60001/webrtc/modules/audi...
webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h:65:
kCaptureStream,     // Noise stream.
On 2015/06/16 23:31:17, Turaj wrote:
> two spaces before '//' (I aggree that it looks prettier the way you have it)

Done.

https://webrtc-codereview.appspot.com/57579004/diff/60001/webrtc/modules/audi...
webrtc/modules/audio_processing/intelligibility/intelligibility_enhancer.h:145:
float** filter_bank_; // TODO(ekmeyerson): Switch to using ChannelBuffer.
On 2015/06/16 23:31:17, Turaj wrote:
> two spaces, please, and further down.

Done.
Sign in to reply to this message.

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