Bug 1507216: Make sure AudioConduit doesn't re-create the recv stream without de-registering the old one. r=dminor
authorByron Campen [:bwc] <docfaraday@gmail.com>
Mon, 26 Nov 2018 19:56:25 +0000
changeset 504755 26598a115edd552ce42fe28561d7b71158f06298
parent 504754 4b8c2df35461931ee0eaa2f9e7fad1c2d41acb6c
child 504756 d05c6310d573658af4a4f2cee332bb01732fcabf
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdminor
bugs1507216
milestone65.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 1507216: Make sure AudioConduit doesn't re-create the recv stream without de-registering the old one. r=dminor Make a cleaner distinction between having a recv stream that is inactive, and no recv stream at all. Requires some modification to local RTC extension configuration, because of a test-case that assumed StopReceiving/StartReceiving would re-create the recv stream as opposed to simply restarting it. Differential Revision: https://phabricator.services.mozilla.com/D12936
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.h
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -105,63 +105,33 @@ bool WebrtcAudioConduit::SetLocalSSRCs(c
     return true;
   }
   // Update the value of the ssrcs in the config structure.
   mRecvStreamConfig.rtp.local_ssrc = aSSRCs[0];
   mSendStreamConfig.rtp.ssrc = aSSRCs[0];
 
   mRecvChannelProxy->SetLocalSSRC(aSSRCs[0]);
 
-  bool wasTransmitting = mEngineTransmitting;
-  if (StopTransmitting() != kMediaConduitNoError) {
-    return false;
-  }
-
-  if (wasTransmitting) {
-    if (StartTransmitting() != kMediaConduitNoError) {
-      return false;
-    }
-  }
-  return true;
+  return RecreateSendStreamIfExists();
 }
 
 std::vector<unsigned int> WebrtcAudioConduit::GetLocalSSRCs() {
   MutexAutoLock lock(mMutex);
   return std::vector<unsigned int>(1, mRecvStreamConfig.rtp.local_ssrc);
 }
 
 bool WebrtcAudioConduit::SetRemoteSSRC(unsigned int ssrc) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mRecvStreamConfig.rtp.remote_ssrc == ssrc) {
     return true;
   }
   mRecvStreamConfig.rtp.remote_ssrc = ssrc;
 
