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

Issue 1376423002: Make overuse estimator one dimensional. (Closed)

Created:
3 years, 8 months ago by stefan-webrtc
Modified:
3 years, 6 months ago
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, mflodman
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Make overuse estimator one dimensional. This drops the payload size difference dimension of the Kalman filter, which doesn't improve the quality of the estimation when pacing packets on the send-side. R=gaetano.carlucci@gmail.com, mflodman@webrtc.org, terelius@webrtc.org Committed: https://crrev.com/06e05a85b9e4def75ed5e6b582c4df842616f25f Cr-Commit-Position: refs/heads/master@{#10809}

Patch Set 1 #

Patch Set 2 : Tests. #

Patch Set 3 : Further cleanups. #

Total comments: 10

Patch Set 4 : comments #

Patch Set 5 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+265 lines, -423 lines) Patch
M webrtc/common_types.h View 1 2 3 1 chunk +0 lines, -25 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/inter_arrival.h View 1 2 3 2 chunks +3 lines, -12 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/inter_arrival.cc View 1 2 3 2 chunks +3 lines, -10 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/inter_arrival_unittest.cc View 1 2 3 9 chunks +67 lines, -103 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/overuse_detector.h View 1 2 3 2 chunks +1 line, -2 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/overuse_detector.cc View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/overuse_detector_unittest.cc View 1 2 3 15 chunks +127 lines, -164 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/overuse_estimator.h View 1 2 3 2 chunks +10 lines, -9 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/overuse_estimator.cc View 1 2 3 4 chunks +36 lines, -67 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc View 1 2 3 3 chunks +3 lines, -8 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc View 1 2 3 3 chunks +11 lines, -14 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc View 1 2 3 2 chunks +1 line, -3 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc View 1 2 3 2 chunks +1 line, -3 lines 0 comments Download

Messages

Total messages: 23 (8 generated)
stefan-webrtc
gaetano, please take a close look at the changes in overuse_estimator.cc. terelius, could you review ...
3 years, 8 months ago (2015-10-01 14:06:16 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376423002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376423002/40001
3 years, 8 months ago (2015-10-01 14:07:14 UTC) #4
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_x64_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_gn_dbg/builds/4334)
3 years, 8 months ago (2015-10-01 14:14:59 UTC) #6
Gaetano Carlucci
lgtm LGTM after having run some tests. My only concern is about the setting of ...
3 years, 8 months ago (2015-10-06 13:50:25 UTC) #7
terelius
I want to look a bit more closely on the estimator, but I think everything ...
3 years, 8 months ago (2015-10-12 12:04:41 UTC) #8
stefan-webrtc
https://codereview.webrtc.org/1376423002/diff/40001/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc File webrtc/modules/remote_bitrate_estimator/inter_arrival.cc (right): https://codereview.webrtc.org/1376423002/diff/40001/webrtc/modules/remote_bitrate_estimator/inter_arrival.cc#newcode73 webrtc/modules/remote_bitrate_estimator/inter_arrival.cc:73: current_timestamp_group_.complete_time_ms = arrival_time_ms; On 2015/10/12 12:04:40, terelius wrote: > ...
3 years, 6 months ago (2015-11-24 16:08:03 UTC) #9
stefan-webrtc
Any further comments on this CL?
3 years, 6 months ago (2015-11-26 07:57:24 UTC) #10
terelius
lgtm
3 years, 6 months ago (2015-11-26 09:15:39 UTC) #11
stefan-webrtc
Magnus, I need your eyes on webrtc/common_types.h. I believe you will happily accept that change. ...
3 years, 6 months ago (2015-11-26 10:11:00 UTC) #12
mflodman
Very much so, LGTM :)
3 years, 6 months ago (2015-11-26 12:15:07 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376423002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376423002/80001
3 years, 6 months ago (2015-11-26 12:30:21 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
3 years, 6 months ago (2015-11-26 14:31:00 UTC) #18
stefan-webrtc
Committed patchset #5 (id:80001) manually as 06e05a85b9e4def75ed5e6b582c4df842616f25f (presubmit successful).
3 years, 6 months ago (2015-11-26 14:35:16 UTC) #20
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/06e05a85b9e4def75ed5e6b582c4df842616f25f Cr-Commit-Position: refs/heads/master@{#10809}
3 years, 6 months ago (2015-11-26 14:35:19 UTC) #22
stefan-webrtc
3 years, 6 months ago (2015-11-27 09:02:14 UTC) #23
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.webrtc.org/1481003002/ by stefan@webrtc.org.

The reason for reverting is: Broke webrtc_perf_tests on bots..

Powered by Google App Engine
This is Rietveld 408576698