Bug 1376873 - Use audio/video sync groups; r=bwc

This uses sync groups in the receive stream configs for the conduits rather
than establishing sync through direct calls. When the streams are created
in call.cc, ConfigureSync is called, which results in SetSync being called
on the video stream which is the replacement for SetSyncChannel in branch 57
of webrtc.org.

With the current code, a video stream can only be synchronized to a single
audio stream. Using sync groups enforces a stronger constraint that only one
pair of audio and video streams can be synchronized for each sync group. The
comments in call.cc imply this is what is supported by webrtc.org, it seems
safer to also follow this constraint rather than circumvent it by calling
directly into the underlying code.

Differential Revision: https://phabricator.services.mozilla.com/D7445

--HG--
extra : rebase_source : fa8b1d301398edcedc6e488c51009b6667792f45

media/webrtc/signaling/gtest/MockCall.h

@@ -105,8 +105,6 @@ public:
     return false;
   }
 
-  void SetSyncChannel(VoiceEngine* voice_engine, int audio_channel_id) override {}
-
   void EnableEncodedFrameRecording(rtc::PlatformFile file,
                                    size_t byte_limit) override {}
 

media/webrtc/signaling/src/media-conduit/AudioConduit.cpp

@@ -187,6 +187,12 @@ bool WebrtcAudioConduit::SetLocalMID(const std::string& mid)
   return true;
 }
 
+void WebrtcAudioConduit::SetSyncGroup(const std::string& group)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  mRecvStreamConfig.sync_group = group;
+}
+
 bool WebrtcAudioConduit::GetSendPacketTypeStats(
   webrtc::RtcpPacketTypeCounter* aPacketCounts)
 {

media/webrtc/signaling/src/media-conduit/AudioConduit.h

@@ -215,6 +215,8 @@ public:
   bool SetLocalCNAME(const char* cname) override;
   bool SetLocalMID(const std::string& mid) override;
 
+  void SetSyncGroup(const std::string& group) override;
+
   bool GetSendPacketTypeStats(
       webrtc::RtcpPacketTypeCounter* aPacketCounts) override;
 

media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h

@@ -218,6 +218,8 @@ public:
 
   virtual bool SetLocalMID(const std::string& mid) = 0;
 
+  virtual void SetSyncGroup(const std::string& group) = 0;
+
   /**
    * Functions returning stats needed by w3c stats model.
    */

media/webrtc/signaling/src/media-conduit/VideoConduit.cpp

@@ -493,7 +493,6 @@ WebrtcVideoConduit::~WebrtcVideoConduit()
   mCall->UnregisterConduit(this);
 
   // Release AudioConduit first by dropping reference on MainThread, where it expects to be
-  SyncTo(nullptr);
   MOZ_ASSERT(!mSendStream && !mRecvStream, "Call DeleteStreams prior to ~WebrtcVideoConduit.");
 }
 
@@ -570,6 +569,11 @@ bool WebrtcVideoConduit::SetLocalMID(const std::string& mid)
   return true;
 }
 
+void WebrtcVideoConduit::SetSyncGroup(const std::string& group)
+{
+  mRecvStreamConfig.sync_group = group;
+}
+
 MediaConduitErrorCode
 WebrtcVideoConduit::ConfigureCodecMode(webrtc::VideoCodecMode mode)
 {
@@ -1420,31 +1424,6 @@ WebrtcVideoConduit::DeleteStreams()
   DeleteRecvStream();
 }
 
