Bug 1337777: if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream r=bwc a=lizzard
authorRandell Jesup <rjesup@jesup.org>
Thu, 02 Mar 2017 15:11:22 -0500
changeset 376743 f6e2a8eabd7ea17672a006d6e69a6718e1083002
parent 376742 a571068c57c875374166901a883e7a635c066e92
child 376744 c1babb29398b8bbef723dd17f575d616822b8327
push id7031
push userrjesup@wgate.com
push dateSat, 11 Mar 2017 01:48:21 +0000
treeherdermozilla-beta@65c3d718dd5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc, lizzard
bugs1337777
milestone53.0
Bug 1337777: if no receive-SSRC was signaled for video, on the first packet reset the VideoReceiveStream r=bwc a=lizzard Note that this stumbles over the use of the PCHandle as a global when initializing the OpenH264 gmp plugin.
dom/media/systemservices/MediaUtils.h
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.h
media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.h
media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
--- a/dom/media/systemservices/MediaUtils.h
+++ b/dom/media/systemservices/MediaUtils.h
@@ -168,17 +168,17 @@ private:
  *
  * It's worse with more variables. Lambdas have a leg up with variable capture:
  *
  *   void Foo()
  *   {
  *     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
  * access on the other thread (threadsafe refcounting in bar is assumed).
  *
  * The 'mutable' keyword is only needed for non-const access to bar.
  */
 
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -759,17 +759,17 @@ WebrtcAudioConduit::GetAudioFrame(int16_
 #endif
   CSFLogDebug(logTag,"%s GetAudioFrame:Got samples: length %d ",__FUNCTION__,
                                                                lengthSamples);
   return kMediaConduitNoError;
 }
 
 // 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);
 
   if(mEngineReceiving)
   {
 #if !defined(MOZILLA_EXTERNAL_LINKAGE)
     if (MOZ_LOG_TEST(GetLatencyLog(), LogLevel::Debug)) {
       // timestamp is at 32 bits in ([1])
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -52,17 +52,17 @@ class WebrtcAudioConduit: public AudioSe
 public:
   //VoiceEngine defined constant for Payload Name Size.
   static const unsigned int CODEC_PLNAME_SIZE;
 
   /**
    * 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
    * feed in received RTCP Frames to the VoiceEngine for decoding
    */
   virtual MediaConduitErrorCode ReceivedRTCPPacket(const void *data, int len) override;
 
   virtual MediaConduitErrorCode StopTransmitting() override;
@@ -158,16 +158,17 @@ public:
   /**
    * Webrtc transport implementation to send and receive RTCP packet.
    * AudioConduit registers itself as ExternalTransport to the VoiceEngine
    */
   virtual bool SendRtcp(const uint8_t *data,
                         size_t len) override;
 
   virtual uint64_t CodecPluginID() override { return 0; }
+  virtual void SetPCHandle(const std::string& aPCHandle) {}
 
   explicit WebrtcAudioConduit():
                       mVoiceEngine(nullptr),
                       mTransportMonitor("WebrtcAudioConduit"),
                       mTransmitterTransport(nullptr),
                       mReceiverTransport(nullptr),
                       mEngineTransmitting(false),
                       mEngineReceiving(false),
--- a/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
+++ b/media/webrtc/signaling/src/media-conduit/MediaConduitInterface.h
@@ -191,17 +191,17 @@ public:
   /**
    * Function triggered on Incoming RTP packet from the remote
    * endpoint by the transport implementation.
    * @param data : RTP Packet (audio/video) to be processed
    * @param len  : Length of the media packet
    * 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
    * endpoint by the transport implementation.
    * @param data : RTCP Packet (audio/video) to be processed
    * @param len  : Length of the media packet
    * Obtained packets are passed to the Media-Engine for further
    * processing , say, decoding
@@ -273,16 +273,18 @@ public:
                                      uint32_t* cumulativeLost,
                                      int32_t* rttMs) = 0;
   virtual bool GetRTCPSenderReport(DOMHighResTimeStamp* timestamp,
                                    unsigned int* packetsSent,
                                    uint64_t* bytesSent) = 0;
 
   virtual uint64_t CodecPluginID() = 0;
 
+  virtual void SetPCHandle(const std::string& aPCHandle) = 0;
+
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(MediaSessionConduit)
 
 };
 
 // Abstract base classes for external encoder/decoder.
 class CodecPluginID
 {
 public:
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -197,16 +197,18 @@ WebrtcVideoConduit::WebrtcVideoConduit(R
   , mNegotiatedMaxBitrate(0)
   , mMinBitrateEstimate(0)
   , mCodecMode(webrtc::kRealtimeVideo)
   , mCall(aCall) // refcounted store of the call object
   , mSendStream(nullptr)
   , 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))
 {
   mRecvStreamConfig.renderer = this;
 
   // Video Stats Callback
   nsTimerCallbackFunc callback = [](nsITimer* aTimer, void* aClosure) {
@@ -682,21 +684,22 @@ WebrtcVideoConduit::ConfigureSendMediaCo
 
   return condError;
 }
 
 bool
 WebrtcVideoConduit::SetRemoteSSRC(unsigned int ssrc)
 {
   mRecvStreamConfig.rtp.remote_ssrc = ssrc;
+
   unsigned int current_ssrc;
-
   if (!GetRemoteSSRC(&current_ssrc)) {
     return false;
   }
+  mRecvSSRCSet = true;
 
   if (current_ssrc == ssrc || !mEngineReceiving) {
     return true;
   }
 
   if (StopReceiving() != kMediaConduitNoError) {
     return false;
   }
@@ -1139,38 +1142,53 @@ WebrtcVideoConduit::ConfigureRecvMediaCo
     mRecvStreamConfig.rtp.keyframe_method = kf_request_method;
 
     if (use_fec) {
       mRecvStreamConfig.rtp.fec.ulpfec_payload_type = ulpfec_payload_type;
       mRecvStreamConfig.rtp.fec.red_payload_type = red_payload_type;
       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
     // collision, or when we decide to send.  it should be called on receiver creation.
     // Here, we're generating the SSRC value - but this causes ssrc_forced in set in rtp_sender,
     // which locks us into the SSRC - even a collision won't change it!!!
     auto ssrc = mRecvStreamConfig.rtp.remote_ssrc;
     do {
       SECStatus rv = PK11_GenerateRandom(reinterpret_cast<unsigned char*>(&ssrc), sizeof(ssrc));
       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;
 
     // XXX Copy over those that are the same and don't rebuild them
     mRecvCodecList.SwapElements(recv_codecs);
     recv_codecs.Clear();
     mRecvStreamConfig.rtp.rtx.clear();
+    DeleteRecvStream();
     // Rebuilds mRecvStream from mRecvStreamConfig
-    DeleteRecvStream();
     MediaConduitErrorCode rval = CreateRecvStream();
     if (rval != kMediaConduitNoError) {
       CSFLogError(logTag, "%s Start Receive Error %d ", __FUNCTION__, rval);
       return rval;
     }
 
     return StartReceiving();
   }
@@ -1740,18 +1758,73 @@ WebrtcVideoConduit::DeliverPacket(const 
     CSFLogError(logTag, "%s DeliverPacket Failed, %d", __FUNCTION__, status);
     return kMediaConduitRTPProcessingFailed;
   }
 
   return kMediaConduitNoError;
 }
 
 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.
+#if !defined(MOZILLA_EXTERNAL_LINKAGE)
+          WebrtcGmpPCHandleSetter setter(self->mPCHandle);
+#endif
+          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);
 
   if (DeliverPacket(data, len) != kMediaConduitNoError) {
     CSFLogError(logTag, "%s RTP Processing Failed", __FUNCTION__);
     return kMediaConduitRTPProcessingFailed;
   }
   return kMediaConduitNoError;
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ -97,17 +97,17 @@ public:
    */
   virtual MediaConduitErrorCode AttachRenderer(RefPtr<mozilla::VideoRenderer> aVideoRenderer) override;
   virtual void DetachRenderer() override;
 
   /**
    * 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
    * feed in received RTP Frames to the VideoEngine for decoding
    */
   virtual MediaConduitErrorCode ReceivedRTCPPacket(const void* data, int len) override;
 
   virtual MediaConduitErrorCode StopTransmitting() override;
@@ -256,16 +256,20 @@ public:
    * ------------------------------------
    */
   virtual bool SmoothsRenderedFrames() const override {
     return false;
   }
 
   virtual uint64_t CodecPluginID() override;
 
+  virtual void SetPCHandle(const std::string& aPCHandle) override {
+    mPCHandle = aPCHandle;
+  }
+
   unsigned short SendingWidth() override {
     return mSendingWidth;
   }
 
   unsigned short SendingHeight() override {
     return mSendingHeight;
   }
 
@@ -463,16 +467,17 @@ private:
   unsigned short mReceivingWidth;
   unsigned short mReceivingHeight;
   unsigned int   mSendingFramerate;
   // scaled by *10 because Atomic<double/float> isn't supported
   mozilla::Atomic<int32_t, mozilla::Relaxed> mLastFramerateTenths;
   unsigned short mNumReceivingStreams;
   bool mVideoLatencyTestEnable;
   uint64_t mVideoLatencyAvg;
+  // all in bps!
   int mMinBitrate;
   int mStartBitrate;
   int mPrefMaxBitrate;
   int mNegotiatedMaxBitrate;
   int mMinBitrateEstimate;
 
   bool mRtpStreamIdEnabled;
   uint8_t mRtpStreamIdExtId;
@@ -493,24 +498,37 @@ private:
   // Must call webrtc::Call::DestroyVideoSendStream to delete
   webrtc::VideoSendStream::Config mSendStreamConfig;
   VideoEncoderConfigBuilder mEncoderConfig;
   webrtc::VideoCodecH264 mEncoderSpecificH264;
 
   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,
   // on construction.
   nsAutoPtr<webrtc::VideoEncoder> mEncoder; // only one encoder for now
   std::vector<std::unique_ptr<webrtc::VideoDecoder>> mDecoders;
   WebrtcVideoEncoder* mSendCodecPlugin;
   WebrtcVideoDecoder* mRecvCodecPlugin;
 
   nsCOMPtr<nsITimer> mVideoStatsTimer;
   SendStreamStatistics mSendStreamStats;
   ReceiveStreamStatistics mRecvStreamStats;
+
+  std::string mPCHandle;
 };
 } // end namespace
 
 #endif
--- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
+++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ -1081,23 +1081,22 @@ void MediaPipeline::RtpPacketReceived(Tr
     SprintfLiteral(tmp, "%.2x %.2x %.2x %.2x",
                    inner_data[0],
                    inner_data[1],
                    inner_data[2],
                    inner_data[3]);
 
     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,
                                        const unsigned char *data,
                                        size_t len) {
   if (!transport_->pipeline()) {
     MOZ_MTLOG(ML_DEBUG, "Discarding incoming packet; transport disconnected");
     return;
--- a/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
@@ -449,16 +449,17 @@ MediaPipelineFactory::CreateOrUpdateMedi
     if (NS_FAILED(rv)) {
       return rv;
     }
   } else if (aTrack.GetMediaType() == SdpMediaSection::kVideo) {
     rv = GetOrCreateVideoConduit(aTrackPair, aTrack, &conduit);
     if (NS_FAILED(rv)) {
       return rv;
     }
+    conduit->SetPCHandle(mPC->GetHandle());
   } else {
     // We've created the TransportFlow, nothing else to do here.
     return NS_OK;
   }
 
   if (aTrack.GetActive()) {
     if (receiving) {
       auto error = conduit->StartReceiving();
@@ -833,21 +834,19 @@ MediaPipelineFactory::GetOrCreateVideoCo
         return NS_ERROR_FAILURE;
       }
     }
 
     ssrcs = &aTrack.GetSsrcs();
     // 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()) {
-      MOZ_MTLOG(ML_ERROR, "No SSRC set for receive track");
-      return NS_ERROR_FAILURE;
+    if (!ssrcs->empty()) {
+      conduit->SetRemoteSSRC(ssrcs->front());
     }
-    conduit->SetRemoteSSRC(ssrcs->front());
 
     auto error = conduit->ConfigureRecvMediaCodecs(configs.values);
     if (error) {
       MOZ_MTLOG(ML_ERROR, "ConfigureRecvMediaCodecs failed: " << error);
       return NS_ERROR_FAILURE;
     }
   } else { //Create a send side
     // For now we only expect to have one ssrc per local track.