Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2530393003:
Move histogram for number of pause events to per stream. (Closed)
|
Created:
4 years ago by åsapersson Modified:
4 years ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionMove histogram for number of pause events to per stream:
"WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents"
Recorded if a certain time has passed (10 sec) since the first media packet was sent.
Moved to per stream to know when media has started and to prevent logging stats for calls that was never in use.
Add histogram for percentage of paused video time for sent video streams:
"WebRTC.Video.PausedTimeInPercent"
BUG=b/32659204
Review-Url: https://codereview.webrtc.org/2530393003
Cr-Commit-Position: refs/heads/master@{#15681}
Committed: https://chromium.googlesource.com/external/webrtc/+/66d4b3741487da22a7bd63d07160052883413245
Patch Set 1 : Moved from https://codereview.webrtc.org/2482763003/ #Patch Set 2 #Patch Set 3 #Patch Set 4 : rebase #
Total comments: 3
Patch Set 5 #Patch Set 6 : change int to int64_t #Patch Set 7 : fix comment #
Messages
Total messages: 53 (45 generated)
Patchset #3 (id:40001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/18947)
Description was changed from ========== Update call histograms that do not have a minimum lifetime limit before being recorded. Updated histograms: "WebRTC.Call.NumberOfPauseEvents" Prevents logging stats if call was never (or shortly) in use. BUG=b/32659204 ========== to ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ==========
Patchset #3 (id:60001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ========== to ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ==========
Description was changed from ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ========== to ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ==========
Patchset #3 (id:80001) has been deleted
Patchset #3 (id:100001) has been deleted
Patchset #3 (id:120001) has been deleted
Patchset #3 (id:140001) has been deleted
Patchset #3 (id:160001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
asapersson@webrtc.org changed reviewers: + stefan@webrtc.org
Description was changed from ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ========== to ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and to prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:180001) has been deleted
Patchset #3 (id:200001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
asapersson@webrtc.org changed reviewers: + mflodman@webrtc.org
https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... File webrtc/video/send_statistics_proxy.cc (right): https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... webrtc/video/send_statistics_proxy.cc:329: paused_time_counter_.Percent(metrics::kMinRunTimeInSeconds * 1000); Why is this argument passed in? We already know it has been running that long based on line 324, ,right?
https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... File webrtc/video/send_statistics_proxy.cc (right): https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... webrtc/video/send_statistics_proxy.cc:329: paused_time_counter_.Percent(metrics::kMinRunTimeInSeconds * 1000); On 2016/12/13 12:18:01, stefan-webrtc (holmer) wrote: > Why is this argument passed in? We already know it has been running that long > based on line 324, ,right? Right, should be about the same if OnSetEncoderTargetRate is called at the beginning and the end of a call. Maybe good to have kMinRunTimeInSeconds as argument in case this would change.
Patchset #6 (id:280001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #6 (id:300001) has been deleted
The CQ bit was checked by asapersson@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... File webrtc/video/send_statistics_proxy.cc (right): https://codereview.webrtc.org/2530393003/diff/240001/webrtc/video/send_statis... webrtc/video/send_statistics_proxy.cc:329: paused_time_counter_.Percent(metrics::kMinRunTimeInSeconds * 1000); On 2016/12/13 12:51:32, åsapersson wrote: > On 2016/12/13 12:18:01, stefan-webrtc (holmer) wrote: > > Why is this argument passed in? We already know it has been running that long > > based on line 324, ,right? > > Right, should be about the same if OnSetEncoderTargetRate is called at the > beginning and the end of a call. Maybe good to have kMinRunTimeInSeconds as > argument in case this would change. Acknowledged.
The CQ bit was checked by asapersson@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2530393003/#ps340001 (title: "fix comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/19481)
The CQ bit was checked by asapersson@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 340001, "attempt_start_ts": 1482158194935490,
"parent_rev": "0cd27ba088766b7822df1bfe7c21c3dfd57d368c", "commit_rev":
"66d4b3741487da22a7bd63d07160052883413245"}
Message was sent while issue was closed.
Description was changed from ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and to prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 ========== to ========== Move histogram for number of pause events to per stream: "WebRTC.Call.NumberOfPauseEvents" -> "WebRTC.Video.NumberOfPauseEvents" Recorded if a certain time has passed (10 sec) since the first media packet was sent. Moved to per stream to know when media has started and to prevent logging stats for calls that was never in use. Add histogram for percentage of paused video time for sent video streams: "WebRTC.Video.PausedTimeInPercent" BUG=b/32659204 Review-Url: https://codereview.webrtc.org/2530393003 Cr-Commit-Position: refs/heads/master@{#15681} Committed: https://chromium.googlesource.com/external/webrtc/+/66d4b3741487da22a7bd63d07... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:340001) as https://chromium.googlesource.com/external/webrtc/+/66d4b3741487da22a7bd63d07... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
