|
|
Created:
3 years, 3 months ago by hlundin-webrtc Modified:
3 years, 3 months ago Reviewers:
kwiberg-webrtc CC:
webrtc-reviews_webrtc.org, henrika_webrtc, zhengzhonghou_agora.io, tterriberry_mozilla.com, fengyue_agora.io, peah-webrtc, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionRtpToNtpEstimator:: Add a DCHECK to avoid div-by-0
The division that follows the added DCHECK should not be div-by-0,
since when params_.calculated is true, params_.frequency_khz would
have been updated (with something non-zero).
BUG=none
Review-Url: https://codereview.webrtc.org/3006003002
Cr-Commit-Position: refs/heads/master@{#19671}
Committed: https://chromium.googlesource.com/external/webrtc/+/d56ef8085718e15c497b0d9c60baa1a4640420fa
Patch Set 1 #
Total comments: 1
Messages
Total messages: 23 (10 generated)
henrik.lundin@webrtc.org changed reviewers: + kwiberg@webrtc.org
Karl, ptal.
lgtm https://codereview.webrtc.org/3006003002/diff/1/webrtc/system_wrappers/source... File webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc (right): https://codereview.webrtc.org/3006003002/diff/1/webrtc/system_wrappers/source... webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc:168: RTC_DCHECK_NE(params_.frequency_khz, 0.0); Is it OK if the value is very, very close to zero?
The CQ bit was checked by henrik.lundin@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
On 2017/09/01 12:53:06, kwiberg-webrtc wrote: > lgtm > > https://codereview.webrtc.org/3006003002/diff/1/webrtc/system_wrappers/source... > File webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc (right): > > https://codereview.webrtc.org/3006003002/diff/1/webrtc/system_wrappers/source... > webrtc/system_wrappers/source/rtp_to_ntp_estimator.cc:168: > RTC_DCHECK_NE(params_.frequency_khz, 0.0); > Is it OK if the value is very, very close to zero? Well... Submitting after offline convo. YOLO.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_tsan2 on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/25639)
The CQ bit was checked by henrik.lundin@webrtc.org
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_arm64_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)
The CQ bit was checked by henrik.lundin@webrtc.org
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_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27377)
On 2017/09/04 07:58:16, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > android_dbg on master.tryserver.webrtc (JOB_FAILED, > http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27377) The failures on Android trybots were fixed: crbug.com/761720 You can try the commit queue again.
On 2017/09/04 09:13:07, oprypin_webrtc wrote: > On 2017/09/04 07:58:16, commit-bot: I haz the power wrote: > > Try jobs failed on following builders: > > android_dbg on master.tryserver.webrtc (JOB_FAILED, > > > http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/27377) > > The failures on Android trybots were fixed: crbug.com/761720 > You can try the commit queue again. Thanks!
The CQ bit was checked by henrik.lundin@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": 1, "attempt_start_ts": 1504538930908570, "parent_rev": "ba09f79ba31c543d0dcf3a6d824992685087005a", "commit_rev": "d56ef8085718e15c497b0d9c60baa1a4640420fa"}
Message was sent while issue was closed.
Description was changed from ========== RtpToNtpEstimator:: Add a DCHECK to avoid div-by-0 The division that follows the added DCHECK should not be div-by-0, since when params_.calculated is true, params_.frequency_khz would have been updated (with something non-zero). BUG=none ========== to ========== RtpToNtpEstimator:: Add a DCHECK to avoid div-by-0 The division that follows the added DCHECK should not be div-by-0, since when params_.calculated is true, params_.frequency_khz would have been updated (with something non-zero). BUG=none Review-Url: https://codereview.webrtc.org/3006003002 Cr-Commit-Position: refs/heads/master@{#19671} Committed: https://chromium.googlesource.com/external/webrtc/+/d56ef8085718e15c497b0d9c6... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/external/webrtc/+/d56ef8085718e15c497b0d9c6... |