Event log visualizer TimeSeries is now created on the stack and then moved into the vector of serie…

Event log visualizer TimeSeries is now created on the stack and then moved into the vector of series.

BUG=none

Review-Url: https://codereview.webrtc.org/2824973003
Cr-Commit-Position: refs/heads/master@{#17763}
Committed: https://chromium.googlesource.com/external/webrtc/+/35ba9bd5970f91cdb1513a21694cb8fd78de5308

[philipel, 2017-04-18 12:54:23]

philipel@webrtc.org changed reviewers:
+ terelius@webrtc.org

[philipel, 2017-04-18 12:54:24]

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
File webrtc/tools/event_log_visualizer/plot_python.cc (right):

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/plot_python.cc:36: printf("\n# === Series: %s
===\n", series_list_[i].label.c_str());
Makes it easier to debug the generated python code when something fails.

[terelius, 2017-04-18 16:17:54]

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:869: TimeSeries time_series =
TimeSeries("Fraction lost", LINE_DOT_GRAPH);
TimeSeries time_series("Fraction lost", LINE_DOT_GRAPH); to avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:938: TimeSeries loss_series =
TimeSeries("Loss-based estimate", LINE_STEP_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:947: TimeSeries("Delay-based
estimate", LINE_STEP_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:955: TimeSeries created_series =
TimeSeries("Probe cluster created.", DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:962: TimeSeries result_series =
TimeSeries("Probing results.", DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1301: TimeSeries("Audio encoder
target bitrate", LINE_DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1317: TimeSeries time_series =
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1336: TimeSeries("Audio encoder
uplink packet loss fraction", LINE_DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1353: TimeSeries time_series =
TimeSeries("Audio encoder FEC", LINE_DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1369: TimeSeries time_series =
TimeSeries("Audio encoder DTX", LINE_DOT_GRAPH);
Avoid extra copy.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1385: TimeSeries time_series =
Avoid extra copy.

[philipel, 2017-04-19 08:59:37]

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:869: TimeSeries time_series =
TimeSeries("Fraction lost", LINE_DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> TimeSeries time_series("Fraction lost", LINE_DOT_GRAPH); to avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:938: TimeSeries loss_series =
TimeSeries("Loss-based estimate", LINE_STEP_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:947: TimeSeries("Delay-based
estimate", LINE_STEP_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:955: TimeSeries created_series =
TimeSeries("Probe cluster created.", DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:962: TimeSeries result_series =
TimeSeries("Probing results.", DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1301: TimeSeries("Audio encoder
target bitrate", LINE_DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1317: TimeSeries time_series =
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1336: TimeSeries("Audio encoder
uplink packet loss fraction", LINE_DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1353: TimeSeries time_series =
TimeSeries("Audio encoder FEC", LINE_DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1369: TimeSeries time_series =
TimeSeries("Audio encoder DTX", LINE_DOT_GRAPH);
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

https://codereview.webrtc.org/2824973003/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1385: TimeSeries time_series =
On 2017/04/18 16:17:54, terelius wrote:
> Avoid extra copy.

Done.

[terelius, 2017-04-19 09:20:19]

Nice! Thanks for fixing this.

lgtm with one nit:

https://codereview.webrtc.org/2824973003/diff/20001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2824973003/diff/20001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1311:
plot->AppendTimeSeries(std::move(time_series));
nit: I'd like to finish the timeseries specific things before setting the
general graph properties. I.e., please append the timeseries before setting
x-axis, y-axis and title.

Same below.

[philipel, 2017-04-19 12:32:11]

https://codereview.webrtc.org/2824973003/diff/20001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2824973003/diff/20001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1311:
plot->AppendTimeSeries(std::move(time_series));
On 2017/04/19 09:20:18, terelius wrote:
> nit: I'd like to finish the timeseries specific things before setting the
> general graph properties. I.e., please append the timeseries before setting
> x-axis, y-axis and title.
> 
> Same below.

Done.

[philipel, 2017-04-19 12:32:17]

The CQ bit was checked by philipel@webrtc.org

[philipel, 2017-04-19 12:32:17]

The patchset sent to the CQ was uploaded after l-g-t-m from terelius@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2824973003/#ps40001 (title:
"Feedback")

[commit-bot: I haz the power, 2017-04-19 12:32:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-19 12:58:52]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492605137314630,
"parent_rev": "13fc18068f3e9013e036d584c9c18acaa6f1430b", "commit_rev":
"35ba9bd5970f91cdb1513a21694cb8fd78de5308"}

[commit-bot: I haz the power, 2017-04-19 12:58:55]

Description was changed from

==========
Event log visualizer TimeSeries is now created on the stack and then moved into
the vector of series.

BUG=none
==========

to

==========
Event log visualizer TimeSeries is now created on the stack and then moved into
the vector of series.

BUG=none

Review-Url: https://codereview.webrtc.org/2824973003
Cr-Commit-Position: refs/heads/master@{#17763}
Committed:
https://chromium.googlesource.com/external/webrtc/+/35ba9bd5970f91cdb1513a216...
==========

[commit-bot: I haz the power, 2017-04-19 12:58:56]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/35ba9bd5970f91cdb1513a216...