Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(188)

Issue 1269863005: MediaController/Call instantiation. (Closed)

Created:
5 years, 4 months ago by the sun
Modified:
5 years, 3 months ago
Reviewers:
pbos-webrtc, pthatcher1
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), Andrew MacDonald, kwiberg-webrtc, tlegrand-webrtc
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Move instantiation of webrtc::Call into a MediaController class so that it can be used for both audio and video media channels. I'm not super happy with the GetVoE() function added on MediaEngineInterface, but this will eventually be gone, once webrtc::Call owns the shared VoE state (or initially, maps ADM* to an implicitly created VoE). BUG=webrtc:4690 R=pbos@webrtc.org, pthatcher@webrtc.org Committed: https://crrev.com/709ed67c38d0a942f3bf3e68e337cc27a27bc353 Cr-Commit-Position: refs/heads/master@{#9939}

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Rebase+tests #

Patch Set 4 : Added MediaController class #

Patch Set 5 : Get VoE* for Call::Config #

Patch Set 6 : merge #

Patch Set 7 : Merge with per-stream-transports branch #

Patch Set 8 : rebase #

Patch Set 9 : more #

Patch Set 10 : rebase #

Patch Set 11 : nullptr #

Total comments: 21

Patch Set 12 : rebase+comments #

Patch Set 13 : Misplaced assertion #

Total comments: 5

Patch Set 14 : Merge with loadobserver change #

Patch Set 15 : comments #

Patch Set 16 : rebase+comments #

Total comments: 10

Patch Set 17 : comments #

Total comments: 9

Patch Set 18 : rebase+comments #

Patch Set 19 : .cc internals -> anonymous namespace #

Patch Set 20 : rebase #

Patch Set 21 : rebase #

Patch Set 22 : comment #

Patch Set 23 : rebase #

Patch Set 24 : Remove redundant reset(nullptr) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+380 lines, -466 lines) Patch
M talk/app/webrtc/mediacontroller.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +22 lines, -2 lines 0 comments Download
M talk/app/webrtc/mediacontroller.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +59 lines, -3 lines 0 comments Download
M talk/app/webrtc/webrtcsession.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 3 chunks +2 lines, -1 line 0 comments Download
M talk/app/webrtc/webrtcsession.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 6 chunks +8 lines, -6 lines 0 comments Download
M talk/libjingle.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -0 lines 0 comments Download
M talk/media/base/fakemediaengine.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 5 chunks +6 lines, -7 lines 0 comments Download
M talk/media/base/mediachannel.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +0 lines, -2 lines 0 comments Download
M talk/media/base/mediaengine.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 5 chunks +21 lines, -11 lines 0 comments Download
M talk/media/base/videoengine_unittest.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 5 chunks +9 lines, -2 lines 0 comments Download
M talk/media/webrtc/webrtcvideoengine2.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 6 chunks +8 lines, -30 lines 0 comments Download
M talk/media/webrtc/webrtcvideoengine2.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 9 chunks +12 lines, -63 lines 0 comments Download
M talk/media/webrtc/webrtcvideoengine2_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 21 chunks +36 lines, -73 lines 0 comments Download
M talk/media/webrtc/webrtcvoiceengine.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 6 chunks +10 lines, -8 lines 0 comments Download
M talk/media/webrtc/webrtcvoiceengine.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 13 chunks +39 lines, -53 lines 0 comments Download
M talk/media/webrtc/webrtcvoiceengine_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 24 chunks +65 lines, -130 lines 0 comments Download
M talk/session/media/channelmanager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +31 lines, -27 lines 0 comments Download
M talk/session/media/channelmanager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +25 lines, -38 lines 0 comments Download
M talk/session/media/channelmanager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 6 chunks +25 lines, -10 lines 0 comments Download

Messages