-void
-WebrtcVideoConduit::SyncTo(WebrtcAudioConduit* aConduit)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  CSFLogDebug(LOGTAG, "%s Synced to %p", __FUNCTION__, aConduit);
-
-  if (!mRecvStream) {
-    CSFLogError(LOGTAG, "SyncTo called with no receive stream");
-    return;
-  }
-
-  MutexAutoLock lock(mMutex);
-  //TODO: fix sync in a post voice engine world
-/*
-  if (aConduit) {
-    mRecvStream->SetSyncChannel(aConduit->GetVoiceEngine(),
-                                aConduit->GetChannel());
-  } else if (mSyncedTo) {
-    mRecvStream->SetSyncChannel(mSyncedTo->GetVoiceEngine(), -1);
-  }
-*/
-  mSyncedTo = aConduit;
-}
-
 MediaConduitErrorCode
 WebrtcVideoConduit::AttachRenderer(RefPtr<mozilla::VideoRenderer> aVideoRenderer)
 {

media/webrtc/signaling/src/media-conduit/VideoConduit.h

@@ -83,11 +83,6 @@ public:
   SetLocalRTPExtensions(MediaSessionConduitLocalDirection aDirection,
                         const RtpExtList& aExtensions) override;
 
-  /**
-   * Set up A/V sync between this (incoming) VideoConduit and an audio conduit.
-   */
-  void SyncTo(WebrtcAudioConduit *aConduit);
-
   /**
    * Function to attach Renderer end-point for the Media-Video conduit.
    * @param aRenderer : Reference to the concrete mozilla Video renderer implementation
@@ -250,6 +245,8 @@ public:
   bool SetLocalCNAME(const char* cname) override;
   bool SetLocalMID(const std::string& mid) override;
 
+  void SetSyncGroup(const std::string& group) override;
+
   bool GetRemoteSSRCLocked(unsigned int* ssrc);
   bool SetRemoteSSRCLocked(unsigned int ssrc);
 

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp

@@ -330,18 +330,20 @@ PeerConnectionMedia::UpdateMediaPipelines()
   WebrtcGmpPCHandleSetter setter(mParentHandle);
 
   for (RefPtr<TransceiverImpl>& transceiver : mTransceivers) {
-    nsresult rv = transceiver->UpdateConduit();
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
+    transceiver->ResetSync();
+  }
 
+  for (RefPtr<TransceiverImpl>& transceiver : mTransceivers) {
     if (!transceiver->IsVideo()) {
-      rv = transceiver->SyncWithMatchingVideoConduits(mTransceivers);
+      nsresult rv = transceiver->SyncWithMatchingVideoConduits(mTransceivers);
       if (NS_FAILED(rv)) {
         return rv;
       }
-      // TODO: If there is no audio, we should probably de-sync. However, this
-      // has never been done before, and it is unclear whether it is safe...
+    }
+
+    nsresult rv = transceiver->UpdateConduit();
+    if (NS_FAILED(rv)) {
+      return rv;
     }
   }
 

media/webrtc/signaling/src/peerconnection/TransceiverImpl.cpp

@@ -287,6 +287,12 @@ TransceiverImpl::UpdatePrincipal(nsIPrincipal* aPrincipal)
   return NS_OK;
 }
 
+void
+TransceiverImpl::ResetSync()
+{
+  mConduit->SetSyncGroup("");
+}
+
 nsresult
 TransceiverImpl::SyncWithMatchingVideoConduits(
     std::vector<RefPtr<TransceiverImpl>>& transceivers)
@@ -319,15 +325,18 @@ TransceiverImpl::SyncWithMatchingVideoConduits(
          transceiver->mJsepTransceiver->mRecvTrack.GetStreamIds()) {
       if (myReceiveStreamIds.count(streamId)) {
         // Ok, we have one video, one non-video - cross the streams!
-        WebrtcAudioConduit *audio_conduit =
-          static_cast<WebrtcAudioConduit*>(mConduit.get());
-        WebrtcVideoConduit *video_conduit =
-          static_cast<WebrtcVideoConduit*>(transceiver->mConduit.get());
+        mConduit->SetSyncGroup(streamId);
+        transceiver->mConduit->SetSyncGroup(streamId);
 
-        video_conduit->SyncTo(audio_conduit);
         MOZ_MTLOG(ML_DEBUG, mPCHandle << "[" << mMid << "]: " << __FUNCTION__ <<
-                            " Syncing " << video_conduit << " to "
-                            << audio_conduit);
+                            " Syncing " << mConduit.get() << " to "
+                            << transceiver->mConduit.get());
+
+        // The sync code in call.cc only permits sync between audio stream and
+        // one video stream. They take the first match, so there's no point in
+        // continuing here. If we want to change the default, we should sort
+        // video streams here and only call SetSyncGroup on the chosen stream.
+        break;
       }
     }
   }

media/webrtc/signaling/src/peerconnection/TransceiverImpl.h

@@ -76,7 +76,8 @@ public:
 
   nsresult UpdatePrincipal(nsIPrincipal* aPrincipal);
 
-  // TODO: We probably need to de-Sync when transceivers are stopped.
+  void ResetSync();
+
   nsresult SyncWithMatchingVideoConduits(
       std::vector<RefPtr<TransceiverImpl>>& transceivers);
 

media/webrtc/trunk/webrtc/call/video_receive_stream.h

@@ -238,9 +238,6 @@ class VideoReceiveStream {
   // TODO(pbos): Add info on currently-received codec to Stats.
   virtual Stats GetStats() const = 0;
 
-  //TODO: find replacement for this using call interface
-  //virtual void SetSyncChannel(VoiceEngine* voice_engine, int audio_channel_id) = 0;
-
   // Takes ownership of the file, is responsible for closing it later.
   // Calling this method will close and finalize any current log.
   // Giving rtc::kInvalidPlatformFileValue disables logging.