Bug 873888 - Wait for construction of tracks before returning onAddStream. r=jesup, a=akeybl
☠☠ backed out by 8f1d6355024d ☠ ☠
authorEthan Hugg <ethanhugg@gmail.com>
Tue, 21 May 2013 07:49:50 -0700
changeset 142768 217559e937471e32f44e6dd21d9339510ed6ff30
parent 142767 52db029185a4b143c52d652b559706efdda1e10b
child 142769 4e643d5826ac8db4905c47c9f087e84799ce54ea
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, akeybl
bugs873888
milestone23.0a2
Bug 873888 - Wait for construction of tracks before returning onAddStream. r=jesup, a=akeybl
media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
media/webrtc/signaling/src/sipcc/include/vcm.h
media/webrtc/signaling/test/FakeMediaStreams.h
--- a/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
+++ b/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp
@@ -961,16 +961,71 @@ short vcmCreateRemoteStream(cc_mcapid_t 
                         mcap_id,
                         peerconnection,
                         pc_stream_id,
                         &ret));
 
   return ret;
 }
 
+/*
+ * Add remote stream hint
+ *
+ * We are sending track information up to PeerConnection before
+ * the tracks exist so it knows when the stream is fully constructed.
+ *
+ * @param[in] peerconnection
+ * @param[in] pc_stream_id
+ * @param[in] is_video
+ *
+ * Returns: zero(0) for success; otherwise, ERROR for failure
+ */
+static short vcmAddRemoteStreamHint_m(
+  const char *peerconnection,
+  int pc_stream_id,
+  cc_boolean is_video) {
+  nsresult res;
+
+  sipcc::PeerConnectionWrapper pc(peerconnection);
+  ENSURE_PC(pc, VCM_ERROR);
+
+  res = pc.impl()->media()->AddRemoteStreamHint(pc_stream_id,
+    is_video ? TRUE : FALSE);
+  if (NS_FAILED(res)) {
+    return VCM_ERROR;
+  }
+
+  CSFLogDebug( logTag, "%s: added remote stream hint %u with index %d",
+    __FUNCTION__, is_video, pc_stream_id);
+
+  return 0;
+}
+
+/*
+ * Add remote stream hint
+ *
+ * This is a thunk to vcmAddRemoteStreamHint_m
+ *
+ * Returns: zero(0) for success; otherwise, ERROR for failure
+ */
+short vcmAddRemoteStreamHint(
+  const char *peerconnection,
+  int pc_stream_id,
+  cc_boolean is_video) {
+  short ret = 0;
+
+  mozilla::SyncRunnable::DispatchToThread(VcmSIPCCBinding::getMainThread(),
+      WrapRunnableNMRet(&vcmAddRemoteStreamHint_m,
+                        peerconnection,
+                        pc_stream_id,
+                        is_video,
+                        &ret));
+
+  return ret;
+}
 
 /*
  * Get DTLS key data
  *
  *  @param[in]  peerconnection - the peerconnection in use
  *  @param[out] digest_algp    - the digest algorithm e.g. 'SHA-1'
  *  @param[in] max_digest_alg_len - length of string
  *  @param[out] digestp        - the digest string
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -43,16 +43,17 @@
 #include "nsPrintfCString.h"
 #include "nsURLHelper.h"
 #include "nsNetUtil.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/RTCConfigurationBinding.h"
 #include "MediaStreamList.h"
 #include "nsIScriptGlobalObject.h"
 #include "jsapi.h"
+#include "DOMMediaStream.h"
 #endif
 
 #ifndef USE_FAKE_MEDIA_STREAMS
 #include "MediaSegment.h"
 #endif
 
 #define ICE_PARSING "In RTCConfiguration passed to RTCPeerConnection constructor"
 
@@ -137,16 +138,43 @@ public:
     }
     if ((mCallState == CREATEOFFER) || (mCallState == CREATEANSWER)) {
         mSdpStr = aInfo->getSDP();
     }
   }
 
   ~PeerConnectionObserverDispatch(){}
 
+#ifdef MOZILLA_INTERNAL_API
+  class TracksAvailableCallback : public DOMMediaStream::OnTracksAvailableCallback
+  {
+  public:
+    TracksAvailableCallback(DOMMediaStream::TrackTypeHints aTrackTypeHints,
+                            nsCOMPtr<IPeerConnectionObserver> aObserver)
+      : DOMMediaStream::OnTracksAvailableCallback(aTrackTypeHints),
+        mObserver(aObserver) {}
+
+    virtual void NotifyTracksAvailable(DOMMediaStream* aStream) MOZ_OVERRIDE
+    {
+      MOZ_ASSERT(NS_IsMainThread());
+
+      // Start currentTime from the point where this stream was successfully
+      // returned.
+      aStream->SetLogicalStreamStartTime(aStream->GetStream()->GetCurrentTime());
+
+      CSFLogInfo(logTag, "Returning success for OnAddStream()");
+      // We are running on main thread here so we shouldn't have a race
+      // on this callback
+      mObserver->OnAddStream(aStream);
+    }
+
+    nsCOMPtr<IPeerConnectionObserver> mObserver;
+  };
+#endif
+
   NS_IMETHOD Run() {
 
     CSFLogInfo(logTag, "PeerConnectionObserverDispatch processing "
                        "mCallState = %d (%s)", mCallState, mStateStr.c_str());
 
     if (mCallState == SETLOCALDESCERROR || mCallState == SETREMOTEDESCERROR) {
       const std::vector<std::string> &errors = mPC->GetSdpParseErrors();
       std::vector<std::string>::const_iterator i;
@@ -225,24 +253,31 @@ public:
             CSFLogError(logTag, "%s: GetRemoteStream returned NULL", __FUNCTION__);
           } else {
             stream = mRemoteStream->GetMediaStream();
           }
 
           if (!stream) {
             CSFLogError(logTag, "%s: GetMediaStream returned NULL", __FUNCTION__);
           } else {
+#ifdef MOZILLA_INTERNAL_API
+            TracksAvailableCallback* tracksAvailableCallback =
+              new TracksAvailableCallback(mRemoteStream->mTrackTypeHints, mObserver);
+
+            stream->OnTracksAvailable(tracksAvailableCallback);
+#else
             // We provide a type field because it is in the IDL
             // and we want code that looks at it not to crash.
             // We use "video" so that if an app looks for
             // that string it has some chance of working.
             // TODO(ekr@rtfm.com): Bug 834847
             // The correct way for content JS to know stream type
             // is via get{Audio,Video}Tracks. See Bug 834835.
             mObserver->OnAddStream(stream, "video");
+#endif
           }
           break;
         }
 
       case UPDATELOCALDESC:
         /* No action necessary */
         break;
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp
@@ -385,16 +385,35 @@ PeerConnectionMedia::AddRemoteStream(nsR
 
   *aIndex = mRemoteSourceStreams.Length();
 
   mRemoteSourceStreams.AppendElement(aInfo);
 
   return NS_OK;
 }
 
