Reland of Don't hardcode MediaType::ANY in FakeNetworkPipe.

Reland of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #1 id:1 of https://codereview.webrtc.org/2784543002/ )

Reason for revert:
Intend to fix perf failures and reland.

Original issue's description:
> Revert of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #4 id:60001 of https://codereview.webrtc.org/2774463003/ )
> 
> Reason for revert:
> Reverting since this seems to break multiple WebRTC Perf buildbots
> 
> Original issue's description:
> > Don't hardcode MediaType::ANY in FakeNetworkPipe.
> > 
> > Instead let each test set the appropriate media type. This simplifies
> > demuxing in Call and later in RtpTransportController.
> > 
> > BUG=webrtc:7135
> > 
> > Review-Url: https://codereview.webrtc.org/2774463003
> > Cr-Commit-Position: refs/heads/master@{#17418}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/9c47b00e24da2941eb095df5a4459c6d98a8a88d
> 
> TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.org,sprang@webrtc.org,nisse@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7135
> 
> Review-Url: https://codereview.webrtc.org/2784543002
> Cr-Commit-Position: refs/heads/master@{#17427}
> Committed: https://chromium.googlesource.com/external/webrtc/+/3a3bd5061089da5327fc549337a8430054d66057

TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.org,sprang@webrtc.org,lliuu@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7135
Review-Url: https://codereview.webrtc.org/2783853002
Cr-Commit-Position: refs/heads/master@{#17459}
Committed: https://chromium.googlesource.com/external/webrtc/+/e5ad5ca06ab3bfaaecd6dcfa01d136c4e5d8e9b2

[nisse-webrtc, 2017-03-29 08:25:24]

Created Reland of Don't hardcode MediaType::ANY in FakeNetworkPipe.

[nisse-webrtc, 2017-03-29 12:12:54]

Perf tests now work locally (except for a few h.264 tests
which for some reason always seems to fail for me).

[sprang_webrtc, 2017-03-29 12:24:30]

lgtm

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:46: media_type = MediaType::AUDIO;
nit: {} for if/else

[stefan-webrtc, 2017-03-29 12:27:25]

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:105: } else {
Is there a reason why we don't always use the demuxer?

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h
File webrtc/test/call_test.h (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h#...
webrtc/test/call_test.h:65: class PayloadDemuxer : public PacketReceiver {
It seems like PacketReceiver is the wrong type for this, since we don't actually
know the MediaType of the packet at this point. Should it insetad inherit from
something else, e.g., webrtc::Transport?

[nisse-webrtc, 2017-03-29 12:37:55]

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:46: media_type = MediaType::AUDIO;
On 2017/03/29 12:24:30, språng wrote:
> nit: {} for if/else

Done.

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:105: } else {
On 2017/03/29 12:27:25, stefan-webrtc wrote:
> Is there a reason why we don't always use the demuxer?

I'm afraid it is a bit brittle, so I don't want to depend on payload type it's
not really needed.

Hmm, but since the demuxer class now does the payload type demuxing only when it
gets MediaType::ANY as input, it would probably work fine to use it always.

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h
File webrtc/test/call_test.h (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h#...
webrtc/test/call_test.h:65: class PayloadDemuxer : public PacketReceiver {
On 2017/03/29 12:27:25, stefan-webrtc wrote:
> It seems like PacketReceiver is the wrong type for this, since we don't
actually
> know the MediaType of the packet at this point. Should it insetad inherit from
> something else, e.g., webrtc::Transport?

We can't, since this object is intended to be passed to
FakeNetworkPipe::SetReceiver.

[stefan-webrtc, 2017-03-30 06:47:11]

lgtm

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:105: } else {
On 2017/03/29 12:37:55, nisse-webrtc wrote:
> On 2017/03/29 12:27:25, stefan-webrtc wrote:
> > Is there a reason why we don't always use the demuxer?
> 
> I'm afraid it is a bit brittle, so I don't want to depend on payload type it's
> not really needed.
> 
> Hmm, but since the demuxer class now does the payload type demuxing only when
it
> gets MediaType::ANY as input, it would probably work fine to use it always.

I'm ok with both options. Maybe good to have one less if statement. :)

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h
File webrtc/test/call_test.h (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h#...
webrtc/test/call_test.h:65: class PayloadDemuxer : public PacketReceiver {
On 2017/03/29 12:37:55, nisse-webrtc wrote:
> On 2017/03/29 12:27:25, stefan-webrtc wrote:
> > It seems like PacketReceiver is the wrong type for this, since we don't
> actually
> > know the MediaType of the packet at this point. Should it insetad inherit
from
> > something else, e.g., webrtc::Transport?
> 
> We can't, since this object is intended to be passed to
> FakeNetworkPipe::SetReceiver.

Ah, ok. I guess we could if we changed the FakeNetworkPipe? If you think that
makes sense maybe you could leave a TODO here?

[nisse-webrtc, 2017-03-30 06:56:45]

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:105: } else {
On 2017/03/30 06:47:11, stefan-webrtc wrote:
> On 2017/03/29 12:37:55, nisse-webrtc wrote:
> > On 2017/03/29 12:27:25, stefan-webrtc wrote:
> > > Is there a reason why we don't always use the demuxer?
> > 
> > I'm afraid it is a bit brittle, so I don't want to depend on payload type
it's
> > not really needed.
> > 
> > Hmm, but since the demuxer class now does the payload type demuxing only
when
> it
> > gets MediaType::ANY as input, it would probably work fine to use it always.
> 
> I'm ok with both options. Maybe good to have one less if statement. :)

I don't have a strong opinion either. I think I'll leave this unchanged. Then
the tests which are audio-only or video-only (which are the majority) are forced
to configure the FakeNetworkPipe with the correct media type, and won't
accidentally start relying on the somewhat brittle demux logic.

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h
File webrtc/test/call_test.h (right):

https://codereview.webrtc.org/2783853002/diff/300001/webrtc/test/call_test.h#...
webrtc/test/call_test.h:65: class PayloadDemuxer : public PacketReceiver {
On 2017/03/30 06:47:11, stefan-webrtc wrote:
> On 2017/03/29 12:37:55, nisse-webrtc wrote:
> > On 2017/03/29 12:27:25, stefan-webrtc wrote:
> > > It seems like PacketReceiver is the wrong type for this, since we don't
> > actually
> > > know the MediaType of the packet at this point. Should it insetad inherit
> from
> > > something else, e.g., webrtc::Transport?
> > 
> > We can't, since this object is intended to be passed to
> > FakeNetworkPipe::SetReceiver.
> 
> Ah, ok. I guess we could if we changed the FakeNetworkPipe? If you think that
> makes sense maybe you could leave a TODO here?

For now, I think it makes sense for most tests to configure FakeNetworkPipe with
a media type. If we do any refactorings there, it might make sense to make media
type be a per-packet property in 
FakeNetworkPipe. But it's unclear what changes would be needed at input to the
FakeNetworkPipe to get that right.

[nisse-webrtc, 2017-03-30 06:57:26]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-03-30 06:57:26]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2783853002/#ps320001 (title:
"Fix braces. Add comment on use of demuxer.")

[commit-bot: I haz the power, 2017-03-30 06:57:37]

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-03-30 06:57:44]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1490857046038060,
"parent_rev": "1dcb16409a3e82518ef852ec5614f87dff04f248", "commit_rev":
"e5ad5ca06ab3bfaaecd6dcfa01d136c4e5d8e9b2"}

[commit-bot: I haz the power, 2017-03-30 06:57:46]

Description was changed from

==========
Reland of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #1 id:1 of
https://codereview.webrtc.org/2784543002/ )

Reason for revert:
Intend to fix perf failures and reland.

Original issue's description:
> Revert of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #4
id:60001 of https://codereview.webrtc.org/2774463003/ )
> 
> Reason for revert:
> Reverting since this seems to break multiple WebRTC Perf buildbots
> 
> Original issue's description:
> > Don't hardcode MediaType::ANY in FakeNetworkPipe.
> > 
> > Instead let each test set the appropriate media type. This simplifies
> > demuxing in Call and later in RtpTransportController.
> > 
> > BUG=webrtc:7135
> > 
> > Review-Url: https://codereview.webrtc.org/2774463003
> > Cr-Commit-Position: refs/heads/master@{#17418}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/9c47b00e24da2941eb095df5a...
> 
>
TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.or...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7135
> 
> Review-Url: https://codereview.webrtc.org/2784543002
> Cr-Commit-Position: refs/heads/master@{#17427}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/3a3bd5061089da5327fc54933...

TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.or...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7135
==========

to

==========
Reland of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #1 id:1 of
https://codereview.webrtc.org/2784543002/ )

Reason for revert:
Intend to fix perf failures and reland.

Original issue's description:
> Revert of Don't hardcode MediaType::ANY in FakeNetworkPipe. (patchset #4
id:60001 of https://codereview.webrtc.org/2774463003/ )
> 
> Reason for revert:
> Reverting since this seems to break multiple WebRTC Perf buildbots
> 
> Original issue's description:
> > Don't hardcode MediaType::ANY in FakeNetworkPipe.
> > 
> > Instead let each test set the appropriate media type. This simplifies
> > demuxing in Call and later in RtpTransportController.
> > 
> > BUG=webrtc:7135
> > 
> > Review-Url: https://codereview.webrtc.org/2774463003
> > Cr-Commit-Position: refs/heads/master@{#17418}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/9c47b00e24da2941eb095df5a...
> 
>
TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.or...
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7135
> 
> Review-Url: https://codereview.webrtc.org/2784543002
> Cr-Commit-Position: refs/heads/master@{#17427}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/3a3bd5061089da5327fc54933...

TBR=stefan@webrtc.org,deadbeef@webrtc.org,solenberg@webrtc.org,pbos@webrtc.or...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7135

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

[commit-bot: I haz the power, 2017-03-30 06:57:48]

Committed patchset #4 (id:320001) as
https://chromium.googlesource.com/external/webrtc/+/e5ad5ca06ab3bfaaecd6dcfa0...

[perkj_webrtc, 2017-03-30 09:13:24]

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

[perkj_webrtc, 2017-03-30 09:13:25]

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/direct_trans...
File webrtc/test/direct_transport.h (right):

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/direct_trans...
webrtc/test/direct_transport.h:33: DirectTransport(Call* send_call, MediaType
media_type);
How would you write a call test that test audio and video combined? ie, 
Test joint bwe etc?

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/fake_network...
File webrtc/test/fake_network_pipe.h (right):

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/fake_network...
webrtc/test/fake_network_pipe.h:87: MediaType media_type);
This will not work for tests that mux audio and video on the same transport. The
fake network pipe then needs to know per packet what type it is.

[nisse-webrtc, 2017-03-30 09:22:35]

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/fake_network...
File webrtc/test/fake_network_pipe.h (right):

https://codereview.webrtc.org/2783853002/diff/320001/webrtc/test/fake_network...
webrtc/test/fake_network_pipe.h:87: MediaType media_type);
On 2017/03/30 09:13:24, perkj_webrtc wrote:
> This will not work for tests that mux audio and video on the same transport.
The
> fake network pipe then needs to know per packet what type it is. 

For webrtc tests, I fixed that case by adding a payload-type demuxer, see this
cl's changes to call_test.cc.

Letting the network pipe keep track of media type per packet would make sense to
me, but I haven't looked into how to make that work. We'd then need media type
config at all places where packets are inserted into the pipe.