Issue metadata
Sign in to add a comment
|
Merge fix for resetting BWE to zero when a connection switches |
||||||||||||||||||||||
Issue descriptionBug details are copied from https://bugs.chromium.org/p/webrtc/issues/detail?id=6076 In https://codereview.webrtc.org/2094863003/, we reset the BWE bitrate when there is a connection switch. But it resets the BWE bitrate to zero unexpectedly. This happens because BitrateControllerImpl::ResetBitrates() ignores sets -1 as start bitrate on a newly created SendSideBandwidthEstimation, which means it will keep its default value of zero. Fix: copy the start bitrate only if it is positive. If it is -1, there is no point remembering it. Fixing CL: https://codereview.webrtc.org/2125523004 I would like to merge that CL into WebRTC's M53 branch. Justification for M53 merge: 1. This is a regression compared to M52. 2. Fixing this is important because it may make the video sending rate in WebRTC call lower than expected values.
,
Jul 14 2016
Before we approve merge to M53, Could you please confirm whether this change is baked/verified in Canary OR on ToT and safe to merge?
,
Jul 15 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 15 2016
Thanks! The issue was detected in mobile apps and the fix was verified there and we think it will also affect chrome M53. It is only triggered under certain conditions (max_bitrate is set and a network switch), and it is not straightforward to reproduce it.
,
Jul 15 2016
I have landed the merging CL here https://chromium.googlesource.com/external/webrtc/+/0c54eb178003a7af9f275429205fea483c1d2578 and verified it is in webrtc branch 53 here: https://chromium.googlesource.com/external/webrtc/+log/branch-heads/53 Can someone verify it will be picked by Chrome 53?
,
Jul 15 2016
+tommi@, could you ptal comment #5. Thank you.
,
Jul 15 2016
Looks like everything is in place. Holmer can you double check?
,
Jul 15 2016
Please merge your change to M53 branch 2785 ASAP (latest by 4:00 PM PST on Monday, 07/18) in order to make it to M53 dev release next week before Beta promotion.
,
Jul 15 2016
I have merged the CL to webrtc branch 53. Do I need to do anything extra to merge the change to M53 branch 2785?
,
Jul 18 2016
If change is merged to webrtc branch 53, please remove "Merge-Approved-53" label and apply "merge-merged-53" label. I don't see any update from bugdroid1@ in this bug, so please do make sure that the change is merged. Thank you.
,
Jul 18 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 18 2016
Sorry the landed CL was not linked to this issue probably because I did not prefix chromium when referring to the bug in the CL. The following CL refers to this bug: https://chromium.googlesource.com/external/webrtc/+/0c54eb178003a7af9f275429205fea483c1d2578 commit 0c54eb178003a7af9f275429205fea483c1d2578 author Honghai Zhang <honghaiz@webrtc.org> Date Fri Jul 15 15:22:57 2016 Fix bug where a connection switch causes BWE to be set to zero. BUG= webrtc:6076 , 628084 R=sprang@webrtc.org,stefan@webrtc.org TBR=tommi@webrtc.org patch from issue: Review URL: https://codereview.webrtc.org/2125523004 . (cherry picked from commit be40296ccef50dd79312c6fce533bfa36b7063dc) Review URL: https://codereview.webrtc.org/2154783002 . Cr-Original-Commit-Position: refs/branch-heads/53@{#3} Cr-Original-Branched-From: 65f47275cbb5ad3aa5146b9e75afa9894467daff-refs/heads/master@{#13317} Cr-Commit-Position: refs/branch-heads/53@{#3} Cr-Branched-From: 65f47275cbb5ad3aa5146b9e75afa9894467daff-refs/heads/master@{#13317}
,
Jul 18 2016
,
Jul 18 2016
,
Jul 18 2016
holmer@, can you double check to make sure it will be included in Chrome-53? |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by honghaiz@chromium.org
, Jul 14 2016Labels: -Type-Bug Merge-Request-53 M-53 OS-All Type-Bug-Regression