Overlay REMB in total bitrate graphs in visualization tool.

Overlay REMB in total bitrate graphs in visualization tool.

This doesn't affect the production code.

BUG=webrtc:7726
Review-Url: https://codereview.webrtc.org/2912813002
Cr-Commit-Position: refs/heads/master@{#18400}
Committed: https://chromium.googlesource.com/external/webrtc/+/2c8e8a306a5c2db8566763d0d898cbd08d05f6cb

[terelius, 2017-05-29 17:14:23]

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

[commit-bot: I haz the power, 2017-05-29 17:14:26]

Dry run: CQ is trying da patch.

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

[terelius, 2017-05-29 17:15:27]

terelius@webrtc.org changed reviewers:
+ stefan@webrtc.org, tschumim@webrtc.org

[commit-bot: I haz the power, 2017-05-29 17:31:45]

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

[commit-bot: I haz the power, 2017-05-29 17:31:46]

Dry run: This issue passed the CQ dry run.

[tschumi, 2017-05-30 14:23:14]

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

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:457: std::unique_ptr<rtcp::Remb>
rtcp_packet(new rtcp::Remb());
make_unique ?

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1010: for (const auto& kv :
remb_packets) {
What was the reason to introduce remb_packets.
We should be able to cacl the remb_series in the loop at line 1003 right ?

[terelius, 2017-05-30 15:12:48]

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

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:457: std::unique_ptr<rtcp::Remb>
rtcp_packet(new rtcp::Remb());
On 2017/05/30 14:23:13, tschumi wrote:
> make_unique ?

Isn't that C++14? I don't think we can use it.

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1010: for (const auto& kv :
remb_packets) {
On 2017/05/30 14:23:13, tschumi wrote:
> What was the reason to introduce remb_packets.
> We should be able to cacl the remb_series in the loop at line 1003 right ?

That would probably work, but in principle we're not guaranteed to process the
RTCP packets in timestamp order in the loop above. The purpose of remb_packets
is essentially to sort the messages.

[holmer, 2017-05-30 15:28:46]

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

[holmer, 2017-05-30 15:28:46]

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

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:457: std::unique_ptr<rtcp::Remb>
rtcp_packet(new rtcp::Remb());
On 2017/05/30 15:12:48, terelius wrote:
> On 2017/05/30 14:23:13, tschumi wrote:
> > make_unique ?
> 
> Isn't that C++14? I don't think we can use it.

rtc::MakeUnique

[tschumi, 2017-05-31 10:49:51]

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

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1010: for (const auto& kv :
remb_packets) {
On 2017/05/30 15:12:48, terelius wrote:
> On 2017/05/30 14:23:13, tschumi wrote:
> > What was the reason to introduce remb_packets.
> > We should be able to cacl the remb_series in the loop at line 1003 right ?
> 
> That would probably work, but in principle we're not guaranteed to process the
> RTCP packets in timestamp order in the loop above. The purpose of remb_packets
> is essentially to sort the messages.

Acknowledged.

[terelius, 2017-06-01 16:00:40]

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

[commit-bot: I haz the power, 2017-06-01 16:00:48]

Dry run: CQ is trying da patch.

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

[terelius, 2017-06-01 16:02:19]

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

https://codereview.webrtc.org/2912813002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:457: std::unique_ptr<rtcp::Remb>
rtcp_packet(new rtcp::Remb());
On 2017/05/30 15:28:46, holmer wrote:
> On 2017/05/30 15:12:48, terelius wrote:
> > On 2017/05/30 14:23:13, tschumi wrote:
> > > make_unique ?
> > 
> > Isn't that C++14? I don't think we can use it.
> 
> rtc::MakeUnique

Done.

[commit-bot: I haz the power, 2017-06-01 16:03:37]

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

[commit-bot: I haz the power, 2017-06-01 16:03:38]

Dry run: Try jobs failed on following builders:
  ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  linux_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_arm64_rel/builds/...)
  mac_asan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/25105)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/21952)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[terelius, 2017-06-01 16:04:46]

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

[commit-bot: I haz the power, 2017-06-01 16:04:48]

Dry run: CQ is trying da patch.

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

[holmer, 2017-06-01 16:11:23]

lgtm

[commit-bot: I haz the power, 2017-06-01 16:30:42]

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

[commit-bot: I haz the power, 2017-06-01 16:30:43]

Dry run: This issue passed the CQ dry run.

[tschumi, 2017-06-02 08:26:45]

lgtm

[terelius, 2017-06-02 08:27:37]

The CQ bit was checked by terelius@webrtc.org

[commit-bot: I haz the power, 2017-06-02 08:27:47]

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-06-02 08:29:48]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496392057269400,
"parent_rev": "1476a9d789db03457595cf7dbea7e362972f2a4d", "commit_rev":
"2c8e8a306a5c2db8566763d0d898cbd08d05f6cb"}

[commit-bot: I haz the power, 2017-06-02 08:29:51]

Description was changed from

==========
Overlay REMB in total bitrate graphs in visualization tool.

This doesn't affect the production code.

BUG=webrtc:7726
==========

to

==========
Overlay REMB in total bitrate graphs in visualization tool.

This doesn't affect the production code.

BUG=webrtc:7726

Review-Url: https://codereview.webrtc.org/2912813002
Cr-Commit-Position: refs/heads/master@{#18400}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2c8e8a306a5c2db8566763d0d...
==========

[commit-bot: I haz the power, 2017-06-02 08:29:53]

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