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

Issue 2996643002: BWE allocation strategy

Created:
3 years, 4 months ago by alexnarest
Modified:
3 years, 2 months 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

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 years, 4 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 years, 4 months ago (2017-08-15 06:02:37 UTC) #5
stefan-webrtc
Nisse, would you mind helping out with the review of this?
3 years, 4 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 ...
3 years, 3 months 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: > ...
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 months ago (2017-08-31 18:40:27 UTC) #11
alexnarest
TrackAllocations changed to vector.
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 months 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: ...
3 years, 3 months 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: ...
3 years, 3 months 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: ...
3 years, 3 months 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: ...
3 years, 3 months 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 ...
3 years, 3 months 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 ...
3 years, 3 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 ...
3 years, 3 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 ...
3 years, 3 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 ...
3 years, 3 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 ...
3 years, 3 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 > ...
3 years, 3 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: > ...
3 years, 3 months ago (2017-09-15 08:29:42 UTC) #28
Taylor Brandstetter
lgtm, assuming files will be moved to api/ in a separate CL
3 years, 3 months ago (2017-09-15 17:34:20 UTC) #29
Taylor Brandstetter
3 years, 3 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 408576698