Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2394433003: Add WebrtcAudioModule (Closed)

Created:
4 years ago by Sergey Ulanov
Modified:
4 years ago
Reviewers:
Jamie, nicholss
CC:
chromium-reviews, chromoting-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add WebrtcAudioModule Previously WebrtcTransport was using webrtc::FakeAudioModule, but it doesn't work properly when the client tries to read data from audio tracks. Replaced it with the new remoting::WebrtcAudioModule class. BUG=638505 Committed: https://crrev.com/2c5c0c20d9d7a42eabebe05ef66ab4e4bb78ed84 Cr-Commit-Position: refs/heads/master@{#423324}

Patch Set 1 #

Total comments: 6

Patch Set 2 : . #

Total comments: 10

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+622 lines, -44 lines) Patch
M remoting/protocol/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A + remoting/protocol/webrtc_audio_module.h View 1 2 5 chunks +81 lines, -41 lines 0 comments Download
A remoting/protocol/webrtc_audio_module.cc View 1 2 1 chunk +532 lines, -0 lines 0 comments Download
M remoting/protocol/webrtc_transport.h View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/protocol/webrtc_transport.cc View 3 chunks +6 lines, -3 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 14 (5 generated)
Sergey Ulanov
4 years ago (2016-10-04 19:41:36 UTC) #2
nicholss
LGTM, just a question around how this is intended to be used as a client? ...
4 years ago (2016-10-04 21:42:00 UTC) #4
Sergey Ulanov
https://codereview.chromium.org/2394433003/diff/1/remoting/protocol/webrtc_audio_module.cc File remoting/protocol/webrtc_audio_module.cc (right): https://codereview.chromium.org/2394433003/diff/1/remoting/protocol/webrtc_audio_module.cc#newcode14 remoting/protocol/webrtc_audio_module.cc:14: namespace { On 2016/10/04 21:42:00, nicholss wrote: > Is ...
4 years ago (2016-10-04 22:32:32 UTC) #5
Jamie
https://codereview.chromium.org/2394433003/diff/20001/remoting/protocol/webrtc_audio_module.cc File remoting/protocol/webrtc_audio_module.cc (right): https://codereview.chromium.org/2394433003/diff/20001/remoting/protocol/webrtc_audio_module.cc#newcode32 remoting/protocol/webrtc_audio_module.cc:32: void WebrtcAudioModule::Initialize( Having both Init() and Initialize() methods is ...
4 years ago (2016-10-04 22:57:42 UTC) #6
Sergey Ulanov
https://codereview.chromium.org/2394433003/diff/20001/remoting/protocol/webrtc_audio_module.cc File remoting/protocol/webrtc_audio_module.cc (right): https://codereview.chromium.org/2394433003/diff/20001/remoting/protocol/webrtc_audio_module.cc#newcode32 remoting/protocol/webrtc_audio_module.cc:32: void WebrtcAudioModule::Initialize( On 2016/10/04 22:57:42, Jamie wrote: > Having ...
4 years ago (2016-10-05 18:14:56 UTC) #7
Jamie
lgtm
4 years ago (2016-10-05 21:05:42 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2394433003/40001
4 years ago (2016-10-05 21:29:12 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years ago (2016-10-05 22:51:33 UTC) #12
commit-bot: I haz the power
4 years ago (2016-10-05 22:54:39 UTC) #14
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/2c5c0c20d9d7a42eabebe05ef66ab4e4bb78ed84
Cr-Commit-Position: refs/heads/master@{#423324}

Powered by Google App Engine
This is Rietveld 408576698