Fix simulator issue where chokes didn't apply to non-congested packets.

Fix simulator issue where chokes didn't apply to non-congested packets.

Committed: https://crrev.com/870eee4b17f3e9d065db7e6cfff58c816698f627
Cr-Commit-Position: refs/heads/master@{#9575}

[stefan-webrtc, 2015-07-14 08:55:53]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-14 08:56:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1235143002/1

[stefan-webrtc, 2015-07-14 09:14:52]

stefan@webrtc.org changed reviewers:
+ magalhaesc@webrtc.org

[commit-bot: I haz the power, 2015-07-14 09:38:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-14 09:38:49]

Dry run: This issue passed the CQ dry run.

[magalhaesc, 2015-07-14 09:48:32]

magalhaesc@google.com changed reviewers:
+ magalhaesc@google.com

[magalhaesc, 2015-07-14 09:48:33]

Some minor comments, otherwise LGTM

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
File webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc
(right):

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:669:
const size_t kPacketSize = 125;
Do you think we should add the unit here? 
#kPacketSizeBytes#

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:670:
const int64_t kExpectedSendTimeUs = kPacketSize * 8 * 1000 / kCapacityKbps;
For this value  #kCapacityKbps = 10# we won't have rounding problems. 

But maybe to be consistent with the ChokeFilter implementation we can write:

 const int64_t kExpectedSendTimeUs = (kPacketSize * 8 * 1000 + kCapacityKbps/2)
/ kCapacityKbps;

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:677:
for (uint32_t i = 0; i < 2; ++i) {
Couldn't it be #int i# instead of #uint32_t i# ?

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:680:
packets.push_back(new MediaPacket(0, send_time_us, 125, header));
#kPacketSize# instead of #125#

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:681:
send_time_us += kExpectedSendTimeUs + 1000;
Maybe add a comment?
// Packets are sent 1000ms apart, hence the queue will be empty.

[stefan-webrtc, 2015-07-14 10:04:16]

Comments addressed. Thanks!

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
File webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc
(right):

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:669:
const size_t kPacketSize = 125;
On 2015/07/14 09:48:32, magalhaesc wrote:
> Do you think we should add the unit here? 
> #kPacketSizeBytes#

Done.

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:670:
const int64_t kExpectedSendTimeUs = kPacketSize * 8 * 1000 / kCapacityKbps;
On 2015/07/14 09:48:33, magalhaesc wrote:
> For this value  #kCapacityKbps = 10# we won't have rounding problems. 
> 
> But maybe to be consistent with the ChokeFilter implementation we can write:
> 
>  const int64_t kExpectedSendTimeUs = (kPacketSize * 8 * 1000 +
kCapacityKbps/2)
> / kCapacityKbps;
> 

Done.

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:677:
for (uint32_t i = 0; i < 2; ++i) {
On 2015/07/14 09:48:32, magalhaesc wrote:
> Couldn't it be #int i# instead of #uint32_t i# ?

Done.

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:680:
packets.push_back(new MediaPacket(0, send_time_us, 125, header));
On 2015/07/14 09:48:32, magalhaesc wrote:
> #kPacketSize# instead of #125#

Done.

https://codereview.webrtc.org/1235143002/diff/1/webrtc/modules/remote_bitrate...
webrtc/modules/remote_bitrate_estimator/test/bwe_test_framework_unittest.cc:681:
send_time_us += kExpectedSendTimeUs + 1000;
On 2015/07/14 09:48:33, magalhaesc wrote:
> Maybe add a comment?
> // Packets are sent 1000ms apart, hence the queue will be empty.

Done.

[stefan-webrtc, 2015-07-14 10:07:38]

The CQ bit was checked by stefan@webrtc.org

[stefan-webrtc, 2015-07-14 10:07:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
magalhaesc@google.com
Link to the patchset: https://codereview.webrtc.org/1235143002/#ps20001 (title:
"Comments addressed.")

[commit-bot: I haz the power, 2015-07-14 10:07:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1235143002/20001

[commit-bot: I haz the power, 2015-07-14 10:07:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-14 10:07:53]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[magalhaesc1, 2015-07-14 10:10:37]

lgtm

[stefan-webrtc, 2015-07-14 10:10:54]

The CQ bit was checked by stefan@webrtc.org

[commit-bot: I haz the power, 2015-07-14 10:11:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1235143002/20001

[commit-bot: I haz the power, 2015-07-14 10:54:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-14 10:54:11]

Patchset 2 (id:??) landed as
https://crrev.com/870eee4b17f3e9d065db7e6cfff58c816698f627
Cr-Commit-Position: refs/heads/master@{#9575}