Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2833353003:
Add Parser to analyse the results of the network tester. (Closed)
DescriptionAdd Parser to analyse the results of the network tester.
BUG=webrtc:7426
Review-Url: https://codereview.webrtc.org/2833353003
Cr-Commit-Position: refs/heads/master@{#18159}
Committed: https://chromium.googlesource.com/external/webrtc/+/7be9e42f6c870004a5aa9b123b9f28c8f95f5b88
Patch Set 1 #
Total comments: 14
Patch Set 2 : Respond to comments #Patch Set 3 : Fixes for presubmit tests #Messages
Total messages: 21 (14 generated)
Description was changed from ========== Add Parser to analyse the results of the network tester. BUG=webrtc:7426 ========== to ========== Add Parser to analyse the results of the network tester. BUG=webrtc:7426 ==========
michaelt@webrtc.org changed reviewers: + nisse@webrtc.org, stefan@webrtc.org
lgtm. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... File webrtc/tools/network_tester/parse_packet_log.py (right): https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:10: # To run this script please copy "out/<build_name>/pyproto/webrtc/tools/ Is there any way to make that easier? I guess that network_tester_packet_pb2.py is automatically generated, but not actually depending on what the target platform is?
https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... File webrtc/tools/network_tester/parse_packet_log.py (right): https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:11: # network_tester/network_tester_packet_pb2.py" to this folder. What is "this folder"? https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:14: # form more information call: for more https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:63: return x I'd have written: return [(p.arrival_timestamp - first_arrival_time) / 1000000.0 for p in packets]; https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:72: first_send_time_diff) Probably would have done something similar here. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:98: return self.bytes I think you should comment on units for this method. Is this returning bits/second or bytes/second? Is there an assumption about the window size to be 1 second? https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:105: y.append(bitrate.AddPacket(packet)) and here
https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... File webrtc/tools/network_tester/parse_packet_log.py (right): https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:10: # To run this script please copy "out/<build_name>/pyproto/webrtc/tools/ > Is there any way to make that easier? I don't know. I did not found a easier way till now. > I guess that network_tester_packet_pb2.py > is automatically generated, but not actually depending on what the target > platform is? Yes https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:11: # network_tester/network_tester_packet_pb2.py" to this folder. Changed phrasing. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:14: # form more information call: On 2017/04/24 08:55:21, stefan-webrtc wrote: > for more Done. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:63: return x On 2017/04/24 08:55:21, stefan-webrtc wrote: > I'd have written: > > return [(p.arrival_timestamp - first_arrival_time) / 1000000.0 for p in > packets]; Acknowledged. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:72: first_send_time_diff) On 2017/04/24 08:55:21, stefan-webrtc wrote: > Probably would have done something similar here. Acknowledged. https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:98: return self.bytes 1. Done 2. Nope, 1 second just seamed to be the most piratical value to get bits/sec https://codereview.webrtc.org/2833353003/diff/1/webrtc/tools/network_tester/p... webrtc/tools/network_tester/parse_packet_log.py:105: y.append(bitrate.AddPacket(packet)) On 2017/04/24 08:55:21, stefan-webrtc wrote: > and here Done.
lgtm
The CQ bit was checked by michaelt@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: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17029)
The CQ bit was checked by michaelt@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.
The CQ bit was checked by michaelt@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from nisse@webrtc.org, stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2833353003/#ps40001 (title: "Fixes for presubmit tests")
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": 40001, "attempt_start_ts": 1494926383533900,
"parent_rev": "edddac54bcdd6ec3f52acb124799e8fa477ffdd4", "commit_rev":
"7be9e42f6c870004a5aa9b123b9f28c8f95f5b88"}
Message was sent while issue was closed.
Description was changed from ========== Add Parser to analyse the results of the network tester. BUG=webrtc:7426 ========== to ========== Add Parser to analyse the results of the network tester. BUG=webrtc:7426 Review-Url: https://codereview.webrtc.org/2833353003 Cr-Commit-Position: refs/heads/master@{#18159} Committed: https://chromium.googlesource.com/external/webrtc/+/7be9e42f6c870004a5aa9b123... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/7be9e42f6c870004a5aa9b123... |
