diff --git a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp index fd2a0a0f15dc..57e21b36e3b7 100644 --- a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp +++ b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp @@ -2282,21 +2282,6 @@ static int vcmTxStartICE_m(cc_mcapid_t mcap_id, } } - // This tells the receive MediaPipeline (if there is one) whether we are - // doing bundle, and if so, updates the filter. This does not affect the - // transmit MediaPipeline (created above) at all. - if (attrs->bundle_level) { - nsAutoPtr filter (new MediaPipelineFilter); - for (int s = 0; s < attrs->num_ssrcs; ++s) { - filter->AddRemoteSSRC(attrs->ssrcs[s]); - } - pc.impl()->media()->SetUsingBundle_m(level, true); - pc.impl()->media()->UpdateFilterFromRemoteDescription_m(level, filter); - } else { - // This will also clear the filter. - pc.impl()->media()->SetUsingBundle_m(level, false); - } - if (CC_IS_AUDIO(mcap_id)) { mozilla::AudioCodecConfig *config_raw; @@ -2405,6 +2390,21 @@ static int vcmTxStartICE_m(cc_mcapid_t mcap_id, return VCM_ERROR; } + // This tells the receive MediaPipeline (if there is one) whether we are + // doing bundle, and if so, updates the filter. Once the filter is finalized, + // it is then copied to the transmit pipeline so it can filter RTCP. + if (attrs->bundle_level) { + nsAutoPtr filter (new MediaPipelineFilter); + for (int s = 0; s < attrs->num_ssrcs; ++s) { + filter->AddRemoteSSRC(attrs->ssrcs[s]); + } + pc.impl()->media()->SetUsingBundle_m(level, true); + pc.impl()->media()->UpdateFilterFromRemoteDescription_m(level, filter); + } else { + // This will also clear the filter. + pc.impl()->media()->SetUsingBundle_m(level, false); + } + CSFLogDebug( logTag, "%s success", __FUNCTION__); return 0; } diff --git a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp index dd660106c6fd..a91b6f3ef736 100644 --- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp +++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp @@ -246,32 +246,28 @@ nsresult MediaPipeline::TransportReady_s(TransportInfo &info) { return NS_ERROR_FAILURE; } - if (direction_ == RECEIVE) { - // The TRANSMIT pipeline does not process _any_ RTCP. This is the RECEIVE - // pipeline's job, even for receiver reports. MOZ_MTLOG(ML_INFO, "Listening for " << ToString(info.type_) << " packets received on " << static_cast(dtls->downward())); - switch (info.type_) { - case RTP: - dtls->downward()->SignalPacketReceived.connect( - this, - &MediaPipeline::RtpPacketReceived); - break; - case RTCP: - dtls->downward()->SignalPacketReceived.connect( - this, - &MediaPipeline::RtcpPacketReceived); - break; - case MUX: - dtls->downward()->SignalPacketReceived.connect( - this, - &MediaPipeline::PacketReceived); - break; - default: - MOZ_CRASH(); - } + switch (info.type_) { + case RTP: + dtls->downward()->SignalPacketReceived.connect( + this, + &MediaPipeline::RtpPacketReceived); + break; + case RTCP: + dtls->downward()->SignalPacketReceived.connect( + this, + &MediaPipeline::RtcpPacketReceived); + break; + case MUX: + dtls->downward()->SignalPacketReceived.connect( + this, + &MediaPipeline::PacketReceived); + break; + default: + MOZ_CRASH(); } info.state_ = MP_OPEN; @@ -773,7 +769,7 @@ void MediaPipeline::SetUsingBundle_s(bool decision) { } } -void MediaPipeline::UpdateFilterFromRemoteDescription_s( +MediaPipelineFilter* MediaPipeline::UpdateFilterFromRemoteDescription_s( nsAutoPtr filter) { ASSERT_ON_THREAD(sts_thread_); // This is only supposed to relax the filter. Relaxing a missing filter is @@ -785,6 +781,8 @@ void MediaPipeline::UpdateFilterFromRemoteDescription_s( } else { filter_->IncorporateRemoteDescription(*filter); } + + return filter_.get(); } nsresult MediaPipeline::PipelineTransport::SendRtpPacket( diff --git a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.h b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.h index d48a5fc9806e..e909ab5fe161 100644 --- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.h +++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.h @@ -128,7 +128,7 @@ class MediaPipeline : public sigslot::has_slots<> { // packet is meant for us. We want to get out of this indeterminate state // ASAP, which is what this function can be used for. void SetUsingBundle_s(bool decision); - void UpdateFilterFromRemoteDescription_s( + MediaPipelineFilter* UpdateFilterFromRemoteDescription_s( nsAutoPtr filter); virtual Direction direction() const { return direction_; } diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp index b6748455314d..6cac31f18126 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp @@ -412,21 +412,59 @@ PeerConnectionMedia::SetUsingBundle_m(int level, bool decision) return false; } +static void +UpdateFilterFromRemoteDescription_s( + RefPtr receive, + RefPtr transmit, + nsAutoPtr filter) { + + // Update filter, and make a copy of the final version. + mozilla::MediaPipelineFilter *finalFilter( + receive->UpdateFilterFromRemoteDescription_s(filter)); + + if (finalFilter) { + filter = new mozilla::MediaPipelineFilter(*finalFilter); + } + + // Set same filter on transmit pipeline too. + transmit->UpdateFilterFromRemoteDescription_s(filter); +} + bool PeerConnectionMedia::UpdateFilterFromRemoteDescription_m( int level, nsAutoPtr filter) { ASSERT_ON_THREAD(mMainThread); - for (size_t i = 0; i < mRemoteSourceStreams.Length(); ++i) { - if (mRemoteSourceStreams[i]->UpdateFilterFromRemoteDescription_m(level, - filter)) { - // Found the MediaPipeline for |level| - return true; - } + + RefPtr receive; + for (size_t i = 0; !receive && i < mRemoteSourceStreams.Length(); ++i) { + receive = mRemoteSourceStreams[i]->GetPipelineByLevel_m(level); + } + + RefPtr transmit; + for (size_t i = 0; !transmit && i < mLocalSourceStreams.Length(); ++i) { + transmit = mLocalSourceStreams[i]->GetPipelineByLevel_m(level); + } + + if (receive && transmit) { + // GetPipelineByLevel_m will return nullptr if shutdown is in progress; + // since shutdown is initiated in main, and involves a dispatch to STS + // before the pipelines are released, our dispatch to STS will complete + // before any release can happen due to a shutdown that hasn't started yet. + RUN_ON_THREAD(GetSTSThread(), + WrapRunnableNM( + &UpdateFilterFromRemoteDescription_s, + receive, + transmit, + filter + ), + NS_DISPATCH_NORMAL); + return true; + } else { + CSFLogWarn(logTag, "Could not locate level %d to update filter", + static_cast(level)); } - CSFLogWarn(logTag, "Could not locate level %d to update filter", - static_cast(level)); return false; } @@ -537,55 +575,28 @@ RemoteSourceStreamInfo::StorePipeline(int aTrack, mTypes[aTrack] = aIsVideo; } -RefPtr RemoteSourceStreamInfo::GetPipelineByLevel_m(int level) { +RefPtr SourceStreamInfo::GetPipelineByLevel_m(int level) { ASSERT_ON_THREAD(mParent->GetMainThread()); - for (auto p = mPipelines.begin(); p != mPipelines.end(); ++p) { - if (p->second->level() == level) { - return p->second; + + // Refuse to hand out references if we're tearing down. + // (Since teardown involves a dispatch to and from STS before MediaPipelines + // are released, it is safe to start other dispatches to and from STS with a + // RefPtr, since that reference won't be the last one + // standing) + if (mMediaStream) { + for (auto p = mPipelines.begin(); p != mPipelines.end(); ++p) { + if (p->second->level() == level) { + return p->second; + } } } + return nullptr; } -bool RemoteSourceStreamInfo::UpdateFilterFromRemoteDescription_m( - int aLevel, - nsAutoPtr aFilter) { - ASSERT_ON_THREAD(mParent->GetMainThread()); - - if (!mMediaStream) { - // Guard against dispatching once we've started teardown, since we don't - // want the RefPtr being the last one standing on the call - // to MediaPipeline::UpdateFilterFromRemoteDescription_s; it is not safe - // to delete a MediaPipeline anywhere other than the main thread. - return false; - } - - RefPtr pipeline(GetPipelineByLevel_m(aLevel)); - - if (pipeline) { - RUN_ON_THREAD(mParent->GetSTSThread(), - WrapRunnable( - pipeline, - &MediaPipeline::UpdateFilterFromRemoteDescription_s, - aFilter - ), - NS_DISPATCH_NORMAL); - return true; - } - return false; -} - bool RemoteSourceStreamInfo::SetUsingBundle_m(int aLevel, bool decision) { ASSERT_ON_THREAD(mParent->GetMainThread()); - if (!mMediaStream) { - // Guard against dispatching once we've started teardown, since we don't - // want the RefPtr being the last one standing on the call - // to MediaPipeline::SetUsingBundle_s; it is not safe - // to delete a MediaPipeline anywhere other than the main thread. - return false; - } - RefPtr pipeline(GetPipelineByLevel_m(aLevel)); if (pipeline) { diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h index 4ec5cd614b56..4a1aa6f5b01b 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h @@ -190,6 +190,7 @@ public: // It allows visibility into the pipelines and flows. const std::map>& GetPipelines() const { return mPipelines; } + mozilla::RefPtr GetPipelineByLevel_m(int level); protected: std::map> mPipelines; @@ -244,9 +245,6 @@ class RemoteSourceStreamInfo : public SourceStreamInfo { void StorePipeline(int aTrack, bool aIsVideo, mozilla::RefPtr aPipeline); - bool UpdateFilterFromRemoteDescription_m( - int aLevel, - nsAutoPtr aFilter); bool SetUsingBundle_m(int aLevel, bool decision); void DetachTransport_s(); @@ -257,7 +255,6 @@ class RemoteSourceStreamInfo : public SourceStreamInfo { public: DOMMediaStream::TrackTypeHints mTrackTypeHints; private: - mozilla::RefPtr GetPipelineByLevel_m(int level); std::map mTypes; };