Removed RtcEventLog deps to call:call_interfaces.

This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the media type from the configurations instead. 


BUG=webrtc:7538
TBR=solenberg@webrtc.org // For call.cc and voiceengine.cc

Review-Url: https://codereview.webrtc.org/2855143002
Cr-Commit-Position: refs/heads/master@{#18324}
Committed: https://chromium.googlesource.com/external/webrtc/+/77cd58e140fb8046a7f918c88530ccb6f380bf11

[perkj_webrtc, 2017-05-03 11:32:16]

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

[commit-bot: I haz the power, 2017-05-03 11:32:20]

Dry run: CQ is trying da patch.

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

[perkj_webrtc, 2017-05-03 11:32:32]

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

[perkj_webrtc, 2017-05-03 11:32:32]

Please?

[commit-bot: I haz the power, 2017-05-03 12:24:45]

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

[commit-bot: I haz the power, 2017-05-03 12:24:46]

Dry run: This issue passed the CQ dry run.

[perkj_webrtc, 2017-05-03 12:32:57]

perkj@webrtc.org changed reviewers:
+ danilchap@webrtc.org

[perkj_webrtc, 2017-05-03 12:32:58]

+ danil for RTP module.

[danilchap, 2017-05-03 12:39:38]

rtp_rtcp lgtm

[terelius, 2017-05-18 16:23:44]

I'd prefer to update the parser and dependent tools in the same CL.

[perkj_webrtc, 2017-05-18 17:44:22]

On 2017/05/18 16:23:44, terelius wrote:
> I'd prefer to update the parser and dependent tools in the same CL.

What do you mean? Remove the field from the proto?

[perkj_webrtc, 2017-05-22 19:01:13]

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

[commit-bot: I haz the power, 2017-05-22 19:01:23]

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-05-22 19:24:25]

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

[commit-bot: I haz the power, 2017-05-22 19:24:26]

Dry run: This issue passed the CQ dry run.

[perkj_webrtc, 2017-05-23 09:27:09]

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

[commit-bot: I haz the power, 2017-05-23 09:27:16]

Dry run: CQ is trying da patch.

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

[perkj_webrtc, 2017-05-23 10:46:26]

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

[commit-bot: I haz the power, 2017-05-23 10:46:33]

Dry run: CQ is trying da patch.

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

[perkj_webrtc, 2017-05-23 10:49:15]

Description was changed from

==========
Remove MediaType as argument to RtcEventLog::LogRtpHeader and
RtcEventLog::LogRtcpHeader Instead MediaType::Any is always written to the
proto.

BUG=webrtc:7538
==========

to

==========
This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be
able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader
and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the
media type from the configurations instead. 


BUG=webrtc:7538
==========

[perkj_webrtc, 2017-05-23 10:50:50]

perkj@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[perkj_webrtc, 2017-05-23 10:50:51]

Terelius- can you please review it all? 
Henrik, can you please review voiceengine.cc and the neteq test changes?

