Bug 1506884 - Use DeleteStreams in AudioConduit; r=padenot
☠☠ backed out by a0084ecbf671 ☠ ☠
authorDan Minor <dminor@mozilla.com>
Wed, 27 Mar 2019 16:54:52 +0000
changeset 466402 5837db3740b54ff6e34d90ee0da41fe909022e06
parent 466401 1584d9804aa189196f47e504f27c3a133fca2d47
child 466403 0182d6543001d10fc7504518df4a487c444d1e7a
push id35768
push useropoprus@mozilla.com
push dateThu, 28 Mar 2019 09:55:54 +0000
treeherdermozilla-central@c045dd97faf2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1506884
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1506884 - Use DeleteStreams in AudioConduit; r=padenot This makes the shutdown behaviour of AudioConduit consistent with VideoConduit which should make the conduit code easier to read and reason about. Differential Revision: https://phabricator.services.mozilla.com/D25055
media/webrtc/signaling/gtest/audioconduit_unittests.cpp
media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
media/webrtc/signaling/gtest/mediapipeline_unittest.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.h
--- a/media/webrtc/signaling/gtest/audioconduit_unittests.cpp
+++ b/media/webrtc/signaling/gtest/audioconduit_unittests.cpp
@@ -73,16 +73,18 @@ class AudioConduitTest : public ::testin
  public:
   AudioConduitTest() : mCall(new MockCall()) {
     mAudioConduit = new AudioConduitWithMockChannelProxy(
         WebRtcCallWrapper::Create(UniquePtr<MockCall>(mCall)),
         GetCurrentThreadEventTarget());
     mAudioConduit->Init();
   }
 
+  ~AudioConduitTest() { mAudioConduit->DeleteStreams(); }
+
   MockCall* mCall;
   RefPtr<AudioConduitWithMockChannelProxy> mAudioConduit;
 };
 
 TEST_F(AudioConduitTest, TestConfigureSendMediaCodec) {
   MediaConduitErrorCode ec;
 
   // defaults
--- a/media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
+++ b/media/webrtc/signaling/gtest/mediaconduit_unittests.cpp
@@ -269,17 +269,30 @@ void AudioSendAndReceive::GenerateAndRea
  *  when it has RTP/RTCP frame to transmit.
  *  For everty RTP/RTCP frame we receive, we pass it back
  *  to the conduit for eventual decoding and rendering.
  */
 class WebrtcMediaTransport : public mozilla::TransportInterface {
  public:
   WebrtcMediaTransport() : numPkts(0), mAudio(false), mVideo(false) {}
 
-  ~WebrtcMediaTransport() {}
+  ~WebrtcMediaTransport() {
+    if (mAudioSession) {
+      mAudioSession->DeleteStreams();
+    }
+    if (mOtherAudioSession) {
+      mOtherAudioSession->DeleteStreams();
+    }
+    if (mVideoSession) {
+      mVideoSession->DeleteStreams();
+    }
+    if (mOtherVideoSession) {
+      mOtherVideoSession->DeleteStreams();
+    }
+  }
 
   virtual nsresult SendRtpPacket(const uint8_t* data, size_t len) {
     ++numPkts;
 
     if (mAudio) {
       mOtherAudioSession->ReceivedRTPPacket(data, len, SSRC);
     } else {
       mOtherVideoSession->ReceivedRTPPacket(data, len, SSRC);
--- a/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp
+++ b/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp
@@ -262,18 +262,25 @@ class TestAgent {
     MOZ_MTLOG(ML_DEBUG, "Stopping");
 
     if (audio_pipeline_) audio_pipeline_->Stop();
   }
 
   void Shutdown_s() { transport_->Shutdown(); }
 
   void Shutdown() {
-    if (audio_pipeline_) audio_pipeline_->Shutdown_m();
-    if (audio_stream_track_) audio_stream_track_->Stop();
+    if (audio_pipeline_) {
+      audio_pipeline_->Shutdown_m();
+    }
+    if (audio_stream_track_) {
+      audio_stream_track_->Stop();
+    }
+    if (audio_conduit_) {
+      audio_conduit_->DeleteStreams();
+    }
 
     mozilla::SyncRunnable::DispatchToThread(
         test_utils->sts_target(), WrapRunnable(this, &TestAgent::Shutdown_s));
   }
 
   uint32_t GetRemoteSSRC() {
     uint32_t res = 0;
     audio_conduit_->GetRemoteSSRC(&res);
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -70,25 +70,25 @@ RefPtr<AudioSessionConduit> AudioSession
 /**
  * Destruction defines for our super-classes
  */
 WebrtcAudioConduit::~WebrtcAudioConduit() {
   CSFLogDebug(LOGTAG, "%s ", __FUNCTION__);
   MOZ_ASSERT(NS_IsMainThread());
 
   MutexAutoLock lock(mMutex);
-  DeleteSendStream();
-  DeleteRecvStream();
-
   DeleteChannels();
 
   // We don't Terminate() the VoEBase here, because the Call (owned by
   // PeerConnectionMedia) actually owns the (shared) VoEBase/VoiceEngine
   // here
   mPtrVoEBase = nullptr;
+
+  MOZ_ASSERT(!mSendStream && !mRecvStream,
+             "Call DeleteStreams prior to ~WebrtcAudioConduit.");
 }
 
 bool WebrtcAudioConduit::SetLocalSSRCs(
     const std::vector<unsigned int>& aSSRCs) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aSSRCs.size() == 1,
              "WebrtcAudioConduit::SetLocalSSRCs accepts exactly 1 ssrc.");
 
@@ -1029,16 +1029,24 @@ MediaConduitErrorCode WebrtcAudioConduit
   if (status != webrtc::PacketReceiver::DELIVERY_OK) {
     CSFLogError(LOGTAG, "%s DeliverPacket Failed, %d", __FUNCTION__, status);
     return kMediaConduitRTPProcessingFailed;
   }
 
   return kMediaConduitNoError;
 }
 
+void WebrtcAudioConduit::DeleteStreams() {
+  MOZ_ASSERT(NS_IsMainThread());
+
+  MutexAutoLock lock(mMutex);
+  DeleteSendStream();
+  DeleteRecvStream();
+}
+
 MediaConduitErrorCode WebrtcAudioConduit::CreateChannels() {
   MOZ_ASSERT(NS_IsMainThread());
 
   if ((mRecvChannel = mPtrVoEBase->CreateChannel()) == -1) {
     CSFLogError(LOGTAG, "%s VoiceEngine Channel creation failed", __FUNCTION__);
     return kMediaConduitChannelError;
   }
   mRecvStreamConfig.voe_channel_id = mRecvChannel;
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -165,17 +165,17 @@ class WebrtcAudioConduit : public AudioS
    * AudioConduit registers itself as ExternalTransport to the VoiceEngine
    */
   bool SendRtcp(const uint8_t* data, size_t len) override;
 
   uint64_t CodecPluginID() override { return 0; }
   void SetPCHandle(const std::string& aPCHandle) override {}
   MediaConduitErrorCode DeliverPacket(const void* data, int len) override;
 
-  void DeleteStreams() override {}
+  void DeleteStreams() override;
 
   WebrtcAudioConduit(RefPtr<WebRtcCallWrapper> aCall,
                      nsCOMPtr<nsIEventTarget> aStsThread)
       : mFakeAudioDevice(new webrtc::FakeAudioDeviceModule()),
         mTransportMonitor("WebrtcAudioConduit"),
         mTransmitterTransport(nullptr),
         mReceiverTransport(nullptr),
         mCall(aCall),