Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(15)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1404953002: Move PRNG from BWE test framework to webrtc/test. (Closed)

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

Description

Move PRNG from BWE test framework to webrtc/test. BUG= Committed: https://crrev.com/5bdddf91d300299a8d40f153b842dba61db58c60 Cr-Commit-Position: refs/heads/master@{#10285}

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -158 lines) Patch
M webrtc/modules/remote_bitrate_estimator/overuse_detector_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.h View 5 chunks +4 lines, -30 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework.cc View 2 chunks +1 line, -36 lines 0 comments Download
M webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
D webrtc/modules/remote_bitrate_estimator/test/random.h View 1 chunk +0 lines, -40 lines 0 comments Download
D webrtc/modules/remote_bitrate_estimator/test/random.cc View 1 chunk +0 lines, -41 lines 0 comments Download
A + webrtc/test/random.h View 3 chunks +12 lines, -3 lines 5 comments Download
A + webrtc/test/random.cc View 3 chunks +17 lines, -1 line 2 comments Download
M webrtc/test/webrtc_test_common.gyp View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
the sun
5 years, 2 months ago (2015-10-15 09:45:20 UTC) #2
pbos-webrtc
lgtm https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h File webrtc/test/random.h (right): https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h#newcode25 webrtc/test/random.h:25: // Return pseudo-random number in the interval [0.0, ...
5 years, 2 months ago (2015-10-15 10:28:02 UTC) #3
stefan-webrtc
lgtm
5 years, 2 months ago (2015-10-15 11:36:34 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1404953002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1404953002/1
5 years, 2 months ago (2015-10-15 12:09:07 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 2 months ago (2015-10-15 12:10:33 UTC) #7
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/5bdddf91d300299a8d40f153b842dba61db58c60 Cr-Commit-Position: refs/heads/master@{#10285}
5 years, 2 months ago (2015-10-15 12:10:41 UTC) #8
the sun
https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h File webrtc/test/random.h (right): https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h#newcode25 webrtc/test/random.h:25: // Return pseudo-random number in the interval [0.0, 1.0]. ...
5 years, 2 months ago (2015-10-16 21:50:31 UTC) #9
pbos-webrtc
https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.cc File webrtc/test/random.cc (right): https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.cc#newcode34 webrtc/test/random.cc:34: float uniform = Rand() * (high - low + ...
5 years, 2 months ago (2015-10-19 13:33:01 UTC) #10
the sun
5 years, 2 months ago (2015-10-19 15:10:25 UTC) #11
Message was sent while issue was closed. 
https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.cc
File webrtc/test/random.cc (right):

https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.cc#newcode34
webrtc/test/random.cc:34: float uniform = Rand() * (high - low + 1) + low;
On 2015/10/19 13:33:01, pbos-webrtc wrote:
> Then this one has off-by-one errors?
> 
> for Rand() = 1.0:
> 
> Rand(1, 2):
> 
> Rand() = 1.0 * (2 - 1 + 1) + 1 = 1.0 * 2 + 1 = 3.0.
> 
> Which gets rounded to 3.

Looks like that. Good catch!
https://codereview.webrtc.org/1412183002/

https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h
File webrtc/test/random.h (right):

https://codereview.webrtc.org/1404953002/diff/1/webrtc/test/random.h#newcode25
webrtc/test/random.h:25: // Return pseudo-random number in the interval [0.0,
1.0].
On 2015/10/19 13:33:01, pbos-webrtc wrote:
> On 2015/10/16 21:50:31, the sun wrote:
> > On 2015/10/15 10:28:02, pbos-webrtc wrote:
> > > Is this 1.0 inclusive?
> > 
> > Yes, that is why it says so. :)
> > https://en.wikipedia.org/wiki/Interval_(mathematics)
> 
> I thought [0.0, 1.0) was the "standard" one, since that's what Python does.

Acknowledged.

Powered by Google App Engine
This is Rietveld 408576698