zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MediaTransportTests should use audio-only peer connection.

Browse Source 
Currently (and this has to change), media transport is created two times if audio&video is used (even if bundling is enabled).
The second time it's destroyed really quickly (but given lack of 'Connect' method, the connection has already started).

This change adds a TODO and modifies existing tests to prevent creation of 2 media transports.



Bug: webrtc:9719
Change-Id: I872e98dcd10685beb0326d501f0e0abf36c0fdfc
Reviewed-on: https://webrtc-review.googlesource.com/c/110887
Reviewed-by: Anton Sukhanov <sukhanov@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Peter Slatala <psla@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25660}
This commit is contained in:
Piotr (Peter) Slatala
2018-11-14 10:57:24 -08:00
committed by Commit Bot
parent 0462948c9c
commit 10aeb2a5dc
2 changed files with 25 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pc/jseptransportcontroller.cc
View File

@ -762,6 +762,10 @@ bool JsepTransportController::HandleBundledContent(
// BaseChannel/SctpTransport change the RtpTransport/DtlsTransport first,
// then destroy the cricket::JsepTransport.
if (SetTransportForMid(content_info.name, jsep_transport)) {
// TODO(bugs.webrtc.org/9719) For media transport this is far from ideal,
// because it means that we first create media transport and start
// connecting it, and then we destroy it. We will need to address it before
// video path is enabled.
MaybeDestroyJsepTransport(content_info.name);
return true;
}

42
pc/peerconnection_media_unittest.cc
View File

@ -112,6 +112,18 @@ class PeerConnectionMediaBaseTest : public ::testing::Test {
return wrapper;
}
// Accepts the same arguments as CreatePeerConnection and adds default audio
// track (but no video).
template <typename... Args>
WrapperPtr CreatePeerConnectionWithAudio(Args&&... args) {
auto wrapper = CreatePeerConnection(std::forward<Args>(args)...);
if (!wrapper) {
return nullptr;
}
wrapper->AddAudioTrack("a");
return wrapper;
}
// Accepts the same arguments as CreatePeerConnection and adds default audio
// and video tracks.
template <typename... Args>
@ -1096,8 +1108,8 @@ TEST_P(PeerConnectionMediaTest, MediaTransportPropagatedToVoiceEngine) {
// Force SDES.
config.enable_dtls_srtp = false;
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto caller = CreatePeerConnectionWithAudio(config);
auto callee = CreatePeerConnectionWithAudio(config);
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
auto answer = callee->CreateAnswer();
@ -1120,13 +1132,9 @@ TEST_P(PeerConnectionMediaTest, MediaTransportPropagatedToVoiceEngine) {
EXPECT_TRUE(caller_voice_media_transport->is_caller());
EXPECT_FALSE(callee_voice_media_transport->is_caller());
// TODO(sukhanov): Propagate media transport to video channel. This test
// will fail once media transport is propagated to video channel and it will
// serve as a reminder to add a test for video channel propagation.
auto caller_video = caller->media_engine()->GetVideoChannel(0);
auto callee_video = callee->media_engine()->GetVideoChannel(0);
ASSERT_EQ(nullptr, caller_video->media_transport());
ASSERT_EQ(nullptr, callee_video->media_transport());
// TODO(sukhanov): Propagate media transport to video channel.
// This test does NOT set up video channels, because currently it causes
// us to create two media transports.
}
TEST_P(PeerConnectionMediaTest, MediaTransportOnlyForDataChannels) {
@ -1138,8 +1146,8 @@ TEST_P(PeerConnectionMediaTest, MediaTransportOnlyForDataChannels) {
// Force SDES.
config.enable_dtls_srtp = false;
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto caller = CreatePeerConnectionWithAudio(config);
auto callee = CreatePeerConnectionWithAudio(config);
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
@ -1165,8 +1173,8 @@ TEST_P(PeerConnectionMediaTest, MediaTransportForMediaAndDataChannels) {
// Force SDES.
config.enable_dtls_srtp = false;
auto caller = CreatePeerConnectionWithAudioVideo(config);
auto callee = CreatePeerConnectionWithAudioVideo(config);
auto caller = CreatePeerConnectionWithAudio(config);
auto callee = CreatePeerConnectionWithAudio(config);
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
ASSERT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
@ -1187,14 +1195,6 @@ TEST_P(PeerConnectionMediaTest, MediaTransportForMediaAndDataChannels) {
// Make sure media transport is created with correct is_caller.
EXPECT_TRUE(caller_voice_media_transport->is_caller());
EXPECT_FALSE(callee_voice_media_transport->is_caller());
// TODO(sukhanov): Propagate media transport to video channel. This test
// will fail once media transport is propagated to video channel and it will
// serve as a reminder to add a test for video channel propagation.
auto caller_video = caller->media_engine()->GetVideoChannel(0);
auto callee_video = callee->media_engine()->GetVideoChannel(0);
ASSERT_EQ(nullptr, caller_video->media_transport());
ASSERT_EQ(nullptr, callee_video->media_transport());
}
TEST_P(PeerConnectionMediaTest, MediaTransportNotPropagatedToVoiceEngine) {