-  bool wasReceiving = mEngineReceiving;
-  if (StopReceiving() != kMediaConduitNoError) {
-    return false;
-  }
-
-  {
-    MutexAutoLock lock(mMutex);
-    // On the next StartReceiving() or ConfigureRecvMediaCodec, force
-    // building a new RecvStream to switch SSRCs.
-    DeleteRecvStream();
-    if (!wasReceiving) {
-      return true;
-    }
-    MediaConduitErrorCode rval = CreateRecvStream();
-    if (rval != kMediaConduitNoError) {
-      CSFLogError(LOGTAG, "%s Start Receive Error %d ", __FUNCTION__, rval);
-      return false;
-    }
-  }
-  return (StartReceiving() == kMediaConduitNoError);
-
+  return RecreateRecvStreamIfExists();
 }
 
 bool WebrtcAudioConduit::GetRemoteSSRC(unsigned int* ssrc) {
   {
     MutexAutoLock lock(mMutex);
     if (!mRecvStream) {
       return false;
     }
@@ -192,17 +162,17 @@ void WebrtcAudioConduit::SetSyncGroup(co
   MOZ_ASSERT(NS_IsMainThread());
   mRecvStreamConfig.sync_group = group;
 }
 
 bool WebrtcAudioConduit::GetSendPacketTypeStats(
   webrtc::RtcpPacketTypeCounter* aPacketCounts)
 {
   ASSERT_ON_THREAD(mStsThread);
-  if (!mEngineTransmitting) {
+  if (!mSendStream) {
     return false;
   }
   return mSendChannelProxy->GetRTCPPacketTypeCounters(*aPacketCounts);
 }
 
 bool WebrtcAudioConduit::GetRecvPacketTypeStats(
   webrtc::RtcpPacketTypeCounter* aPacketCounts)
 {
@@ -529,58 +499,86 @@ WebrtcAudioConduit::ConfigureRecvMediaCo
 
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcAudioConduit::SetLocalRTPExtensions(LocalDirection aDirection,
                                           const RtpExtList& extensions)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   CSFLogDebug(LOGTAG, "%s direction: %s", __FUNCTION__,
               MediaSessionConduit::LocalDirectionToString(aDirection).c_str());
-  MOZ_ASSERT(NS_IsMainThread());
 
   bool isSend = aDirection == LocalDirection::kSend;
-  if (isSend) {
-    mSendStreamConfig.rtp.extensions.clear();
-  } else {
-    mRecvStreamConfig.rtp.extensions.clear();
-  }
+  RtpExtList filteredExtensions;
+
+  int ssrcAudioLevelId = -1;
+  int csrcAudioLevelId = -1;
+  int midId = -1;
+
   for(const auto& extension : extensions) {
     // ssrc-audio-level RTP header extension
     if (extension.uri == webrtc::RtpExtension::kAudioLevelUri) {
-      if (isSend) {
-        mSendStreamConfig.rtp.extensions.push_back(
-          webrtc::RtpExtension(extension.uri, extension.id));
-        mSendChannelProxy->SetSendAudioLevelIndicationStatus(true, extension.id);
-      } else {
-        mRecvStreamConfig.rtp.extensions.push_back(
-          webrtc::RtpExtension(extension.uri, extension.id));
-        mRecvChannelProxy->SetReceiveAudioLevelIndicationStatus(true, extension.id);
-      }
+      ssrcAudioLevelId = extension.id;
+      filteredExtensions.push_back(webrtc::RtpExtension(
+            extension.uri, extension.id));
     }
+
     // csrc-audio-level RTP header extension
     if (extension.uri == webrtc::RtpExtension::kCsrcAudioLevelUri) {
       if (isSend) {
         CSFLogError(LOGTAG, "%s SetSendAudioLevelIndicationStatus Failed"
                     " can not send CSRC audio levels.", __FUNCTION__);
         return kMediaConduitMalformedArgument;
       }
-      mRecvStreamConfig.rtp.extensions.push_back(
-        webrtc::RtpExtension(extension.uri, extension.id));
-      mRecvChannelProxy->SetReceiveCsrcAudioLevelIndicationStatus(true, extension.id);
+      csrcAudioLevelId = extension.id;
+      filteredExtensions.push_back(webrtc::RtpExtension(
+            extension.uri, extension.id));
     }
+
     // MID RTP header extension
-    if (aDirection == LocalDirection::kSend &&
-        extension.uri == webrtc::RtpExtension::kMIdUri) {
-        mSendStreamConfig.rtp.extensions.push_back(
-          webrtc::RtpExtension(extension.uri, extension.id));
-        mSendChannelProxy->SetSendMIDStatus(true, extension.id);
+    if (extension.uri == webrtc::RtpExtension::kMIdUri) {
+      if (!isSend) {
+        // TODO(bug 1405495): Why do we error out for csrc-audio-level, but not
+        // mid?
+        continue;
+      }
+      midId = extension.id;
+      filteredExtensions.push_back(webrtc::RtpExtension(
+            extension.uri, extension.id));
     }
   }
+
+  auto& currentExtensions = isSend ?
+    mSendStreamConfig.rtp.extensions : mRecvStreamConfig.rtp.extensions;
+  if (filteredExtensions == currentExtensions) {
+    return kMediaConduitNoError;
+  }
+
+  currentExtensions = filteredExtensions;
+
+  if (isSend) {
+    mSendChannelProxy->SetSendAudioLevelIndicationStatus(
+        ssrcAudioLevelId != -1, ssrcAudioLevelId);
+    mSendChannelProxy->SetSendMIDStatus(midId != -1, midId);
+  } else {
+    mRecvChannelProxy->SetReceiveAudioLevelIndicationStatus(
+        ssrcAudioLevelId != -1, ssrcAudioLevelId);
+    mRecvChannelProxy->SetReceiveCsrcAudioLevelIndicationStatus(
+        csrcAudioLevelId != -1, csrcAudioLevelId);
+    // TODO(bug 1405495): recv mid support
+  }
+
+  if (isSend) {
+    RecreateSendStreamIfExists();
+  } else {
+    RecreateRecvStreamIfExists();
+  }
+
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcAudioConduit::SendAudioFrame(const int16_t audio_data[],
                                    int32_t lengthSamples, // per channel
                                    int32_t samplingFreqHz,
                                    uint32_t channels,
@@ -833,47 +831,54 @@ WebrtcAudioConduit::StartReceiving()
 MediaConduitErrorCode
 WebrtcAudioConduit::StopTransmittingLocked()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMutex.AssertCurrentThreadOwns();
 
   if(mEngineTransmitting)
   {
+    MOZ_ASSERT(mSendStream);
     CSFLogDebug(LOGTAG, "%s Engine Already Sending. Attemping to Stop ", __FUNCTION__);
-    DeleteSendStream();
+    mSendStream->Stop();
     mEngineTransmitting = false;
   }
 
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcAudioConduit::StartTransmittingLocked()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMutex.AssertCurrentThreadOwns();
 
-  if (!mEngineTransmitting) {
+  if (mEngineTransmitting) {
+    return kMediaConduitNoError;
+  }
+
+  if (!mSendStream) {
     CreateSendStream();
-    mCall->Call()->SignalChannelNetworkState(webrtc::MediaType::AUDIO, webrtc::kNetworkUp);
-    mSendStream->Start();
-    mEngineTransmitting = true;
   }
 
+  mCall->Call()->SignalChannelNetworkState(webrtc::MediaType::AUDIO, webrtc::kNetworkUp);
+  mSendStream->Start();
+  mEngineTransmitting = true;
+
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcAudioConduit::StopReceivingLocked()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMutex.AssertCurrentThreadOwns();
 
-  if(mEngineReceiving && mRecvStream) {
+  if(mEngineReceiving) {
+    MOZ_ASSERT(mRecvStream);
     mRecvStream->Stop();
     mEngineReceiving = false;
   }
 
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
@@ -881,17 +886,20 @@ WebrtcAudioConduit::StartReceivingLocked
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMutex.AssertCurrentThreadOwns();
 
   if (mEngineReceiving) {
     return kMediaConduitNoError;
   }
 
-  CreateRecvStream();
+  if (!mRecvStream) {
+    CreateRecvStream();
+  }
+
   mCall->Call()->SignalChannelNetworkState(webrtc::MediaType::AUDIO, webrtc::kNetworkUp);
   mRecvStream->Start();
   mEngineReceiving = true;
 
   return kMediaConduitNoError;
 }
 
 //WebRTC::RTP Callback Implementation
@@ -1058,16 +1066,17 @@ WebrtcAudioConduit::CreateSendStream()
 }
 
 void
 WebrtcAudioConduit::DeleteRecvStream()
 {
   mMutex.AssertCurrentThreadOwns();
   if (mRecvStream) {
     mRecvStream->Stop();
+    mEngineReceiving = false;
     mCall->Call()->DestroyAudioReceiveStream(mRecvStream);
     mRecvStream = nullptr;
   }
   // Destroying the stream unregisters the transport
   mRecvChannelProxy->RegisterTransport(nullptr);
 }
 
 MediaConduitErrorCode
@@ -1079,16 +1088,56 @@ WebrtcAudioConduit::CreateRecvStream()
   mRecvStream = mCall->Call()->CreateAudioReceiveStream(mRecvStreamConfig);
   if (!mRecvStream) {
     return kMediaConduitUnknownError;
   }
 
   return kMediaConduitNoError;
 }
 
+bool
+WebrtcAudioConduit::RecreateSendStreamIfExists()
+{
+  MutexAutoLock lock(mMutex);
+  bool wasTransmitting = mEngineTransmitting;
+  bool hadSendStream = mSendStream;
+  DeleteSendStream();
+
+  if (wasTransmitting) {
+    if (StartTransmittingLocked() != kMediaConduitNoError) {
+      return false;
+    }
+  } else if (hadSendStream) {
+    if (CreateSendStream() != kMediaConduitNoError) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool
+WebrtcAudioConduit::RecreateRecvStreamIfExists()
+{
+  MutexAutoLock lock(mMutex);
+  bool wasReceiving = mEngineReceiving;
+  bool hadRecvStream = mRecvStream;
+  DeleteRecvStream();
+
+  if (wasReceiving) {
+    if (StartReceivingLocked() != kMediaConduitNoError) {
+      return false;
+    }
+  } else if (hadRecvStream) {
+    if (CreateRecvStream() != kMediaConduitNoError) {
+      return false;
+    }
+  }
+  return true;
+}
+
 MediaConduitErrorCode
 WebrtcAudioConduit::DeliverPacket(const void *data, int len)
 {
   // Bug 1499796 - we need to get passed the time the packet was received
   webrtc::PacketReceiver::DeliveryStatus status =
     mCall->Call()->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
                                              static_cast<const uint8_t*>(data),
                                              len, webrtc::PacketTime());
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -292,16 +292,19 @@ private:
   //Checks the codec to be applied
   MediaConduitErrorCode ValidateCodecConfig(const AudioCodecConfig* codecInfo, bool send);
 
   MediaConduitErrorCode CreateSendStream();
   void DeleteSendStream();
   MediaConduitErrorCode CreateRecvStream();
   void DeleteRecvStream();
 
+  bool RecreateSendStreamIfExists();
+  bool RecreateRecvStreamIfExists();
+
   MediaConduitErrorCode CreateChannels();
   virtual void DeleteChannels();
 
   UniquePtr<webrtc::FakeAudioDeviceModule> mFakeAudioDevice;
   mozilla::ReentrantMonitor mTransportMonitor;
   RefPtr<TransportInterface> mTransmitterTransport;
   RefPtr<TransportInterface> mReceiverTransport;
   ScopedCustomReleasePtr<webrtc::VoEBase> mPtrVoEBase;