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

Issue 2996643002: BWE allocation strategy

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by alexnarest
Modified:
4 days, 17 hours ago
CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, kwiberg-webrtc, the sun, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

BWE allocation strategy allows controlling of bitrate allocation with WEBRTC external logic. This CL implements the main logic and IOS appRTC integration. Unit tests and Android appRTC will be in separate CL. BUG=8243

Patch Set 1 #

Patch Set 2 : BWE allocation strategy #

Patch Set 3 : BWE allocation strategy #

Total comments: 8

Patch Set 4 : BWE allocation strategy #

Patch Set 5 : BWE allocation strategy #

Patch Set 6 : . #

Total comments: 16

Patch Set 7 : BWE allocation strategy #

Patch Set 8 : BWE allocation strategy #

Patch Set 9 : BWE allocation strategy #

Patch Set 10 : BWE allocation strategy #

Patch Set 11 : BWE allocation strategy #

Total comments: 1

Patch Set 12 : BWE allocation strategy #

Patch Set 13 : BWE allocation strategy #

Total comments: 23

Patch Set 14 : BWE allocation strategy #

Patch Set 15 : BWE allocation strategy #

Total comments: 2

Patch Set 16 : BWE allocation strategy #

Total comments: 34

Patch Set 17 : Documentation and comments handling #

Total comments: 6

Patch Set 18 : Comments handling #

Unified diffs Side-by-side diffs Delta from patch set Stats (+510 lines, -21 lines) Patch
M webrtc/api/peerconnectioninterface.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +11 lines, -0 lines 0 comments Download
M webrtc/api/peerconnectionproxy.h View 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/audio/audio_send_stream.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/call/bitrate_allocator.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +17 lines, -11 lines 0 comments Download
M webrtc/call/bitrate_allocator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +28 lines, -1 line 0 comments Download
M webrtc/call/call.h View 1 2 3 4 8 9 10 11 12 13 14 15 2 chunks +4 lines, -0 lines 0 comments Download
M webrtc/call/call.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +15 lines, -0 lines 0 comments Download
M webrtc/examples/BUILD.gn View 1 2 3 4 2 chunks +3 lines, -0 lines 0 comments Download
M webrtc/examples/objc/AppRTCMobile/ARDAppClient.m View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 5 chunks +12 lines, -1 line 0 comments Download
A webrtc/examples/objc/AppRTCMobile/ARDBitrateAllocationStrategy.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +18 lines, -0 lines 0 comments Download
A webrtc/examples/objc/AppRTCMobile/ARDBitrateAllocationStrategy.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +48 lines, -0 lines 0 comments Download
M webrtc/examples/objc/AppRTCMobile/ios/ARDAppDelegate.m View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M webrtc/modules/pacing/paced_sender.h View 1 2 3 4 2 chunks +7 lines, -0 lines 0 comments Download
M webrtc/modules/pacing/paced_sender.cc View 1 2 3 4 3 chunks +8 lines, -4 lines 0 comments Download
M webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M webrtc/pc/peerconnection.h View 1 2 3 4 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/pc/peerconnection.cc View 1 2 3 4 8 9 10 11 12 13 14 15 1 chunk +13 lines, -0 lines 0 comments Download
M webrtc/rtc_base/BUILD.gn View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
A webrtc/rtc_base/bitrateallocationstrategy.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +101 lines, -0 lines 0 comments Download
A webrtc/rtc_base/bitrateallocationstrategy.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +124 lines, -0 lines 0 comments Download
M webrtc/sdk/BUILD.gn View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
A webrtc/sdk/objc/Framework/Classes/PeerConnection/RTCBitrateAllocationStrategy.mm View 1 2 3 4 8 9 10 11 12 13 14 15 1 chunk +28 lines, -0 lines 0 comments Download
M webrtc/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnection.mm View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +9 lines, -0 lines 0 comments Download
A webrtc/sdk/objc/Framework/Headers/WebRTC/RTCBitrateAllocationStrategy.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +33 lines, -0 lines 0 comments Download
M webrtc/sdk/objc/Framework/Headers/WebRTC/RTCPeerConnection.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +4 lines, -0 lines 0 comments Download
M webrtc/video/video_send_stream.cc View 1 2 3 4 5 3 chunks +3 lines, -3 lines 0 comments Download
M webrtc/voice_engine/channel.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
Trybot results: Sign in to try more bots
Commit queue not available (can’t edit this change).

