diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index 3ddb3560de..fecc19ba0e 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -463,27 +463,16 @@ bool ConvertRtcOptionsForOffer( return false; } - // According to the spec, offer to receive audio/video if the constraint is - // not set and there are send streams. - if (rtc_options.offer_to_receive_audio == RTCOfferAnswerOptions::kUndefined) { - session_options->recv_audio = - session_options->HasSendMediaStream(cricket::MEDIA_TYPE_AUDIO); - } else { + if (rtc_options.offer_to_receive_audio != RTCOfferAnswerOptions::kUndefined) { session_options->recv_audio = (rtc_options.offer_to_receive_audio > 0); } - if (rtc_options.offer_to_receive_video == RTCOfferAnswerOptions::kUndefined) { - session_options->recv_video = - session_options->HasSendMediaStream(cricket::MEDIA_TYPE_VIDEO); - } else { + if (rtc_options.offer_to_receive_video != RTCOfferAnswerOptions::kUndefined) { session_options->recv_video = (rtc_options.offer_to_receive_video > 0); } session_options->vad_enabled = rtc_options.voice_activity_detection; session_options->transport_options.ice_restart = rtc_options.ice_restart; - session_options->bundle_enabled = - rtc_options.use_rtp_mux && - (session_options->has_audio() || session_options->has_video() || - session_options->has_data()); + session_options->bundle_enabled = rtc_options.use_rtp_mux; return true; } @@ -525,10 +514,6 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints, // kUseRtpMux defaults to true according to spec. session_options->bundle_enabled = true; } - session_options->bundle_enabled = - session_options->bundle_enabled && - (session_options->has_audio() || session_options->has_video() || - session_options->has_data()); if (FindConstraint(constraints, MediaConstraintsInterface::kIceRestart, &value, &mandatory_constraints_satisfied)) { @@ -1043,8 +1028,8 @@ void PeerConnection::SetRemoteDescription( audio_content->description); UpdateRemoteStreamsList(GetActiveStreams(desc), desc->type(), new_streams); remote_info_.default_audio_track_needed = - MediaContentDirectionHasSend(desc->direction()) && - desc->streams().empty(); + !remote_desc->msid_supported() && desc->streams().empty() && + MediaContentDirectionHasSend(desc->direction()); } // Find all video rtp streams and create corresponding remote VideoTracks @@ -1056,8 +1041,8 @@ void PeerConnection::SetRemoteDescription( video_content->description); UpdateRemoteStreamsList(GetActiveStreams(desc), desc->type(), new_streams); remote_info_.default_video_track_needed = - MediaContentDirectionHasSend(desc->direction()) && - desc->streams().empty(); + !remote_desc->msid_supported() && desc->streams().empty() && + MediaContentDirectionHasSend(desc->direction()); } // Update the DataChannels with the information from the remote peer. @@ -1396,12 +1381,28 @@ void PeerConnection::PostCreateSessionDescriptionFailure( bool PeerConnection::GetOptionsForOffer( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, cricket::MediaSessionOptions* session_options) { - SetStreams(session_options, local_streams_, rtp_data_channels_); - if (!ConvertRtcOptionsForOffer(rtc_options, session_options)) { return false; } + SetStreams(session_options, local_streams_, rtp_data_channels_); + // Offer to receive audio/video if the constraint is not set and there are + // send streams, or we're currently receiving. + if (rtc_options.offer_to_receive_audio == RTCOfferAnswerOptions::kUndefined) { + session_options->recv_audio = + session_options->HasSendMediaStream(cricket::MEDIA_TYPE_AUDIO) || + !remote_audio_tracks_.empty(); + } + if (rtc_options.offer_to_receive_video == RTCOfferAnswerOptions::kUndefined) { + session_options->recv_video = + session_options->HasSendMediaStream(cricket::MEDIA_TYPE_VIDEO) || + !remote_video_tracks_.empty(); + } + session_options->bundle_enabled = + session_options->bundle_enabled && + (session_options->has_audio() || session_options->has_video() || + session_options->has_data()); + if (session_->data_channel_type() == cricket::DCT_SCTP && HasDataChannels()) { session_options->data_channel_type = cricket::DCT_SCTP; } @@ -1411,14 +1412,18 @@ bool PeerConnection::GetOptionsForOffer( bool PeerConnection::GetOptionsForAnswer( const MediaConstraintsInterface* constraints, cricket::MediaSessionOptions* session_options) { - SetStreams(session_options, local_streams_, rtp_data_channels_); session_options->recv_audio = false; session_options->recv_video = false; - if (!ParseConstraintsForAnswer(constraints, session_options)) { return false; } + SetStreams(session_options, local_streams_, rtp_data_channels_); + session_options->bundle_enabled = + session_options->bundle_enabled && + (session_options->has_audio() || session_options->has_video() || + session_options->has_data()); + // RTP data channel is handled in MediaSessionOptions::AddStream. SCTP streams // are not signaled in the SDP so does not go through that path and must be // handled here. diff --git a/talk/app/webrtc/peerconnection.h b/talk/app/webrtc/peerconnection.h index c47f90372e..2d388ae9f9 100644 --- a/talk/app/webrtc/peerconnection.h +++ b/talk/app/webrtc/peerconnection.h @@ -51,8 +51,6 @@ typedef std::vector // Populates |session_options| from |rtc_options|, and returns true if options // are valid. -// Send streams should already be added to |session_options| before this method -// is called, as this affects the values of recv_audio and recv_video. bool ConvertRtcOptionsForOffer( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, cricket::MediaSessionOptions* session_options); diff --git a/talk/app/webrtc/peerconnectioninterface_unittest.cc b/talk/app/webrtc/peerconnectioninterface_unittest.cc index bee6380bf4..63163fd651 100644 --- a/talk/app/webrtc/peerconnectioninterface_unittest.cc +++ b/talk/app/webrtc/peerconnectioninterface_unittest.cc @@ -260,6 +260,7 @@ using webrtc::DataChannelInterface; using webrtc::FakeConstraints; using webrtc::FakePortAllocatorFactory; using webrtc::IceCandidateInterface; +using webrtc::MediaConstraintsInterface; using webrtc::MediaStream; using webrtc::MediaStreamInterface; using webrtc::MediaStreamTrackInterface; @@ -658,26 +659,30 @@ class PeerConnectionInterfaceTest : public testing::Test { observer_.renegotiation_needed_ = false; } - bool DoCreateOfferAnswer(SessionDescriptionInterface** desc, bool offer) { + bool DoCreateOfferAnswer(SessionDescriptionInterface** desc, + bool offer, + MediaConstraintsInterface* constraints) { rtc::scoped_refptr observer(new rtc::RefCountedObject< MockCreateSessionDescriptionObserver>()); if (offer) { - pc_->CreateOffer(observer, NULL); + pc_->CreateOffer(observer, constraints); } else { - pc_->CreateAnswer(observer, NULL); + pc_->CreateAnswer(observer, constraints); } EXPECT_EQ_WAIT(true, observer->called(), kTimeout); *desc = observer->release_desc(); return observer->result(); } - bool DoCreateOffer(SessionDescriptionInterface** desc) { - return DoCreateOfferAnswer(desc, true); + bool DoCreateOffer(SessionDescriptionInterface** desc, + MediaConstraintsInterface* constraints) { + return DoCreateOfferAnswer(desc, true, constraints); } - bool DoCreateAnswer(SessionDescriptionInterface** desc) { - return DoCreateOfferAnswer(desc, false); + bool DoCreateAnswer(SessionDescriptionInterface** desc, + MediaConstraintsInterface* constraints) { + return DoCreateOfferAnswer(desc, false, constraints); } bool DoSetSessionDescription(SessionDescriptionInterface* desc, bool local) { @@ -737,7 +742,7 @@ class PeerConnectionInterfaceTest : public testing::Test { void CreateOfferAsRemoteDescription() { rtc::scoped_ptr offer; - ASSERT_TRUE(DoCreateOffer(offer.use())); + ASSERT_TRUE(DoCreateOffer(offer.use(), nullptr)); std::string sdp; EXPECT_TRUE(offer->ToString(&sdp)); SessionDescriptionInterface* remote_offer = @@ -757,7 +762,7 @@ class PeerConnectionInterfaceTest : public testing::Test { void CreateAnswerAsLocalDescription() { scoped_ptr answer; - ASSERT_TRUE(DoCreateAnswer(answer.use())); + ASSERT_TRUE(DoCreateAnswer(answer.use(), nullptr)); // TODO(perkj): Currently SetLocalDescription fails if any parameters in an // audio codec change, even if the parameter has nothing to do with @@ -777,7 +782,7 @@ class PeerConnectionInterfaceTest : public testing::Test { void CreatePrAnswerAsLocalDescription() { scoped_ptr answer; - ASSERT_TRUE(DoCreateAnswer(answer.use())); + ASSERT_TRUE(DoCreateAnswer(answer.use(), nullptr)); std::string sdp; EXPECT_TRUE(answer->ToString(&sdp)); @@ -797,7 +802,7 @@ class PeerConnectionInterfaceTest : public testing::Test { void CreateOfferAsLocalDescription() { rtc::scoped_ptr offer; - ASSERT_TRUE(DoCreateOffer(offer.use())); + ASSERT_TRUE(DoCreateOffer(offer.use(), nullptr)); // TODO(perkj): Currently SetLocalDescription fails if any parameters in an // audio codec change, even if the parameter has nothing to do with // receiving. Not all parameters are serialized to SDP. @@ -967,7 +972,7 @@ TEST_F(PeerConnectionInterfaceTest, AddedStreamsPresentInOffer) { CreatePeerConnection(); AddAudioVideoStream(kStreamLabel1, "audio_track", "video_track"); scoped_ptr offer; - ASSERT_TRUE(DoCreateOffer(offer.accept())); + ASSERT_TRUE(DoCreateOffer(offer.accept(), nullptr)); const cricket::ContentInfo* audio_content = cricket::GetFirstAudioContent(offer->description()); @@ -988,7 +993,7 @@ TEST_F(PeerConnectionInterfaceTest, AddedStreamsPresentInOffer) { // Add another stream and ensure the offer includes both the old and new // streams. AddAudioVideoStream(kStreamLabel2, "audio_track2", "video_track2"); - ASSERT_TRUE(DoCreateOffer(offer.accept())); + ASSERT_TRUE(DoCreateOffer(offer.accept(), nullptr)); audio_content = cricket::GetFirstAudioContent(offer->description()); audio_desc = static_cast( @@ -1084,12 +1089,12 @@ TEST_F(PeerConnectionInterfaceTest, IceCandidates) { // SetRemoteDescription takes ownership of offer. SessionDescriptionInterface* offer = NULL; AddVideoStream(kStreamLabel1); - EXPECT_TRUE(DoCreateOffer(&offer)); + EXPECT_TRUE(DoCreateOffer(&offer, nullptr)); EXPECT_TRUE(DoSetRemoteDescription(offer)); // SetLocalDescription takes ownership of answer. SessionDescriptionInterface* answer = NULL; - EXPECT_TRUE(DoCreateAnswer(&answer)); + EXPECT_TRUE(DoCreateAnswer(&answer, nullptr)); EXPECT_TRUE(DoSetLocalDescription(answer)); EXPECT_TRUE_WAIT(observer_.last_candidate_.get() != NULL, kTimeout); @@ -1104,7 +1109,7 @@ TEST_F(PeerConnectionInterfaceTest, CreateOfferAnswerWithInvalidStream) { CreatePeerConnection(); // Create a regular offer for the CreateAnswer test later. SessionDescriptionInterface* offer = NULL; - EXPECT_TRUE(DoCreateOffer(&offer)); + EXPECT_TRUE(DoCreateOffer(&offer, nullptr)); EXPECT_TRUE(offer != NULL); delete offer; offer = NULL; @@ -1113,11 +1118,11 @@ TEST_F(PeerConnectionInterfaceTest, CreateOfferAnswerWithInvalidStream) { AddAudioVideoStream(kStreamLabel1, "track_label", "track_label"); // Test CreateOffer - EXPECT_FALSE(DoCreateOffer(&offer)); + EXPECT_FALSE(DoCreateOffer(&offer, nullptr)); // Test CreateAnswer SessionDescriptionInterface* answer = NULL; - EXPECT_FALSE(DoCreateAnswer(&answer)); + EXPECT_FALSE(DoCreateAnswer(&answer, nullptr)); } // Test that we will get different SSRCs for each tracks in the offer and answer @@ -1129,7 +1134,7 @@ TEST_F(PeerConnectionInterfaceTest, SsrcInOfferAnswer) { // Test CreateOffer scoped_ptr offer; - ASSERT_TRUE(DoCreateOffer(offer.use())); + ASSERT_TRUE(DoCreateOffer(offer.use(), nullptr)); int audio_ssrc = 0; int video_ssrc = 0; EXPECT_TRUE(GetFirstSsrc(GetFirstAudioContent(offer->description()), @@ -1141,7 +1146,7 @@ TEST_F(PeerConnectionInterfaceTest, SsrcInOfferAnswer) { // Test CreateAnswer EXPECT_TRUE(DoSetRemoteDescription(offer.release())); scoped_ptr answer; - ASSERT_TRUE(DoCreateAnswer(answer.use())); + ASSERT_TRUE(DoCreateAnswer(answer.use(), nullptr)); audio_ssrc = 0; video_ssrc = 0; EXPECT_TRUE(GetFirstSsrc(GetFirstAudioContent(answer->description()), @@ -1588,6 +1593,73 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveUpdatedAudioOfferWithBadCodecs) { CreateAnswerAsLocalDescription(); } +// Test that if we're receiving (but not sending) a track, subsequent offers +// will have m-lines with a=recvonly. +TEST_F(PeerConnectionInterfaceTest, CreateSubsequentRecvOnlyOffer) { + FakeConstraints constraints; + constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, + true); + CreatePeerConnection(&constraints); + CreateAndSetRemoteOffer(kSdpStringWithStream1); + CreateAnswerAsLocalDescription(); + + // At this point we should be receiving stream 1, but not sending anything. + // A new offer should be recvonly. + SessionDescriptionInterface* offer; + DoCreateOffer(&offer, nullptr); + + const cricket::ContentInfo* video_content = + cricket::GetFirstVideoContent(offer->description()); + const cricket::VideoContentDescription* video_desc = + static_cast( + video_content->description); + ASSERT_EQ(cricket::MD_RECVONLY, video_desc->direction()); + + const cricket::ContentInfo* audio_content = + cricket::GetFirstAudioContent(offer->description()); + const cricket::AudioContentDescription* audio_desc = + static_cast( + audio_content->description); + ASSERT_EQ(cricket::MD_RECVONLY, audio_desc->direction()); +} + +// Test that if we're receiving (but not sending) a track, and the +// offerToReceiveVideo/offerToReceiveAudio constraints are explicitly set to +// false, the generated m-lines will be a=inactive. +TEST_F(PeerConnectionInterfaceTest, CreateSubsequentInactiveOffer) { + FakeConstraints constraints; + constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, + true); + CreatePeerConnection(&constraints); + CreateAndSetRemoteOffer(kSdpStringWithStream1); + CreateAnswerAsLocalDescription(); + + // At this point we should be receiving stream 1, but not sending anything. + // A new offer would be recvonly, but we'll set the "no receive" constraints + // to make it inactive. + SessionDescriptionInterface* offer; + FakeConstraints offer_constraints; + offer_constraints.AddMandatory( + webrtc::MediaConstraintsInterface::kOfferToReceiveVideo, false); + offer_constraints.AddMandatory( + webrtc::MediaConstraintsInterface::kOfferToReceiveAudio, false); + DoCreateOffer(&offer, &offer_constraints); + + const cricket::ContentInfo* video_content = + cricket::GetFirstVideoContent(offer->description()); + const cricket::VideoContentDescription* video_desc = + static_cast( + video_content->description); + ASSERT_EQ(cricket::MD_INACTIVE, video_desc->direction()); + + const cricket::ContentInfo* audio_content = + cricket::GetFirstAudioContent(offer->description()); + const cricket::AudioContentDescription* audio_desc = + static_cast( + audio_content->description); + ASSERT_EQ(cricket::MD_INACTIVE, audio_desc->direction()); +} + // Test that PeerConnection::Close changes the states to closed and all remote // tracks change state to ended. TEST_F(PeerConnectionInterfaceTest, CloseAndTestStreamsAndStates) { @@ -1644,9 +1716,9 @@ TEST_F(PeerConnectionInterfaceTest, CloseAndTestMethods) { EXPECT_TRUE(pc_->remote_description() != NULL); rtc::scoped_ptr offer; - EXPECT_TRUE(DoCreateOffer(offer.use())); + EXPECT_TRUE(DoCreateOffer(offer.use(), nullptr)); rtc::scoped_ptr answer; - EXPECT_TRUE(DoCreateAnswer(answer.use())); + EXPECT_TRUE(DoCreateAnswer(answer.use(), nullptr)); std::string sdp; ASSERT_TRUE(pc_->remote_description()->ToString(&sdp)); @@ -1750,7 +1822,7 @@ TEST_F(PeerConnectionInterfaceTest, RejectMediaContent) { EXPECT_EQ(webrtc::MediaStreamTrackInterface::kLive, remote_audio->state()); rtc::scoped_ptr local_answer; - EXPECT_TRUE(DoCreateAnswer(local_answer.accept())); + EXPECT_TRUE(DoCreateAnswer(local_answer.accept(), nullptr)); cricket::ContentInfo* video_info = local_answer->description()->GetContentByName("video"); video_info->rejected = true; @@ -1760,7 +1832,7 @@ TEST_F(PeerConnectionInterfaceTest, RejectMediaContent) { // Now create an offer where we reject both video and audio. rtc::scoped_ptr local_offer; - EXPECT_TRUE(DoCreateOffer(local_offer.accept())); + EXPECT_TRUE(DoCreateOffer(local_offer.accept(), nullptr)); video_info = local_offer->description()->GetContentByName("video"); ASSERT_TRUE(video_info != nullptr); video_info->rejected = true; @@ -1933,7 +2005,7 @@ TEST_F(PeerConnectionInterfaceTest, LocalDescriptionChanged) { // Create an offer just to ensure we have an identity before we manually // call SetLocalDescription. rtc::scoped_ptr throwaway; - ASSERT_TRUE(DoCreateOffer(throwaway.accept())); + ASSERT_TRUE(DoCreateOffer(throwaway.accept(), nullptr)); rtc::scoped_ptr desc_1; CreateSessionDescriptionAndReference(2, 2, desc_1.accept()); @@ -1970,7 +2042,7 @@ TEST_F(PeerConnectionInterfaceTest, // Create an offer just to ensure we have an identity before we manually // call SetLocalDescription. rtc::scoped_ptr throwaway; - ASSERT_TRUE(DoCreateOffer(throwaway.accept())); + ASSERT_TRUE(DoCreateOffer(throwaway.accept(), nullptr)); rtc::scoped_ptr desc_1; CreateSessionDescriptionAndReference(2, 2, desc_1.accept()); @@ -1999,7 +2071,7 @@ TEST_F(PeerConnectionInterfaceTest, // Create an offer just to ensure we have an identity before we manually // call SetLocalDescription. rtc::scoped_ptr throwaway; - ASSERT_TRUE(DoCreateOffer(throwaway.accept())); + ASSERT_TRUE(DoCreateOffer(throwaway.accept(), nullptr)); rtc::scoped_ptr desc; CreateSessionDescriptionAndReference(1, 1, desc.accept()); @@ -2045,7 +2117,7 @@ TEST_F(PeerConnectionInterfaceTest, SignalSameTracksInSeparateMediaStream) { // Create an offer just to ensure we have an identity before we manually // call SetLocalDescription. rtc::scoped_ptr throwaway; - ASSERT_TRUE(DoCreateOffer(throwaway.accept())); + ASSERT_TRUE(DoCreateOffer(throwaway.accept(), nullptr)); rtc::scoped_ptr desc; CreateSessionDescriptionAndReference(1, 1, desc.accept()); @@ -2082,6 +2154,9 @@ TEST_F(PeerConnectionInterfaceTest, SignalSameTracksInSeparateMediaStream) { } // The following tests verify that session options are created correctly. +// TODO(deadbeef): Convert these tests to be more end-to-end. Instead of +// "verify options are converted correctly", should be "pass options into +// CreateOffer and verify the correct offer is produced." TEST(CreateSessionOptionsTest, GetOptionsForOfferWithInvalidAudioOption) { RTCOfferAnswerOptions rtc_options; @@ -2108,8 +2183,7 @@ TEST(CreateSessionOptionsTest, GetOptionsForOfferWithInvalidVideoOption) { } // Test that a MediaSessionOptions is created for an offer if -// OfferToReceiveAudio and OfferToReceiveVideo options are set but no -// MediaStreams are sent. +// OfferToReceiveAudio and OfferToReceiveVideo options are set. TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudioVideo) { RTCOfferAnswerOptions rtc_options; rtc_options.offer_to_receive_audio = 1; @@ -2123,7 +2197,7 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudioVideo) { } // Test that a correct MediaSessionOptions is created for an offer if -// OfferToReceiveAudio is set but no MediaStreams are sent. +// OfferToReceiveAudio is set. TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudio) { RTCOfferAnswerOptions rtc_options; rtc_options.offer_to_receive_audio = 1; @@ -2136,21 +2210,21 @@ TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithAudio) { } // Test that a correct MediaSessionOptions is created for an offer if -// the default OfferOptons is used or MediaStreams are sent. +// the default OfferOptions are used. TEST(CreateSessionOptionsTest, GetDefaultMediaSessionOptionsForOffer) { RTCOfferAnswerOptions rtc_options; cricket::MediaSessionOptions options; EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options)); - EXPECT_FALSE(options.has_audio()); + EXPECT_TRUE(options.has_audio()); EXPECT_FALSE(options.has_video()); - EXPECT_FALSE(options.bundle_enabled); + EXPECT_TRUE(options.bundle_enabled); EXPECT_TRUE(options.vad_enabled); EXPECT_FALSE(options.transport_options.ice_restart); } // Test that a correct MediaSessionOptions is created for an offer if -// OfferToReceiveVideo is set but no MediaStreams are sent. +// OfferToReceiveVideo is set. TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithVideo) { RTCOfferAnswerOptions rtc_options; rtc_options.offer_to_receive_audio = 0; @@ -2209,19 +2283,19 @@ TEST(CreateSessionOptionsTest, MediaConstraintsInAnswer) { EXPECT_TRUE(answer_options.has_audio()); EXPECT_TRUE(answer_options.has_video()); - RTCOfferAnswerOptions rtc_offer_optoins; + RTCOfferAnswerOptions rtc_offer_options; cricket::MediaSessionOptions offer_options; - EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_offer_optoins, &offer_options)); - EXPECT_FALSE(offer_options.has_audio()); + EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_offer_options, &offer_options)); + EXPECT_TRUE(offer_options.has_audio()); EXPECT_FALSE(offer_options.has_video()); - RTCOfferAnswerOptions updated_rtc_offer_optoins; - updated_rtc_offer_optoins.offer_to_receive_audio = 1; - updated_rtc_offer_optoins.offer_to_receive_video = 1; + RTCOfferAnswerOptions updated_rtc_offer_options; + updated_rtc_offer_options.offer_to_receive_audio = 1; + updated_rtc_offer_options.offer_to_receive_video = 1; cricket::MediaSessionOptions updated_offer_options; - EXPECT_TRUE(ConvertRtcOptionsForOffer(updated_rtc_offer_optoins, + EXPECT_TRUE(ConvertRtcOptionsForOffer(updated_rtc_offer_options, &updated_offer_options)); EXPECT_TRUE(updated_offer_options.has_audio()); EXPECT_TRUE(updated_offer_options.has_video()); @@ -2240,12 +2314,4 @@ TEST(CreateSessionOptionsTest, MediaConstraintsInAnswer) { ParseConstraintsForAnswer(&updated_answer_c, &updated_answer_options)); EXPECT_TRUE(updated_answer_options.has_audio()); EXPECT_TRUE(updated_answer_options.has_video()); - - RTCOfferAnswerOptions default_rtc_options; - EXPECT_TRUE( - ConvertRtcOptionsForOffer(default_rtc_options, &updated_offer_options)); - // By default, |has_audio| or |has_video| are false if there is no media - // track. - EXPECT_FALSE(updated_offer_options.has_audio()); - EXPECT_FALSE(updated_offer_options.has_video()); } diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index 6a0f62271e..3fa9a7d469 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -268,7 +268,6 @@ static bool IsDtlsSctp(const std::string& protocol); static bool ParseSessionDescription(const std::string& message, size_t* pos, std::string* session_id, std::string* session_version, - bool* supports_msid, TransportDescription* session_td, RtpHeaderExtensions* session_extmaps, cricket::SessionDescription* desc, @@ -280,7 +279,6 @@ static bool ParseMediaDescription( const std::string& message, const TransportDescription& session_td, const RtpHeaderExtensions& session_extmaps, - bool supports_msid, size_t* pos, cricket::SessionDescription* desc, std::vector* candidates, SdpParseError* error); @@ -898,20 +896,18 @@ bool SdpDeserialize(const std::string& message, cricket::SessionDescription* desc = new cricket::SessionDescription(); std::vector candidates; size_t current_pos = 0; - bool supports_msid = false; // Session Description if (!ParseSessionDescription(message, ¤t_pos, &session_id, - &session_version, &supports_msid, &session_td, - &session_extmaps, desc, error)) { + &session_version, &session_td, &session_extmaps, + desc, error)) { delete desc; return false; } // Media Description - if (!ParseMediaDescription(message, session_td, session_extmaps, - supports_msid, ¤t_pos, desc, &candidates, - error)) { + if (!ParseMediaDescription(message, session_td, session_extmaps, ¤t_pos, + desc, &candidates, error)) { delete desc; for (std::vector::const_iterator it = candidates.begin(); it != candidates.end(); ++it) { @@ -1379,13 +1375,7 @@ void BuildRtpContentAttributes( // RFC 3264 // a=sendrecv || a=sendonly || a=sendrecv || a=inactive - - cricket::MediaContentDirection direction = media_desc->direction(); - if (media_desc->streams().empty() && direction == cricket::MD_SENDRECV) { - direction = cricket::MD_RECVONLY; - } - - switch (direction) { + switch (media_desc->direction()) { case cricket::MD_INACTIVE: InitAttrLine(kAttributeInactive, &os); break; @@ -1798,13 +1788,14 @@ bool IsDtlsSctp(const std::string& protocol) { bool ParseSessionDescription(const std::string& message, size_t* pos, std::string* session_id, std::string* session_version, - bool* supports_msid, TransportDescription* session_td, RtpHeaderExtensions* session_extmaps, cricket::SessionDescription* desc, SdpParseError* error) { std::string line; + desc->set_msid_supported(false); + // RFC 4566 // v= (protocol version) if (!GetLineWithType(message, pos, &line, kLineTypeVersion)) { @@ -1936,7 +1927,8 @@ bool ParseSessionDescription(const std::string& message, size_t* pos, if (!GetValue(line, kAttributeMsidSemantics, &semantics, error)) { return false; } - *supports_msid = CaseInsensitiveFind(semantics, kMediaStreamSemantic); + desc->set_msid_supported( + CaseInsensitiveFind(semantics, kMediaStreamSemantic)); } else if (HasAttribute(line, kAttributeExtmap)) { RtpHeaderExtension extmap; if (!ParseExtmap(line, &extmap, error)) { @@ -2146,7 +2138,6 @@ static C* ParseContentDescription(const std::string& message, bool ParseMediaDescription(const std::string& message, const TransportDescription& session_td, const RtpHeaderExtensions& session_extmaps, - bool supports_msid, size_t* pos, cricket::SessionDescription* desc, std::vector* candidates, @@ -2241,14 +2232,6 @@ bool ParseMediaDescription(const std::string& message, } if (IsRtp(protocol)) { - // Make sure to set the media direction correctly. If the direction is not - // MD_RECVONLY or Inactive and no streams are parsed, - // a default MediaStream will be created to prepare for receiving media. - if (supports_msid && content->streams().empty() && - content->direction() == cricket::MD_SENDRECV) { - content->set_direction(cricket::MD_RECVONLY); - } - // Set the extmap. if (!session_extmaps.empty() && !content->rtp_header_extensions().empty()) { diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index de2ba2f479..cb6a392ab4 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -883,6 +883,9 @@ class WebRtcSdpTest : public testing::Test { EXPECT_TRUE(CompareCandidates(transport1.description.candidates, transport2.description.candidates)); } + + // global attributes + EXPECT_EQ(desc1.msid_supported(), desc2.msid_supported()); } bool CompareCandidates(const Candidates& cs1, const Candidates& cs2) { @@ -1967,6 +1970,7 @@ TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithRejectedAudioVideo) { // Tests that we can still handle the sdp uses mslabel and label instead of // msid for backward compatibility. TEST_F(WebRtcSdpTest, DeserializeSessionDescriptionWithoutMsid) { + jdesc_.description()->set_msid_supported(false); JsepSessionDescription jdesc(kDummyString); std::string sdp_without_msid = kSdpFullString; Replace("msid", "xmsid", &sdp_without_msid); diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 9618db9d80..3eb46f1d3c 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -571,9 +571,24 @@ class WebRtcSessionTest void GetOptionsForOffer( const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options, cricket::MediaSessionOptions* session_options) { - AddStreamsToOptions(session_options); ASSERT_TRUE(ConvertRtcOptionsForOffer(rtc_options, session_options)); + AddStreamsToOptions(session_options); + if (rtc_options.offer_to_receive_audio == + RTCOfferAnswerOptions::kUndefined) { + session_options->recv_audio = + session_options->HasSendMediaStream(cricket::MEDIA_TYPE_AUDIO); + } + if (rtc_options.offer_to_receive_video == + RTCOfferAnswerOptions::kUndefined) { + session_options->recv_video = + session_options->HasSendMediaStream(cricket::MEDIA_TYPE_VIDEO); + } + session_options->bundle_enabled = + session_options->bundle_enabled && + (session_options->has_audio() || session_options->has_video() || + session_options->has_data()); + if (session_->data_channel_type() == cricket::DCT_SCTP && data_channel_) { session_options->data_channel_type = cricket::DCT_SCTP; } @@ -581,11 +596,16 @@ class WebRtcSessionTest void GetOptionsForAnswer(const webrtc::MediaConstraintsInterface* constraints, cricket::MediaSessionOptions* session_options) { - AddStreamsToOptions(session_options); session_options->recv_audio = false; session_options->recv_video = false; ASSERT_TRUE(ParseConstraintsForAnswer(constraints, session_options)); + AddStreamsToOptions(session_options); + session_options->bundle_enabled = + session_options->bundle_enabled && + (session_options->has_audio() || session_options->has_video() || + session_options->has_data()); + if (session_->data_channel_type() == cricket::DCT_SCTP) { session_options->data_channel_type = cricket::DCT_SCTP; } diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index 2cd2d46d9c..7413026092 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -1036,12 +1036,15 @@ static bool CreateMediaContentAnswer( answer->set_direction(MD_RECVONLY); break; case MD_RECVONLY: - answer->set_direction(MD_SENDONLY); + answer->set_direction(answer->streams().empty() ? MD_INACTIVE + : MD_SENDONLY); break; case MD_SENDRECV: - answer->set_direction(MD_SENDRECV); + answer->set_direction(answer->streams().empty() ? MD_RECVONLY + : MD_SENDRECV); break; default: + RTC_DCHECK(false && "MediaContentDescription has unexpected direction."); break; } @@ -1529,8 +1532,18 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); SetMediaProtocol(secure_transport, audio.get()); - if (!options.recv_audio) { - audio->set_direction(MD_SENDONLY); + if (!audio->streams().empty()) { + if (options.recv_audio) { + audio->set_direction(MD_SENDRECV); + } else { + audio->set_direction(MD_SENDONLY); + } + } else { + if (options.recv_audio) { + audio->set_direction(MD_RECVONLY); + } else { + audio->set_direction(MD_INACTIVE); + } } desc->AddContent(CN_AUDIO, NS_JINGLE_RTP, audio.release()); @@ -1574,8 +1587,18 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED); SetMediaProtocol(secure_transport, video.get()); - if (!options.recv_video) { - video->set_direction(MD_SENDONLY); + if (!video->streams().empty()) { + if (options.recv_video) { + video->set_direction(MD_SENDRECV); + } else { + video->set_direction(MD_SENDONLY); + } + } else { + if (options.recv_video) { + video->set_direction(MD_RECVONLY); + } else { + video->set_direction(MD_INACTIVE); + } } desc->AddContent(CN_VIDEO, NS_JINGLE_RTP, video.release()); diff --git a/webrtc/p2p/base/sessiondescription.h b/webrtc/p2p/base/sessiondescription.h index 1182a67740..7880167569 100644 --- a/webrtc/p2p/base/sessiondescription.h +++ b/webrtc/p2p/base/sessiondescription.h @@ -160,10 +160,15 @@ class SessionDescription { // Remove the first group with the same semantics specified by |name|. void RemoveGroupByName(const std::string& name); + // Global attributes. + void set_msid_supported(bool supported) { msid_supported_ = supported; } + bool msid_supported() const { return msid_supported_; } + private: ContentInfos contents_; TransportInfos transport_infos_; ContentGroups content_groups_; + bool msid_supported_ = true; }; // Indicates whether a ContentDescription was an offer or an answer, as