Bug 1337777: if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream r=bwc

Note that this stumbles over the use of the PCHandle as a global when
initializing the OpenH264 gmp plugin.

MozReview-Commit-ID: 7GEvIwwsitk

dom/media/systemservices/MediaUtils.h

@@ -173,7 +173,7 @@ private:
  *     RefPtr<Bar> bar = new Bar();
  *     NS_DispatchToMainThread(media::NewRunnableFrom([bar]() mutable {
  *       // use bar
- *     });
+ *     }));
  *   }
  *
  * Capture is by-copy by default, so the nsRefPtr 'bar' is safely copied for

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

@@ -751,7 +751,7 @@ WebrtcAudioConduit::GetAudioFrame(int16_t speechData[],
 
 // Transport Layer Callbacks
 MediaConduitErrorCode
-WebrtcAudioConduit::ReceivedRTPPacket(const void *data, int len)
+WebrtcAudioConduit::ReceivedRTPPacket(const void *data, int len, uint32_t ssrc)
 {
   CSFLogDebug(logTag,  "%s : channel %d", __FUNCTION__, mChannel);
 

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

@@ -57,7 +57,7 @@ public:
    * APIs used by the registered external transport to this Conduit to
    * feed in received RTP Frames to the VoiceEngine for decoding
    */
-  virtual MediaConduitErrorCode ReceivedRTPPacket(const void *data, int len) override;
+  virtual MediaConduitErrorCode ReceivedRTPPacket(const void *data, int len, uint32_t ssrc) override;
 
   /**
    * APIs used by the registered external transport to this Conduit to
@@ -163,6 +163,7 @@ public:
                         size_t len) override;
 
   virtual uint64_t CodecPluginID() override { return 0; }
+  virtual void SetPCHandle(const std::string& aPCHandle) {}
 
   explicit WebrtcAudioConduit():
                       mVoiceEngine(nullptr),

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

@@ -196,7 +196,7 @@ public:
    * Obtained packets are passed to the Media-Engine for further
    * processing , say, decoding
    */
-  virtual MediaConduitErrorCode ReceivedRTPPacket(const void *data, int len) = 0;
+  virtual MediaConduitErrorCode ReceivedRTPPacket(const void *data, int len, uint32_t ssrc) = 0;
 
   /**
    * Function triggered on Incoming RTCP packet from the remote
@@ -278,6 +278,8 @@ public:
 
   virtual uint64_t CodecPluginID() = 0;
 
+  virtual void SetPCHandle(const std::string& aPCHandle) = 0;
+
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaSessionConduit)
 
 };

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

@@ -198,6 +198,8 @@ WebrtcVideoConduit::WebrtcVideoConduit(RefPtr<WebRtcCallWrapper> aCall)
   , mSendStreamConfig(this) // 'this' is stored but not  dereferenced in the constructor.
   , mRecvStream(nullptr)
   , mRecvStreamConfig(this) // 'this' is stored but not  dereferenced in the constructor.
+  , mRecvSSRCSet(false)
+  , mRecvSSRCSetInProgress(false)
   , mSendCodecPlugin(nullptr)
   , mRecvCodecPlugin(nullptr)
   , mVideoStatsTimer(do_CreateInstance(NS_TIMER_CONTRACTID))
@@ -682,11 +684,12 @@ bool
 WebrtcVideoConduit::SetRemoteSSRC(unsigned int ssrc)
 {
   mRecvStreamConfig.rtp.remote_ssrc = ssrc;
-  unsigned int current_ssrc;
 
+  unsigned int current_ssrc;
   if (!GetRemoteSSRC(&current_ssrc)) {
     return false;
   }
+  mRecvSSRCSet = true;
 
   if (current_ssrc == ssrc || !mEngineReceiving) {
     return true;
@@ -1137,6 +1140,20 @@ WebrtcVideoConduit::ConfigureRecvMediaCodecs(
       mRecvStreamConfig.rtp.fec.red_rtx_payload_type = -1;
     }
 
+    if (!mRecvSSRCSet) {
+      // Handle un-signalled SSRCs by creating a random one and then when it actually gets set,
+      // we'll destroy and recreate.  Simpler than trying to unwind all the logic that assumes
+      // the receive stream is created and started when we ConfigureRecvMediaCodecs()
+      unsigned int ssrc;
+      do {
+        SECStatus rv = PK11_GenerateRandom(reinterpret_cast<unsigned char*>(&ssrc), sizeof(ssrc));
+        if (rv != SECSuccess) {
+          return kMediaConduitUnknownError;
+        }
+      } while (ssrc == 0); // webrtc.org code has fits if you select an SSRC of 0
+
+      mRecvStreamConfig.rtp.remote_ssrc = ssrc;
+    }
     // FIXME(jesup) - Bug 1325447 -- SSRCs configured here are a problem.
     // 0 isn't allowed.  Would be best to ask for a random SSRC from the RTP code.
     // Would need to call rtp_sender.cc -- GenerateSSRC(), which isn't exposed.  It's called on
@@ -1149,7 +1166,8 @@ WebrtcVideoConduit::ConfigureRecvMediaCodecs(
       if (rv != SECSuccess) {
         return kMediaConduitUnknownError;
       }
-    } while (ssrc == mRecvStreamConfig.rtp.remote_ssrc);
+    } while (ssrc == mRecvStreamConfig.rtp.remote_ssrc || ssrc == 0);
+    // webrtc.org code has fits if you select an SSRC of 0
 
     mRecvStreamConfig.rtp.local_ssrc = ssrc;
 
@@ -1157,8 +1175,8 @@ WebrtcVideoConduit::ConfigureRecvMediaCodecs(
     mRecvCodecList.SwapElements(recv_codecs);
     recv_codecs.Clear();
     mRecvStreamConfig.rtp.rtx.clear();
-    // Rebuilds mRecvStream from mRecvStreamConfig
     DeleteRecvStream();
+    // Rebuilds mRecvStream from mRecvStreamConfig
     MediaConduitErrorCode rval = CreateRecvStream();
     if (rval != kMediaConduitNoError) {
       CSFLogError(logTag, "%s Start Receive Error %d ", __FUNCTION__, rval);
@@ -1734,8 +1752,61 @@ WebrtcVideoConduit::DeliverPacket(const void* data, int len)
 }
 
 MediaConduitErrorCode
-WebrtcVideoConduit::ReceivedRTPPacket(const void* data, int len)
+WebrtcVideoConduit::ReceivedRTPPacket(const void* data, int len, uint32_t ssrc)
 {
+  bool queue = mRecvSSRCSetInProgress;
+  if (!mRecvSSRCSet && !mRecvSSRCSetInProgress) {
+    mRecvSSRCSetInProgress = true;
+    queue = true;
+    // Handle the ssrc-not-signaled case; lock onto first ssrc
+    // We can't just do this here; it has to happen on MainThread :-(
+    // We also don't want to drop the packet, nor stall this thread, so we hold
+    // the packet (and any following) for inserting once the SSRC is set.
+
+    // Ensure lamba captures refs
+    RefPtr<WebrtcVideoConduit> self = this;
+    nsCOMPtr<nsIThread> thread;
+    if (NS_WARN_IF(NS_FAILED(NS_GetCurrentThread(getter_AddRefs(thread))))) {
+      return kMediaConduitRTPProcessingFailed;
+    }
+    NS_DispatchToMainThread(media::NewRunnableFrom([self, thread, ssrc]() mutable {
+          // Normally this is done in CreateOrUpdateMediaPipeline() for
+          // initial creation and renegotiation, but here we're rebuilding the
+          // Receive channel at a lower level.  This is needed whenever we're
+          // creating a GMPVideoCodec (in particular, H264) so it can communicate
+          // errors to the PC.
+          WebrtcGmpPCHandleSetter setter(self->mPCHandle);
+          self->SetRemoteSSRC(ssrc); // this will likely re-create the VideoReceiveStream
+          // We want to unblock the queued packets on the original thread
+          thread->Dispatch(media::NewRunnableFrom([self]() mutable {
+                self->mRecvSSRCSetInProgress = false;
+                // SSRC is set; insert queued packets
+                for (auto& packet : self->mQueuedPackets) {
+                  CSFLogDebug(logTag, "%s: seq# %u, Len %d ", __FUNCTION__,
+                              (uint16_t)ntohs(((uint16_t*) packet->mData)[1]), packet->mLen);
+
+                  if (self->DeliverPacket(packet->mData, packet->mLen) != kMediaConduitNoError) {
+                    CSFLogError(logTag, "%s RTP Processing Failed", __FUNCTION__);
+                    // Keep delivering and then clear the queue
+                  }
+                }
+                self->mQueuedPackets.Clear();
+
+                return NS_OK;
+              }), NS_DISPATCH_NORMAL);
+          return NS_OK;
+        }));
+    // we'll return after queuing
+  }
+  if (queue) {
+    // capture packet for insertion after ssrc is set
+    UniquePtr<QueuedPacket> packet((QueuedPacket*) malloc(sizeof(QueuedPacket) + len-1));
+    packet->mLen = len;
+    memcpy(packet->mData, data, len);
+    mQueuedPackets.AppendElement(Move(packet));
+    return kMediaConduitNoError;
+  }
+
   CSFLogDebug(logTag, "%s: seq# %u, Len %d ", __FUNCTION__,
               (uint16_t)ntohs(((uint16_t*) data)[1]), len);
 

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

@@ -103,7 +103,7 @@ public:
    * APIs used by the registered external transport to this Conduit to
    * feed in received RTP Frames to the VideoEngine for decoding
    */
-  virtual MediaConduitErrorCode ReceivedRTPPacket(const void* data, int len) override;
+  virtual MediaConduitErrorCode ReceivedRTPPacket(const void* data, int len, uint32_t ssrc) override;
 
   /**
    * APIs used by the registered external transport to this Conduit to
@@ -262,6 +262,10 @@ public:
 
   virtual uint64_t CodecPluginID() override;
 
+  virtual void SetPCHandle(const std::string& aPCHandle) override {
+    mPCHandle = aPCHandle;
+  }
+
   unsigned short SendingWidth() override {
     return mSendingWidth;
   }
@@ -469,6 +473,7 @@ private:
   unsigned short mNumReceivingStreams;
   bool mVideoLatencyTestEnable;
   uint64_t mVideoLatencyAvg;
+  // all in bps!
   int mMinBitrate;
   int mStartBitrate;
   int mPrefMaxBitrate;
@@ -496,6 +501,17 @@ private:
   webrtc::VideoReceiveStream* mRecvStream;
   // Must call webrtc::Call::DestroyVideoReceiveStream to delete
   webrtc::VideoReceiveStream::Config mRecvStreamConfig;
+  // We can't create mRecvStream without knowing the remote SSRC
+  // Atomic since we key off this on packet insertion, which happens
+  // on a different thread.
+  Atomic<bool> mRecvSSRCSet;
+  // The runnable to set the SSRC is in-flight; queue packets until it's done.
+  bool mRecvSSRCSetInProgress;
+  struct QueuedPacket {
+    int mLen;
+    uint8_t mData[1];
+  };
+  nsTArray<UniquePtr<QueuedPacket>> mQueuedPackets;
 
   // The lifetime of these codecs are maintained by the VideoConduit instance.
   // They are passed to the webrtc::VideoSendStream or VideoReceiveStream,
@@ -508,6 +524,8 @@ private:
   nsCOMPtr<nsITimer> mVideoStatsTimer;
   SendStreamStatistics mSendStreamStats;
   ReceiveStreamStatistics mRecvStreamStats;
+
+  std::string mPCHandle;
 };
 } // end namespace
 

media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp

@@ -1071,13 +1071,12 @@ void MediaPipeline::RtpPacketReceived(TransportLayer *layer,
 
     MOZ_MTLOG(ML_NOTICE, "Error unprotecting RTP in " << description_
               << "len= " << len << "[" << tmp << "...]");
-
     return;
   }
   MOZ_MTLOG(ML_DEBUG, description_ << " received RTP packet.");
   increment_rtp_packets_received(out_len);
 
-  (void)conduit_->ReceivedRTPPacket(inner_data.get(), out_len);  // Ignore error codes
+  (void)conduit_->ReceivedRTPPacket(inner_data.get(), out_len, header.ssrc);  // Ignore error codes
 }
 
 void MediaPipeline::RtcpPacketReceived(TransportLayer *layer,

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

@@ -446,6 +446,7 @@ MediaPipelineFactory::CreateOrUpdateMediaPipeline(
     if (NS_FAILED(rv)) {
       return rv;
     }
+    conduit->SetPCHandle(mPC->GetHandle());
   } else {
     // We've created the TransportFlow, nothing else to do here.
     return NS_OK;
@@ -839,11 +840,9 @@ MediaPipelineFactory::GetOrCreateVideoConduit(
     // NOTE(pkerr) - this is new behavior. Needed because the CreateVideoReceiveStream
     // method of the Call API will assert (in debug) and fail if a value is not provided
     // for the remote_ssrc that will be used by the far-end sender.
-    if (ssrcs->empty()) {
+    if (!ssrcs->empty()) {
-      MOZ_MTLOG(ML_ERROR, "No SSRC set for receive track");
-      return NS_ERROR_FAILURE;
-    }
       conduit->SetRemoteSSRC(ssrcs->front());
+    }
 
     if (!extmaps.empty()) {
       conduit->AddLocalRTPExtensions(false, extmaps);