Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2046943004:
Changes synchronization offset perfomance tracking (Closed)
|
Created:
4 years, 6 months ago by danilchap Modified:
4 years, 6 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, the sun, pbos-webrtc, stefan-webrtc, mflodman Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionChanges synchronization offset perfomance tracking
to be logged after converge only.
BUG=webrtc:5032, chromium:618226
R=pbos@webrtc.org
Committed: https://crrev.com/371b43b86a6a1bf0fb8a9059309a67a6e16a9542
Cr-Commit-Position: refs/heads/master@{#13160}
Patch Set 1 #
Total comments: 2
Patch Set 2 : revert remove, changed instead. #Patch Set 3 : same for min_transmit_bitrate.bitrate_kbps perfomance value #Messages
Total messages: 17 (8 generated)
Description was changed from ========== Removes synchronization offset perfomance tracking BUG=webrtc:5032 ========== to ========== Removes synchronization offset perfomance tracking BUG=webrtc:5032, chromium:618226 ==========
danilchap@webrtc.org changed reviewers: + stefan@webrtc.org
It seems this performance stat does not reflect anything in the code: It is not interesting stat before converged, and converged means it is below configured threshold, i.e. due to test design the value would be just below the threshold. Changed test still have another, interesting stat 'time to converge'.
https://codereview.webrtc.org/2046943004/diff/1/webrtc/call/call_perf_tests.cc File webrtc/call/call_perf_tests.cc (left): https://codereview.webrtc.org/2046943004/diff/1/webrtc/call/call_perf_tests.c... webrtc/call/call_perf_tests.cc:102: sync_offset_ms_list_.push_back(stats.sync_offset_ms); Maybe we should still track this, but track it only after it has been in sync once? That is, when first_time_in_sync_ != -1. That way we can track the accuracy and variance of it.
Description was changed from ========== Removes synchronization offset perfomance tracking BUG=webrtc:5032, chromium:618226 ========== to ========== Changes synchronization offset perfomance tracking to be logged after converge only. BUG=webrtc:5032, chromium:618226 ==========
https://codereview.webrtc.org/2046943004/diff/1/webrtc/call/call_perf_tests.cc File webrtc/call/call_perf_tests.cc (left): https://codereview.webrtc.org/2046943004/diff/1/webrtc/call/call_perf_tests.c... webrtc/call/call_perf_tests.cc:102: sync_offset_ms_list_.push_back(stats.sync_offset_ms); On 2016/06/08 11:02:25, stefan-webrtc (holmer) wrote: > Maybe we should still track this, but track it only after it has been in sync > once? That is, when first_time_in_sync_ != -1. > > That way we can track the accuracy and variance of it. yes, that make sense (I missed the fact that test still run for few seconds after avsync converges, so there will be more than value)
danilchap@webrtc.org changed reviewers: + pbos@webrtc.org
Peter PTAL, since Stefan OOO,
lgtm, thanks
The CQ bit was checked by danilchap@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2046943004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)
Message was sent while issue was closed.
Description was changed from ========== Changes synchronization offset perfomance tracking to be logged after converge only. BUG=webrtc:5032, chromium:618226 ========== to ========== Changes synchronization offset perfomance tracking to be logged after converge only. BUG=webrtc:5032, chromium:618226 R=pbos@webrtc.org Committed: https://crrev.com/371b43b86a6a1bf0fb8a9059309a67a6e16a9542 Cr-Commit-Position: refs/heads/master@{#13160} ==========
Message was sent while issue was closed.
Description was changed from ========== Changes synchronization offset perfomance tracking to be logged after converge only. BUG=webrtc:5032, chromium:618226 R=pbos@webrtc.org Committed: https://crrev.com/371b43b86a6a1bf0fb8a9059309a67a6e16a9542 Cr-Commit-Position: refs/heads/master@{#13160} ========== to ========== Changes synchronization offset perfomance tracking to be logged after converge only. BUG=webrtc:5032, chromium:618226 R=pbos@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/371b43b86a6a1bf0fb8a90593... ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/371b43b86a6a1bf0fb8a9059309a67a6e16a9542 Cr-Commit-Position: refs/heads/master@{#13160}
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 371b43b86a6a1bf0fb8a9059309a67a6e16a9542 (presubmit successful). |
