Add event log visualization of rtp timestamps over time.

Add event log visualization of rtp timestamps over time.

BUG=None

Review-Url: https://codereview.webrtc.org/2658073002
Cr-Commit-Position: refs/heads/master@{#16417}
Committed: https://chromium.googlesource.com/external/webrtc/+/e372d3c51949672de3a4236f5ff3fcc011d528eb

[stefan-webrtc, 2017-01-26 16:22:57]

Description was changed from

==========
Add plotevent log visualization of rtp timestamps over time.

BUG=None
==========

to

==========
Add event log visualization of rtp timestamps over time.

BUG=None
==========

[stefan-webrtc, 2017-01-26 16:23:04]

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

[stefan-webrtc, 2017-01-26 16:24:15]

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

[commit-bot: I haz the power, 2017-01-26 16:24:22]

Dry run: 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-01-26 17:52:55]

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

[commit-bot: I haz the power, 2017-01-26 17:52:59]

Dry run: This issue passed the CQ dry run.

[terelius, 2017-02-01 13:29:31]

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

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:391: if (media_type ==
MediaType::AUDIO || media_type == MediaType::ANY) {
Won't this mean that we do act on the same RTCP report twice?

[stefan-webrtc, 2017-02-02 13:19:17]

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

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:391: if (media_type ==
MediaType::AUDIO || media_type == MediaType::ANY) {
On 2017/02/01 13:29:31, terelius wrote:
> Won't this mean that we do act on the same RTCP report twice?

No. ANY basically means that the packet is logged on the send-side, while AUDIO
and VIDEO is used on the receive-side. Received RTCP packets are logged both on
VIDEO and AUDIO but not on ANY, while sent RTCP are only logged once and as ANY.

[terelius, 2017-02-02 13:45:56]

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

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:391: if (media_type ==
MediaType::AUDIO || media_type == MediaType::ANY) {
On 2017/02/02 13:19:17, stefan-webrtc wrote:
> On 2017/02/01 13:29:31, terelius wrote:
> > Won't this mean that we do act on the same RTCP report twice?
> 
> No. ANY basically means that the packet is logged on the send-side, while
AUDIO
> and VIDEO is used on the receive-side. Received RTCP packets are logged both
on
> VIDEO and AUDIO but not on ANY, while sent RTCP are only logged once and as
ANY.

Acknowledged.

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1237: first_timestamp =
rtc::Optional<uint32_t>(packet.header.timestamp);
Are we using first_timestamp anywhere?

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1259: first_timestamp =
rtc::Optional<uint32_t>(sr->rtp_timestamp());
Are we using first_timestamp anywhere?

[stefan-webrtc, 2017-02-02 15:00:38]

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

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1237: first_timestamp =
rtc::Optional<uint32_t>(packet.header.timestamp);
On 2017/02/02 13:45:56, terelius wrote:
> Are we using first_timestamp anywhere?

Done.

https://codereview.webrtc.org/2658073002/diff/1/webrtc/tools/event_log_visual...
webrtc/tools/event_log_visualizer/analyzer.cc:1259: first_timestamp =
rtc::Optional<uint32_t>(sr->rtp_timestamp());
On 2017/02/02 13:45:56, terelius wrote:
> Are we using first_timestamp anywhere?

Good point, no!

[terelius, 2017-02-02 15:02:49]

lgtm

[terelius, 2017-02-02 15:02:58]

The CQ bit was checked by terelius@webrtc.org

[commit-bot: I haz the power, 2017-02-02 15:03:09]

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-02-02 16:04:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486047778374070,
"parent_rev": "a55f021d4351d94a257a16b72f45d843841252c1", "commit_rev":
"e372d3c51949672de3a4236f5ff3fcc011d528eb"}

[commit-bot: I haz the power, 2017-02-02 16:04:21]

Description was changed from

==========
Add event log visualization of rtp timestamps over time.

BUG=None
==========

to

==========
Add event log visualization of rtp timestamps over time.

BUG=None

Review-Url: https://codereview.webrtc.org/2658073002
Cr-Commit-Position: refs/heads/master@{#16417}
Committed:
https://chromium.googlesource.com/external/webrtc/+/e372d3c51949672de3a4236f5...
==========

[commit-bot: I haz the power, 2017-02-02 16:04:22]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/e372d3c51949672de3a4236f5...