+nsresult
+PeerConnectionMedia::AddRemoteStreamHint(int aIndex, bool aIsVideo)
+{
+  if (aIndex >= mRemoteSourceStreams.Length()) {
+    return NS_ERROR_ILLEGAL_VALUE;
+  }
+
+  RemoteSourceStreamInfo *pInfo = mRemoteSourceStreams.ElementAt(aIndex);
+  MOZ_ASSERT(pInfo);
+
+  if (aIsVideo) {
+    pInfo->mTrackTypeHints |= DOMMediaStream::HINT_CONTENTS_VIDEO;
+  } else {
+    pInfo->mTrackTypeHints |= DOMMediaStream::HINT_CONTENTS_AUDIO;
+  }
+
+  return NS_OK;
+}
+
 
 void
 PeerConnectionMedia::IceGatheringCompleted(NrIceCtx *aCtx)
 {
   MOZ_ASSERT(aCtx);
   SignalIceGatheringCompleted(aCtx);
 }
 
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h
@@ -202,30 +202,34 @@ private:
 class RemoteSourceStreamInfo {
  public:
   typedef mozilla::DOMMediaStream DOMMediaStream;
 
 RemoteSourceStreamInfo(already_AddRefed<DOMMediaStream> aMediaStream,
                        PeerConnectionMedia *aParent)
     : mMediaStream(aMediaStream),
       mPipelines(),
-      mParent(aParent) {
+      mParent(aParent),
+      mTrackTypeHints(0) {
       MOZ_ASSERT(mMediaStream);
     }
 
   DOMMediaStream* GetMediaStream() {
     return mMediaStream;
   }
   void StorePipeline(int aTrack, bool aIsVideo,
                      mozilla::RefPtr<mozilla::MediaPipeline> aPipeline);
 
   void DetachTransport_s();
   void DetachMedia_m();
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RemoteSourceStreamInfo)
+
+public:
+  DOMMediaStream::TrackTypeHints mTrackTypeHints;
  private:
   nsRefPtr<DOMMediaStream> mMediaStream;
   std::map<int, mozilla::RefPtr<mozilla::MediaPipeline> > mPipelines;
   std::map<int, bool> mTypes;
   PeerConnectionMedia *mParent;
 };
 
 class PeerConnectionMedia : public sigslot::has_slots<> {
@@ -266,16 +270,17 @@ class PeerConnectionMedia : public sigsl
   uint32_t RemoteStreamsLength()
   {
     return mRemoteSourceStreams.Length();
   }
   RemoteSourceStreamInfo* GetRemoteStream(int index);
 
   // Add a remote stream. Returns the index in index
   nsresult AddRemoteStream(nsRefPtr<RemoteSourceStreamInfo> aInfo, int *aIndex);
+  nsresult AddRemoteStreamHint(int aIndex, bool aIsVideo);
 
   const nsCOMPtr<nsIThread>& GetMainThread() const { return mMainThread; }
   const nsCOMPtr<nsIEventTarget>& GetSTSThread() const { return mSTSThread; }
 
   // Get a transport flow either RTP/RTCP for a particular stream
   // A stream can be of audio/video/datachannel/budled(?) types
   mozilla::RefPtr<mozilla::TransportFlow> GetTransportFlow(int aStreamIndex,
                                                            bool aIsRtcp) {
--- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
+++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
@@ -20,16 +20,18 @@
 #include "dns_utils.h"
 #include "sip_interface_regmgr.h"
 #include "platform_api.h"
 #include "vcm.h"
 #include "prlog.h"
 #include "plstr.h"
 #include "sdp_private.h"
 
+static const char* logTag = "gsm_sdp";
+
 //TODO Need to place this in a portable location
 #define MULTICAST_START_ADDRESS 0xe1000000
 #define MULTICAST_END_ADDRESS   0xefffffff
 
 /* Only first octet contains codec type */
 #define GET_CODEC_TYPE(a)    ((uint8_t)((a) & 0XFF))
 
 #define GSMSDP_SET_MEDIA_DIABLE(media) \
@@ -6712,16 +6714,18 @@ static boolean gsmsdp_add_remote_stream(
  * media - the media object to add.
  *
  * returns TRUE for success and FALSE for failure
  */
 static boolean gsmsdp_add_remote_track(uint16_t idx, uint16_t track,
                                        fsmdef_dcb_t *dcb_p,
                                        fsmdef_media_t *media) {
   cc_media_remote_track_table_t *stream;
+  int vcm_ret;
+
   PR_ASSERT(idx < CC_MAX_STREAMS);
   if (idx >= CC_MAX_STREAMS)
     return FALSE;
 
   stream = &dcb_p->remote_media_stream_tbl->streams[idx];
 
   PR_ASSERT(stream->created);
   if (!stream->created)
@@ -6732,16 +6736,34 @@ static boolean gsmsdp_add_remote_track(u
     return FALSE;
 
   stream->track[stream->num_tracks].media_stream_track_id = track;
   stream->track[stream->num_tracks].video =
       (media->type == SDP_MEDIA_VIDEO) ? TRUE : FALSE;
 
   ++stream->num_tracks;
 
+  if (media->type == SDP_MEDIA_VIDEO) {
+    vcm_ret = vcmAddRemoteStreamHint(dcb_p->peerconnection, idx, TRUE);
+  } else if (media->type == SDP_MEDIA_AUDIO) {
+    vcm_ret = vcmAddRemoteStreamHint(dcb_p->peerconnection, idx, FALSE);
+  } else {
+    // No other track types should be valid here
+    MOZ_ASSERT(FALSE);
+    // Not setting a hint for this track type will simply cause the
+    // onaddstream callback not to wait for the track to be ready.
+    vcm_ret = 0;
+  }
+
+  if (vcm_ret) {
+      CSFLogError(logTag, "%s: vcmAddRemoteStreamHint returned error: %d",
+          __FUNCTION__, vcm_ret);
+      return FALSE;
+  }
+
   return TRUE;
 }
 
 cc_causes_t
 gsmsdp_find_level_from_mid(fsmdef_dcb_t * dcb_p, const char * mid, uint16_t *level) {
 
     fsmdef_media_t  *media;
     u32              mid_id;
--- a/media/webrtc/signaling/src/sipcc/include/vcm.h
+++ b/media/webrtc/signaling/src/sipcc/include/vcm.h
@@ -495,16 +495,33 @@ short vcmStartIceChecks(const char *peer
  *
  *  Returns: zero(0) for success; otherwise, ERROR for failure
  */
 short vcmCreateRemoteStream(
              cc_mcapid_t mcap_id,
              const char *peerconnection,
              int *pc_stream_id);
 
+/*
+ * Add remote stream hint
+ *
+ * We are sending track information up to PeerConnection before
+ * the tracks exist so it knows when the stream is fully constructed.
+ *
+ * @param[in] peerconnection
+ * @param[in] pc_stream_id
+ * @param[in] is_video
+ *
+ * Returns: zero(0) for success; otherwise, ERROR for failure
+ */
+short vcmAddRemoteStreamHint(
+            const char *peerconnection,
+            int pc_stream_id,
+            cc_boolean is_video);
+
 /*!
  *  Release the allocated port
  * @param[in] mcap_id   - media capability id (0 is audio)
  * @param[in] group_id  - group identifier
  * @param[in] stream_id - stream identifier
  * @param[in] call_handle   - call handle
  * @param[in] port     - port to be released
  *
--- a/media/webrtc/signaling/test/FakeMediaStreams.h
+++ b/media/webrtc/signaling/test/FakeMediaStreams.h
@@ -124,17 +124,17 @@ class Fake_SourceMediaStream : public Fa
       while(!iter.IsEnded()) {
         mozilla::AudioChunk& chunk = *(iter);
         MOZ_ASSERT(chunk.mBuffer);
         const int16_t* buf =
           static_cast<const int16_t*>(chunk.mChannelData[0]);
         for(int i=0; i<chunk.mDuration; i++) {
           if(buf[i]) {
             //atleast one non-zero sample found.
-            nonZeroSample = true; 
+            nonZeroSample = true;
             break;
           }
         }
         //process next chunk
         iter.Next();
       }
       if(nonZeroSample) {
           //we increment segments count if
@@ -203,19 +203,20 @@ public:
 
     return ds.forget();
   }
 
   Fake_MediaStream *GetStream() { return mMediaStream; }
 
   // Hints to tell the SDP generator about whether this
   // MediaStream probably has audio and/or video
+  typedef uint8_t TrackTypeHints;
   enum {
-    HINT_CONTENTS_AUDIO = 0x00000001U,
-    HINT_CONTENTS_VIDEO = 0x00000002U
+    HINT_CONTENTS_AUDIO = 0x01,
+    HINT_CONTENTS_VIDEO = 0x02
   };
   uint32_t GetHintContents() const { return mHintContents; }
   void SetHintContents(uint32_t aHintContents) { mHintContents = aHintContents; }
 
 private:
   nsRefPtr<Fake_MediaStream> mMediaStream;
 
   // tells the SDP generator about whether this