Total messages: 26 (3 generated)
the sun
5 years, 3 months ago (2015-08-31 10:53:42 UTC) #2
pbos-webrtc
On 2015/08/31 10:53:42, the sun wrote: Peter, can you take the first pass at this?
5 years, 3 months ago (2015-08-31 11:21:15 UTC) #3
pthatcher1
The thing I'm most worried about is the interaction between Call, the VideoChannel, and the ...
5 years, 3 months ago (2015-09-02 21:24:22 UTC) #4
the sun
https://codereview.webrtc.org/1269863005/diff/200001/talk/app/webrtc/mediacontroller.cc File talk/app/webrtc/mediacontroller.cc (right): https://codereview.webrtc.org/1269863005/diff/200001/talk/app/webrtc/mediacontroller.cc#newcode56 talk/app/webrtc/mediacontroller.cc:56: video_channel_ = video_channel; On 2015/09/02 21:24:22, pthatcher1 wrote: > ...
5 years, 3 months ago (2015-09-03 15:00:47 UTC) #5
pbos-webrtc
On 2015/09/03 15:00:47, the sun wrote: > https://codereview.webrtc.org/1269863005/diff/200001/talk/app/webrtc/mediacontroller.cc > File talk/app/webrtc/mediacontroller.cc (right): > > https://codereview.webrtc.org/1269863005/diff/200001/talk/app/webrtc/mediacontroller.cc#newcode56 ...
5 years, 3 months ago (2015-09-03 15:07:23 UTC) #6
pbos-webrtc
On 2015/09/03 15:07:23, pbos-webrtc wrote: > On 2015/09/03 15:00:47, the sun wrote: > > > ...
5 years, 3 months ago (2015-09-03 16:31:40 UTC) #7
the sun
On 2015/09/03 16:31:40, pbos-webrtc wrote: > On 2015/09/03 15:07:23, pbos-webrtc wrote: > > On 2015/09/03 ...
5 years, 3 months ago (2015-09-03 20:33:31 UTC) #8
pthatcher1
https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h File talk/app/webrtc/mediacontroller.h (right): https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h#newcode42 talk/app/webrtc/mediacontroller.h:42: // MediaController owns state shared between multiple concurrent media ...
5 years, 3 months ago (2015-09-04 04:52:31 UTC) #9
pthatcher1
So are you going to merge this with the load balancer redo?
5 years, 3 months ago (2015-09-04 04:56:27 UTC) #10
the sun
https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h File talk/app/webrtc/mediacontroller.h (right): https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h#newcode42 talk/app/webrtc/mediacontroller.h:42: // MediaController owns state shared between multiple concurrent media ...
5 years, 3 months ago (2015-09-04 10:18:47 UTC) #11
pthatcher1
https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h File talk/app/webrtc/mediacontroller.h (right): https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h#newcode49 talk/app/webrtc/mediacontroller.h:49: webrtc::Call* call() { return call_.get(); } On 2015/09/04 10:18:47, ...
5 years, 3 months ago (2015-09-04 22:58:32 UTC) #12
the sun
On 2015/09/04 22:58:32, pthatcher1 wrote: > https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h > File talk/app/webrtc/mediacontroller.h (right): > > https://codereview.webrtc.org/1269863005/diff/240001/talk/app/webrtc/mediacontroller.h#newcode49 > ...
5 years, 3 months ago (2015-09-08 13:44:29 UTC) #13
pbos-webrtc
lgtm https://codereview.webrtc.org/1269863005/diff/300001/talk/media/base/videoengine_unittest.h File talk/media/base/videoengine_unittest.h (right): https://codereview.webrtc.org/1269863005/diff/300001/talk/media/base/videoengine_unittest.h#newcode1883 talk/media/base/videoengine_unittest.h:1883: rtc::scoped_ptr<webrtc::Call> call_; const rtc::scoped_ptr https://codereview.webrtc.org/1269863005/diff/300001/talk/media/webrtc/webrtcvideoengine2.h File talk/media/webrtc/webrtcvideoengine2.h (right): ...
5 years, 3 months ago (2015-09-08 14:47:41 UTC) #14
the sun
https://codereview.webrtc.org/1269863005/diff/300001/talk/media/base/videoengine_unittest.h File talk/media/base/videoengine_unittest.h (right): https://codereview.webrtc.org/1269863005/diff/300001/talk/media/base/videoengine_unittest.h#newcode1883 talk/media/base/videoengine_unittest.h:1883: rtc::scoped_ptr<webrtc::Call> call_; On 2015/09/08 14:47:41, pbos-webrtc wrote: > const ...
5 years, 3 months ago (2015-09-08 15:21:35 UTC) #15
pthatcher1
https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc File talk/app/webrtc/mediacontroller.cc (right): https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc#newcode40 talk/app/webrtc/mediacontroller.cc:40: class MediaControllerImpl : public MediaController { And this should ...
5 years, 3 months ago (2015-09-09 07:27:43 UTC) #16
pbos-webrtc
https://codereview.webrtc.org/1269863005/diff/320001/talk/media/webrtc/webrtcvoiceengine.h File talk/media/webrtc/webrtcvoiceengine.h (right): https://codereview.webrtc.org/1269863005/diff/320001/talk/media/webrtc/webrtcvoiceengine.h#newcode421 talk/media/webrtc/webrtcvoiceengine.h:421: WebRtcVoiceEngine* const engine_; On 2015/09/09 07:27:43, pthatcher1 wrote: > ...
5 years, 3 months ago (2015-09-09 07:58:46 UTC) #17
the sun
https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc File talk/app/webrtc/mediacontroller.cc (right): https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc#newcode40 talk/app/webrtc/mediacontroller.cc:40: class MediaControllerImpl : public MediaController { On 2015/09/09 07:27:43, ...
5 years, 3 months ago (2015-09-09 08:24:51 UTC) #18
the sun
On 2015/09/09 08:24:51, the sun wrote: > https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc > File talk/app/webrtc/mediacontroller.cc (right): > > https://codereview.webrtc.org/1269863005/diff/320001/talk/app/webrtc/mediacontroller.cc#newcode40 ...
5 years, 3 months ago (2015-09-10 15:09:36 UTC) #19
the sun
On 2015/09/10 15:09:36, the sun wrote: > On 2015/09/09 08:24:51, the sun wrote: > > ...
5 years, 3 months ago (2015-09-14 08:02:53 UTC) #20
pthatcher1
lgtm
5 years, 3 months ago (2015-09-15 00:30:56 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1269863005/460001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1269863005/460001
5 years, 3 months ago (2015-09-15 08:37:38 UTC) #24
the sun
Committed patchset #24 (id:460001) manually as 709ed67c38d0a942f3bf3e68e337cc27a27bc353 (presubmit successful).
5 years, 3 months ago (2015-09-15 10:26:51 UTC) #25
commit-bot: I haz the power
5 years, 3 months ago (2015-09-15 10:26:52 UTC) #26
Message was sent while issue was closed.
Patchset 24 (id:??) landed as
https://crrev.com/709ed67c38d0a942f3bf3e68e337cc27a27bc353
Cr-Commit-Position: refs/heads/master@{#9939}

Powered by Google App Engine
This is Rietveld 408576698