Depends on Patchset:

Messages

Total messages: 30 (6 generated)
stefan-webrtc
I've taken a look at the high level and have some early comments. https://codereview.webrtc.org/2996643002/diff/40001/webrtc/api/peerconnectioninterface.h File ...
1 month, 1 week ago (2017-08-14 07:46:03 UTC) #4
alexnarest
https://codereview.webrtc.org/2996643002/diff/40001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2996643002/diff/40001/webrtc/api/peerconnectioninterface.h#newcode765 webrtc/api/peerconnectioninterface.h:765: virtual RTCError SetBitrateAllocationStrategy( On 2017/08/14 07:46:03, stefan-webrtc wrote: > ...
1 month ago (2017-08-15 06:02:37 UTC) #5
stefan-webrtc
Nisse, would you mind helping out with the review of this?
1 month ago (2017-08-17 13:49:44 UTC) #7
nisse-webrtc
I've had a first look at webrtc/rtc_base/bitrateallocationstrategy.h. I think I need to understand the interfaces ...
4 weeks ago (2017-08-22 11:46:06 UTC) #8
alexnarest
https://codereview.webrtc.org/2996643002/diff/100001/webrtc/rtc_base/bitrateallocationstrategy.h File webrtc/rtc_base/bitrateallocationstrategy.h (right): https://codereview.webrtc.org/2996643002/diff/100001/webrtc/rtc_base/bitrateallocationstrategy.h#newcode34 webrtc/rtc_base/bitrateallocationstrategy.h:34: TrackConfig(const TrackConfig& track_config) On 2017/08/22 11:46:05, nisse-webrtc wrote: > ...
4 weeks ago (2017-08-22 14:52:18 UTC) #9
nisse-webrtc
https://codereview.webrtc.org/2996643002/diff/100001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/100001/webrtc/call/bitrate_allocator.cc#newcode272 webrtc/call/bitrate_allocator.cc:272: (std::stringstream() Converting a pointer to a string and using ...
4 weeks ago (2017-08-22 15:23:34 UTC) #10
alexnarest
https://codereview.webrtc.org/2996643002/diff/100001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/100001/webrtc/call/bitrate_allocator.cc#newcode272 webrtc/call/bitrate_allocator.cc:272: (std::stringstream() On 2017/08/22 15:23:33, nisse-webrtc wrote: > Converting a ...
2 weeks, 5 days ago (2017-08-31 18:40:27 UTC) #11
alexnarest
TrackAllocations changed to vector.
2 weeks, 1 day ago (2017-09-04 15:46:08 UTC) #12
stefan-webrtc
https://codereview.webrtc.org/2996643002/diff/200001/webrtc/audio/audio_send_stream.cc File webrtc/audio/audio_send_stream.cc (right): https://codereview.webrtc.org/2996643002/diff/200001/webrtc/audio/audio_send_stream.cc#newcode238 webrtc/audio/audio_send_stream.cc:238: // Audio BWE is enabled End with . https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc ...
2 weeks, 1 day ago (2017-09-05 10:42:48 UTC) #13
nisse-webrtc
https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc#newcode92 webrtc/call/bitrate_allocator.cc:92: ObserverConfig* config = static_cast<ObserverConfig*>(track_config.get()); It not so nice to ...
2 weeks, 1 day ago (2017-09-05 11:03:46 UTC) #14
alexnarest
https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc#newcode92 webrtc/call/bitrate_allocator.cc:92: ObserverConfig* config = static_cast<ObserverConfig*>(track_config.get()); On 2017/09/05 11:03:46, nisse-webrtc wrote: ...
2 weeks ago (2017-09-05 13:37:46 UTC) #16
kwiberg-webrtc
https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc#newcode92 webrtc/call/bitrate_allocator.cc:92: ObserverConfig* config = static_cast<ObserverConfig*>(track_config.get()); On 2017/09/05 13:37:45, alexnarest wrote: ...
2 weeks ago (2017-09-06 08:49:46 UTC) #17
nisse-webrtc
https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc#newcode92 webrtc/call/bitrate_allocator.cc:92: ObserverConfig* config = static_cast<ObserverConfig*>(track_config.get()); On 2017/09/06 08:49:46, kwiberg-webrtc wrote: ...
2 weeks ago (2017-09-06 09:04:02 UTC) #18
alexnarest
https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc File webrtc/call/bitrate_allocator.cc (right): https://codereview.webrtc.org/2996643002/diff/240001/webrtc/call/bitrate_allocator.cc#newcode92 webrtc/call/bitrate_allocator.cc:92: ObserverConfig* config = static_cast<ObserverConfig*>(track_config.get()); On 2017/09/06 09:04:02, nisse-webrtc wrote: ...
1 week, 4 days ago (2017-09-08 17:09:23 UTC) #19
Taylor Brandstetter
I think the code could use some more comments, but overall this looks good. A ...
1 week, 1 day ago (2017-09-11 16:55:07 UTC) #20
nisse-webrtc
Looks pretty good now. I'd like to see the additional documentation suggested by Taylor. I ...
1 week, 1 day ago (2017-09-12 10:39:05 UTC) #21
alexnarest
https://codereview.webrtc.org/2996643002/diff/300001/webrtc/api/peerconnectioninterface.h File webrtc/api/peerconnectioninterface.h (right): https://codereview.webrtc.org/2996643002/diff/300001/webrtc/api/peerconnectioninterface.h#newcode776 webrtc/api/peerconnectioninterface.h:776: // WEBRTC allocator will be used. May be changed ...
5 days, 22 hours ago (2017-09-14 13:08:34 UTC) #23
Taylor Brandstetter
https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.cc File webrtc/rtc_base/bitrateallocationstrategy.cc (right): https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.cc#newcode44 webrtc/rtc_base/bitrateallocationstrategy.cc:44: return track_allocations; On 2017/09/14 13:08:33, alexnarest wrote: > On ...
5 days, 9 hours ago (2017-09-15 01:24:41 UTC) #24
Taylor Brandstetter
https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h File webrtc/rtc_base/bitrateallocationstrategy.h (right): https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h#newcode1 webrtc/rtc_base/bitrateallocationstrategy.h:1: /* On 2017/09/14 13:08:34, alexnarest wrote: > On 2017/09/11 ...
5 days, 9 hours ago (2017-09-15 01:26:21 UTC) #25
nisse-webrtc
https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h File webrtc/rtc_base/bitrateallocationstrategy.h (right): https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h#newcode1 webrtc/rtc_base/bitrateallocationstrategy.h:1: /* On 2017/09/15 01:26:21, Taylor Brandstetter wrote: > On ...
5 days, 4 hours ago (2017-09-15 07:05:45 UTC) #26
alexnarest
On 2017/09/15 07:05:45, nisse-webrtc wrote: > https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h > File webrtc/rtc_base/bitrateallocationstrategy.h (right): > > https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.h#newcode1 > ...
5 days, 2 hours ago (2017-09-15 08:29:01 UTC) #27
alexnarest
https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.cc File webrtc/rtc_base/bitrateallocationstrategy.cc (right): https://codereview.webrtc.org/2996643002/diff/300001/webrtc/rtc_base/bitrateallocationstrategy.cc#newcode44 webrtc/rtc_base/bitrateallocationstrategy.cc:44: return track_allocations; On 2017/09/15 01:24:41, Taylor Brandstetter wrote: > ...
5 days, 2 hours ago (2017-09-15 08:29:42 UTC) #28
Taylor Brandstetter
lgtm, assuming files will be moved to api/ in a separate CL
4 days, 17 hours ago (2017-09-15 17:34:20 UTC) #29
Taylor Brandstetter
4 days, 17 hours ago (2017-09-15 17:35:38 UTC) #30
Actually, not lgtm. I just noticed that BitrateAllocationStrategy still needs
tests.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b