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

Issue 2996643002: BWE allocation strategy

Created:
3 months, 1 week ago by alexnarest
Modified:
1 month, 3 weeks 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=webrtc: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:

Depends on Patchset:

Messages

Total messages: 31 (7 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 ...
3 months 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: > ...
3 months ago (2017-08-15 06:02:37 UTC) #5
stefan-webrtc
Nisse, would you mind helping out with the review of this?
3 months 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 ...
2 months, 3 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: > ...
2 months, 3 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 ...
2 months, 3 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 months, 2 weeks ago (2017-08-31 18:40:27 UTC) #11
alexnarest
TrackAllocations changed to vector.
2 months, 2 weeks 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 months, 1 week 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 months, 1 week 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 months, 1 week 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 months, 1 week 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 months, 1 week 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: ...
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months 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 ...
2 months 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 ...
2 months 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 ...
2 months 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 ...
2 months 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 > ...
2 months 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: > ...
2 months ago (2017-09-15 08:29:42 UTC) #28
Taylor Brandstetter
lgtm, assuming files will be moved to api/ in a separate CL
2 months ago (2017-09-15 17:34:20 UTC) #29
Taylor Brandstetter
2 months ago (2017-09-15 17:35:38 UTC) #30
Actually, not lgtm. I just noticed that BitrateAllocationStrategy still needs
tests.

Powered by Google App Engine
This is Rietveld efc10ee0f