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

webrtc/test/call_test.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/test/call_test.h"
 
 #include <algorithm>
 
 #include "webrtc/api/audio_codecs/builtin_audio_decoder_factory.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/config.h"
 #include "webrtc/modules/audio_mixer/audio_mixer_impl.h"
 #include "webrtc/test/testsupport/fileutils.h"
 #include "webrtc/voice_engine/include/voe_base.h"
 
 namespace webrtc {
 namespace test {
 
 namespace {
 const int kVideoRotationRtpExtensionId = 4;
 }
 
+void CallTest::PayloadDemuxer::SetReceiver(PacketReceiver* receiver) {
+  receiver_ = receiver;
+}
+
+PacketReceiver::DeliveryStatus CallTest::PayloadDemuxer::DeliverPacket(
+    MediaType media_type,
+    const uint8_t* packet,
+    size_t length,
+    const PacketTime& packet_time) {
+  if (media_type == MediaType::ANY) {
+    // This simplistic demux logic will not make much sense for RTCP
+    // packets, but it seems that doesn't matter.
+    RTC_CHECK_GE(length, 2);
+    uint8_t pt = packet[1] & 0x7f;
+    if (pt == kFakeVideoSendPayloadType || pt == kFlexfecPayloadType)
+      media_type = MediaType::VIDEO;
+    else
+      media_type = MediaType::AUDIO;

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

nit: {} for if/else

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

Done.

+  }
+  return receiver_->DeliverPacket(media_type, packet, length, packet_time);
+}
+
 CallTest::CallTest()
     : clock_(Clock::GetRealTimeClock()),
       video_send_config_(nullptr),
       video_send_stream_(nullptr),
       audio_send_config_(nullptr),
       audio_send_stream_(nullptr),
       fake_encoder_(clock_),
       num_video_streams_(1),
       num_audio_streams_(0),
       num_flexfec_streams_(0),
@@ skipping 29 matching lines @@
       audio_state_config.audio_mixer = AudioMixerImpl::Create();
       recv_config.audio_state = AudioState::Create(audio_state_config);
     }
     CreateReceiverCall(recv_config);
   }
   test->OnCallsCreated(sender_call_.get(), receiver_call_.get());
   receive_transport_.reset(test->CreateReceiveTransport());
   send_transport_.reset(test->CreateSendTransport(sender_call_.get()));
 
   if (test->ShouldCreateReceivers()) {
-    send_transport_->SetReceiver(receiver_call_->Receiver());
-    receive_transport_->SetReceiver(sender_call_->Receiver());
+    if (num_video_streams_ > 0 && num_audio_streams_ > 0) {
+      receive_demuxer_.SetReceiver(receiver_call_->Receiver());
+      send_transport_->SetReceiver(&receive_demuxer_);
+      send_demuxer_.SetReceiver(sender_call_->Receiver());
+      receive_transport_->SetReceiver(&send_demuxer_);
+    } else {

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

Is there a reason why we don't always use the demuxer?

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

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.

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

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

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

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.

+      send_transport_->SetReceiver(receiver_call_->Receiver());
+      receive_transport_->SetReceiver(sender_call_->Receiver());
+    }
     if (num_video_streams_ > 0)
       receiver_call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
     if (num_audio_streams_ > 0)
       receiver_call_->SignalChannelNetworkState(MediaType::AUDIO, kNetworkUp);
   } else {
     // Sender-only call delivers to itself.
     send_transport_->SetReceiver(sender_call_->Receiver());
     receive_transport_->SetReceiver(nullptr);
   }
 
@@ skipping 361 matching lines @@
 
 Call::Config BaseTest::GetReceiverCallConfig() {
   return Call::Config(&event_log_);
 }
 
 void BaseTest::OnCallsCreated(Call* sender_call, Call* receiver_call) {
 }
 
 test::PacketTransport* BaseTest::CreateSendTransport(Call* sender_call) {
   return new PacketTransport(sender_call, this, test::PacketTransport::kSender,
+                             MediaType::VIDEO,
                              FakeNetworkPipe::Config());
 }
 
 test::PacketTransport* BaseTest::CreateReceiveTransport() {
   return new PacketTransport(nullptr, this, test::PacketTransport::kReceiver,
+                             MediaType::VIDEO,
                              FakeNetworkPipe::Config());
 }
 
 size_t BaseTest::GetNumVideoStreams() const {
   return 1;
 }
 
 size_t BaseTest::GetNumAudioStreams() const {
   return 0;
 }
@@ skipping 47 matching lines @@
 
 EndToEndTest::EndToEndTest(unsigned int timeout_ms) : BaseTest(timeout_ms) {
 }
 
 bool EndToEndTest::ShouldCreateReceivers() const {
   return true;
 }
 
 }  // namespace test
 }  // namespace webrtc