[terelius, 2017-05-23 11:57:58]

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:49: struct Stream {
If this struct is used by all/most tools, maybe the definition should be moved
to the parser?

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:58: std::vector<Stream>
global_streams;
Another alternative is to make this a part of the parser, so you can ask the
parser whether a packet is adio or video. I'm fine with either way though.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:233: MediaType media_type
= GetMediaType(packet_ssrc, direction);
Please add a TODO or comment documenting that media types for RTCP doesn't work.
Or maybe we should remove the RTCP filtering completely?

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log2text.cc (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2text.cc:118: const char*
StreamInfo(webrtc::PacketDirection direction,
This could be a toString() on the Stream object.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/BUILD.gn (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/BUILD.gn:89: "../../base:rtc_base",
Why do we need to add this? I thought we only wanted to depend on
rtc_base_approved, but maybe the meaning changed?

[commit-bot: I haz the power, 2017-05-23 12:19:11]

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

[commit-bot: I haz the power, 2017-05-23 12:19:12]

Dry run: This issue passed the CQ dry run.

[hlundin-webrtc, 2017-05-23 13:00:31]

webrtc/modules/audio_coding: LGTM
webrtc/voice_engine: LG, but I am not an OWNER

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h:54: // SSRCs of
incoming Audio streams found in the event log.
Nit: Audio -> audio

[perkj_webrtc, 2017-05-24 12:21:24]

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

[commit-bot: I haz the power, 2017-05-24 12:21:29]

Dry run: CQ is trying da patch.

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

[perkj_webrtc, 2017-05-24 12:32:21]

PTAL

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:49: struct Stream {
On 2017/05/23 11:57:58, terelius wrote:
> If this struct is used by all/most tools, maybe the definition should be moved
> to the parser?

Done.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:58: std::vector<Stream>
global_streams;
On 2017/05/23 11:57:58, terelius wrote:
> Another alternative is to make this a part of the parser, so you can ask the
> parser whether a packet is adio or video. I'm fine with either way though.

Done.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log2text.cc (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log2text.cc:118: const char*
StreamInfo(webrtc::PacketDirection direction,
On 2017/05/23 11:57:58, terelius wrote:
> This could be a toString() on the Stream object.

Prefer not to. I moved the Stream into the parser.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h:54: // SSRCs of
incoming Audio streams found in the event log.
On 2017/05/23 13:00:31, hlundin-webrtc wrote:
> Nit: Audio -> audio

Done.

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
File webrtc/modules/congestion_controller/BUILD.gn (right):

https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
webrtc/modules/congestion_controller/BUILD.gn:89: "../../base:rtc_base",
On 2017/05/23 11:57:58, terelius wrote:
> Why do we need to add this? I thought we only wanted to depend on
> rtc_base_approved, but maybe the meaning changed?

It uses webrtc/base/socket.h
rtc::SentPacket
CongestionController already depend on base. 
 It previously probably worked since call.h also includes socket.h

[perkj_webrtc, 2017-05-30 05:06:50]

On 2017/05/24 12:32:21, perkj_webrtc wrote:
> PTAL
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
> File webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc (right):
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
> webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:49: struct Stream {
> On 2017/05/23 11:57:58, terelius wrote:
> > If this struct is used by all/most tools, maybe the definition should be
moved
> > to the parser?
> 
> Done.
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
> webrtc/logging/rtc_event_log/rtc_event_log2rtp_dump.cc:58: std::vector<Stream>
> global_streams;
> On 2017/05/23 11:57:58, terelius wrote:
> > Another alternative is to make this a part of the parser, so you can ask the
> > parser whether a packet is adio or video. I'm fine with either way though.
> 
> Done.
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
> File webrtc/logging/rtc_event_log/rtc_event_log2text.cc (right):
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/logging/rtc_event...
> webrtc/logging/rtc_event_log/rtc_event_log2text.cc:118: const char*
> StreamInfo(webrtc::PacketDirection direction,
> On 2017/05/23 11:57:58, terelius wrote:
> > This could be a toString() on the Stream object.
> 
> Prefer not to. I moved the Stream into the parser.
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
> File webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h (right):
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/audio_cod...
> webrtc/modules/audio_coding/neteq/tools/rtc_event_log_source.h:54: // SSRCs of
> incoming Audio streams found in the event log.
> On 2017/05/23 13:00:31, hlundin-webrtc wrote:
> > Nit: Audio -> audio
> 
> Done.
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
> File webrtc/modules/congestion_controller/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2855143002/diff/120001/webrtc/modules/congestio...
> webrtc/modules/congestion_controller/BUILD.gn:89: "../../base:rtc_base",
> On 2017/05/23 11:57:58, terelius wrote:
> > Why do we need to add this? I thought we only wanted to depend on
> > rtc_base_approved, but maybe the meaning changed?
> 
> It uses webrtc/base/socket.h
> rtc::SentPacket
> CongestionController already depend on base. 
>  It previously probably worked since call.h also includes socket.h

Ping!

[terelius, 2017-05-30 08:11:19]

lgtm with one nit:

https://codereview.webrtc.org/2855143002/diff/160001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_parser.h (right):

https://codereview.webrtc.org/2855143002/diff/160001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_parser.h:119: void
GetVideoReceiveConfig(const rtclog::Event& event,
Could we make these helper functions private?

[perkj_webrtc, 2017-05-30 10:17:19]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2017-05-30 10:17:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, henrik.lundin@webrtc.org, terelius@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2855143002/#ps180001 (title:
"Made GetSend/ReceiveConfig private.")

[commit-bot: I haz the power, 2017-05-30 10:17:21]

CQ is trying da patch.

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

[perkj_webrtc, 2017-05-30 10:17:54]

https://codereview.webrtc.org/2855143002/diff/160001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_parser.h (right):

https://codereview.webrtc.org/2855143002/diff/160001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_parser.h:119: void
GetVideoReceiveConfig(const rtclog::Event& event,
On 2017/05/30 08:11:19, terelius wrote:
> Could we make these helper functions private?

yes

[commit-bot: I haz the power, 2017-05-30 10:21:34]

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

[commit-bot: I haz the power, 2017-05-30 10:21:36]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17545)

[perkj_webrtc, 2017-05-30 10:49:31]

Description was changed from

==========
This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be
able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader
and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the
media type from the configurations instead. 


BUG=webrtc:7538
==========

to

==========
This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be
able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader
and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the
media type from the configurations instead. 


BUG=webrtc:7538
TBR=solenberg@webrtc.org // For call.cc and voiceengine.cc
==========

[perkj_webrtc, 2017-05-30 10:49:45]

perkj@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[perkj_webrtc, 2017-05-30 10:49:54]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2017-05-30 10:49:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, henrik.lundin@webrtc.org, terelius@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2855143002/#ps180001 (title:
"Made GetSend/ReceiveConfig private.")

[commit-bot: I haz the power, 2017-05-30 10:49:56]

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-05-30 10:52:11]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1496141394051440,
"parent_rev": "367aba92bf14bd1101fa61cb5766c766fde86152", "commit_rev":
"77cd58e140fb8046a7f918c88530ccb6f380bf11"}

[commit-bot: I haz the power, 2017-05-30 10:52:15]

Description was changed from

==========
This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be
able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader
and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the
media type from the configurations instead. 


BUG=webrtc:7538
TBR=solenberg@webrtc.org // For call.cc and voiceengine.cc
==========

to

==========
This cl removes RtcEventLog deps to call:call_interfaces. The purpose is to be
able to use the event log from the upcoming RtpTransport. 

Biggest change is to Remove MediaType as argument to RtcEventLog::LogRtpHeader
and RtcEventLog::LogRtcpHeader.
Since the type is used by tools, these tools are rewritten to figure out the
media type from the configurations instead. 


BUG=webrtc:7538
TBR=solenberg@webrtc.org // For call.cc and voiceengine.cc

Review-Url: https://codereview.webrtc.org/2855143002
Cr-Commit-Position: refs/heads/master@{#18324}
Committed:
https://chromium.googlesource.com/external/webrtc/+/77cd58e140fb8046a7f918c88...
==========

[commit-bot: I haz the power, 2017-05-30 10:52:17]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/external/webrtc/+/77cd58e140fb8046a7f918c88...