Bug 1132318 - Merge SelectSendFrameRate with SelectSendResolution. r=bwc, a=abillings
authorRandell Jesup <rjesup@jesup.org>
Fri, 05 Jun 2015 20:27:38 -0400
changeset 266210 48c9f45a00f2
parent 266209 da14f82d9caf
child 266211 4241def0561b
push id4787
push userryanvm@gmail.com
push date2015-06-08 17:52 +0000
treeherdermozilla-beta@48c9f45a00f2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc, abillings
bugs1132318
milestone39.0
Bug 1132318 - Merge SelectSendFrameRate with SelectSendResolution. r=bwc, a=abillings
media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
media/webrtc/signaling/src/media-conduit/AudioConduit.h
media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
media/webrtc/signaling/src/media-conduit/VideoConduit.h
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp
@@ -64,17 +64,16 @@ WebrtcAudioConduit::~WebrtcAudioConduit(
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
 
   CSFLogDebug(logTag,  "%s ", __FUNCTION__);
   for(std::vector<AudioCodecConfig*>::size_type i=0;i < mRecvCodecList.size();i++)
   {
     delete mRecvCodecList[i];
   }
-  delete mCurSendCodecConfig;
 
   // The first one of a pair to be deleted shuts down media for both
   if(mPtrVoEXmedia)
   {
     mPtrVoEXmedia->SetExternalRecordingStatus(false);
     mPtrVoEXmedia->SetExternalPlayoutStatus(false);
   }
 
@@ -331,20 +330,22 @@ WebrtcAudioConduit::SetReceiverTransport
 MediaConduitErrorCode
 WebrtcAudioConduit::ConfigureSendMediaCodec(const AudioCodecConfig* codecConfig)
 {
   CSFLogDebug(logTag,  "%s ", __FUNCTION__);
   MediaConduitErrorCode condError = kMediaConduitNoError;
   int error = 0;//webrtc engine errors
   webrtc::CodecInst cinst;
 
-  //validate codec param
-  if((condError = ValidateCodecConfig(codecConfig, true)) != kMediaConduitNoError)
   {
-    return condError;
+    //validate codec param
+    if((condError = ValidateCodecConfig(codecConfig, true)) != kMediaConduitNoError)
+    {
+      return condError;
+    }
   }
 
   condError = StopTransmitting();
   if (condError != kMediaConduitNoError) {
     return condError;
   }
 
   if(!CodecConfigToWebRTCCodec(codecConfig,cinst))
@@ -382,26 +383,27 @@ WebrtcAudioConduit::ConfigureSendMediaCo
   }
 #endif
 
   condError = StartTransmitting();
   if (condError != kMediaConduitNoError) {
     return condError;
   }
 
-  //Copy the applied config for future reference.
-  delete mCurSendCodecConfig;
+  {
+    MutexAutoLock lock(mCodecMutex);
 
-  mCurSendCodecConfig = new AudioCodecConfig(codecConfig->mType,
-                                              codecConfig->mName,
-                                              codecConfig->mFreq,
-                                              codecConfig->mPacSize,
-                                              codecConfig->mChannels,
-                                              codecConfig->mRate);
-
+    //Copy the applied config for future reference.
+    mCurSendCodecConfig = new AudioCodecConfig(codecConfig->mType,
+                                               codecConfig->mName,
+                                               codecConfig->mFreq,
+                                               codecConfig->mPacSize,
+                                               codecConfig->mChannels,
+                                               codecConfig->mRate);
+  }
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcAudioConduit::ConfigureRecvMediaCodecs(
                     const std::vector<AudioCodecConfig*>& codecConfigList)
 {
   CSFLogDebug(logTag,  "%s ", __FUNCTION__);
@@ -989,17 +991,17 @@ WebrtcAudioConduit::CheckCodecForMatch(c
 
 
 /**
  * Perform validation on the codecConfig to be applied.
  * Verifies if the codec is already applied.
  */
 MediaConduitErrorCode
 WebrtcAudioConduit::ValidateCodecConfig(const AudioCodecConfig* codecInfo,
-                                        bool send) const
+                                        bool send)
 {
   bool codecAppliedAlready = false;
 
   if(!codecInfo)
   {
     CSFLogError(logTag, "%s Null CodecConfig ", __FUNCTION__);
     return kMediaConduitMalformedArgument;
   }
@@ -1016,16 +1018,18 @@ WebrtcAudioConduit::ValidateCodecConfig(
   {
     CSFLogError(logTag, "%s Channel Unsupported ", __FUNCTION__);
     return kMediaConduitMalformedArgument;
   }
 
   //check if we have the same codec already applied
   if(send)
   {
+    MutexAutoLock lock(mCodecMutex);
+
     codecAppliedAlready = CheckCodecsForMatch(mCurSendCodecConfig,codecInfo);
   } else {
     codecAppliedAlready = CheckCodecForMatch(codecInfo);
   }
 
   if(codecAppliedAlready)
   {
     CSFLogDebug(logTag, "%s Codec %s Already Applied  ", __FUNCTION__, codecInfo->mName.c_str());
--- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h
@@ -164,17 +164,17 @@ public:
   WebrtcAudioConduit():
                       mVoiceEngine(nullptr),
                       mTransportMonitor("WebrtcAudioConduit"),
                       mTransmitterTransport(nullptr),
                       mReceiverTransport(nullptr),
                       mEngineTransmitting(false),
                       mEngineReceiving(false),
                       mChannel(-1),
-                      mCurSendCodecConfig(nullptr),
+                      mCodecMutex("AudioConduit codec db"),
                       mCaptureDelay(150),
 #ifdef MOZILLA_INTERNAL_API
                       mLastTimestamp(0),
 #endif // MOZILLA_INTERNAL_API
                       mSamples(0),
                       mLastSyncLog(0)
   {
   }
@@ -240,17 +240,17 @@ private:
   bool CopyCodecToDB(const AudioCodecConfig* codecInfo);
 
   // Functions to verify if the codec passed is already in
   // conduits database
   bool CheckCodecForMatch(const AudioCodecConfig* codecInfo) const;
   bool CheckCodecsForMatch(const AudioCodecConfig* curCodecConfig,
                            const AudioCodecConfig* codecInfo) const;
   //Checks the codec to be applied
-  MediaConduitErrorCode ValidateCodecConfig(const AudioCodecConfig* codecInfo, bool send) const;
+  MediaConduitErrorCode ValidateCodecConfig(const AudioCodecConfig* codecInfo, bool send);
 
   //Utility function to dump recv codec database
   void DumpCodecDB() const;
 
   webrtc::VoiceEngine* mVoiceEngine;
   mozilla::ReentrantMonitor mTransportMonitor;
   mozilla::RefPtr<TransportInterface> mTransmitterTransport;
   mozilla::RefPtr<TransportInterface> mReceiverTransport;
@@ -272,17 +272,19 @@ private:
   struct Processing {
     TimeStamp mTimeStamp;
     uint32_t mRTPTimeStamp; // RTP timestamps received
   };
   nsAutoTArray<Processing,8> mProcessing;
 
   int mChannel;
   RecvCodecList    mRecvCodecList;
-  AudioCodecConfig* mCurSendCodecConfig;
+
+  Mutex mCodecMutex; // protects mCurSendCodecConfig
+  nsAutoPtr<AudioCodecConfig> mCurSendCodecConfig;
 
   // Current "capture" delay (really output plus input delay)
   int32_t mCaptureDelay;
 
 #ifdef MOZILLA_INTERNAL_API
   uint32_t mLastTimestamp;
 #endif // MOZILLA_INTERNAL_API
 
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ -69,17 +69,17 @@ WebrtcVideoConduit::WebrtcVideoConduit()
   mTransmitterTransport(nullptr),
   mReceiverTransport(nullptr),
   mRenderer(nullptr),
   mPtrExtCapture(nullptr),
   mEngineTransmitting(false),
   mEngineReceiving(false),
   mChannel(-1),
   mCapId(-1),
-  mCurSendCodecConfig(nullptr),
+  mCodecMutex("VideoConduit codec db"),
   mSendingWidth(0),
   mSendingHeight(0),
   mReceivingWidth(640),
   mReceivingHeight(480),
   mSendingFramerate(DEFAULT_VIDEO_MAX_FRAMERATE),
   mNumReceivingStreams(1),
   mVideoLatencyTestEnable(false),
   mVideoLatencyAvg(0),
@@ -95,18 +95,16 @@ WebrtcVideoConduit::~WebrtcVideoConduit(
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   CSFLogDebug(logTag,  "%s ", __FUNCTION__);
 
   for(std::vector<VideoCodecConfig*>::size_type i=0;i < mRecvCodecList.size();i++)
   {
     delete mRecvCodecList[i];
   }
 
-  delete mCurSendCodecConfig;
-
   // The first one of a pair to be deleted shuts down media for both
   //Deal with External Capturer
   if(mPtrViECapture)
   {
     mPtrViECapture->DisconnectCaptureDevice(mCapId);
     mPtrViECapture->ReleaseCaptureDevice(mCapId);
     mPtrExtCapture = nullptr;
   }
@@ -548,39 +546,41 @@ WebrtcVideoConduit::ConfigureCodecMode(w
 {
   CSFLogDebug(logTag,  "%s ", __FUNCTION__);
   mCodecMode = mode;
   return kMediaConduitNoError;
 }
 /**
  * Note: Setting the send-codec on the Video Engine will restart the encoder,
  * sets up new SSRC and reset RTP_RTCP module with the new codec setting.
+ *
+ * Note: this is called from MainThread, and the codec settings are read on
+ * videoframe delivery threads (i.e in SendVideoFrame().  With
+ * renegotiation/reconfiguration, this now needs a lock!  Alternatively
+ * changes could be queued until the next frame is delivered using an
+ * Atomic pointer and swaps.
  */
 MediaConduitErrorCode
 WebrtcVideoConduit::ConfigureSendMediaCodec(const VideoCodecConfig* codecConfig)
 {
   CSFLogDebug(logTag,  "%s for %s", __FUNCTION__, codecConfig ? codecConfig->mName.c_str() : "<null>");
   bool codecFound = false;
   MediaConduitErrorCode condError = kMediaConduitNoError;
   int error = 0; //webrtc engine errors
   webrtc::VideoCodec  video_codec;
   std::string payloadName;
 
   memset(&video_codec, 0, sizeof(video_codec));
 
-  //validate basic params
-  if((condError = ValidateCodecConfig(codecConfig,true)) != kMediaConduitNoError)
   {
-    return condError;
-  }
-
-  //Check if we have same codec already applied
-  if(CheckCodecsForMatch(mCurSendCodecConfig, codecConfig))
-  {
-    CSFLogDebug(logTag,  "%s Codec has been applied already ", __FUNCTION__);
+    //validate basic params
+    if((condError = ValidateCodecConfig(codecConfig,true)) != kMediaConduitNoError)
+    {
+      return condError;
+    }
   }
 
   condError = StopTransmitting();
   if (condError != kMediaConduitNoError) {
     return condError;
   }
 
   if (mExternalSendCodec &&
@@ -662,20 +662,22 @@ WebrtcVideoConduit::ConfigureSendMediaCo
     }
   }
 
   condError = StartTransmitting();
   if (condError != kMediaConduitNoError) {
     return condError;
   }
 
-  //Copy the applied config for future reference.
-  delete mCurSendCodecConfig;
+  {
+    MutexAutoLock lock(mCodecMutex);
 
-  mCurSendCodecConfig = new VideoCodecConfig(*codecConfig);
+    //Copy the applied config for future reference.
+    mCurSendCodecConfig = new VideoCodecConfig(*codecConfig);
+  }
 
   mPtrRTP->SetRembStatus(mChannel, true, false);
 
   return kMediaConduitNoError;
 }
 
 MediaConduitErrorCode
 WebrtcVideoConduit::ConfigureRecvMediaCodecs(
@@ -868,20 +870,22 @@ WebrtcVideoConduit::ConfigureRecvMediaCo
   // by now we should be successfully started the reception
   mPtrRTP->SetRembStatus(mChannel, false, true);
   DumpCodecDB();
   return kMediaConduitNoError;
 }
 
 // XXX we need to figure out how to feed back changes in preferred capture
 // resolution to the getUserMedia source
+// Invoked under lock of mCodecMutex!
 bool
 WebrtcVideoConduit::SelectSendResolution(unsigned short width,
                                          unsigned short height)
 {
+  mCodecMutex.AssertCurrentThreadOwns();
   // XXX This will do bandwidth-resolution adaptation as well - bug 877954
 
   // Limit resolution to max-fs while keeping same aspect ratio as the
   // incoming image.
   if (mCurSendCodecConfig && mCurSendCodecConfig->mMaxFrameSize)
   {
     unsigned int cur_fs, max_width, max_height, mb_width, mb_height, mb_max;
 
@@ -941,99 +945,93 @@ WebrtcVideoConduit::SelectSendResolution
 
     // Favor even multiples of pixels for width and height.
     width = std::max(width & ~1, 2);
     height = std::max(height & ~1, 2);
   }
 
   // Adapt to getUserMedia resolution changes
   // check if we need to reconfigure the sending resolution
+  bool changed = false;
   if (mSendingWidth != width || mSendingHeight != height)
   {
     // This will avoid us continually retrying this operation if it fails.
     // If the resolution changes, we'll try again.  In the meantime, we'll
     // keep using the old size in the encoder.
     mSendingWidth = width;
     mSendingHeight = height;
+    changed = true;
+  }
 
+  // uses mSendingWidth/Height
+  unsigned int framerate = SelectSendFrameRate(mSendingFramerate);
+  if (mSendingFramerate != framerate) {
+    mSendingFramerate = framerate;
+    changed = true;
+  }
+
+  if (changed) {
     // Get current vie codec.
     webrtc::VideoCodec vie_codec;
     int32_t err;
 
     if ((err = mPtrViECodec->GetSendCodec(mChannel, vie_codec)) != 0)
     {
       CSFLogError(logTag, "%s: GetSendCodec failed, err %d", __FUNCTION__, err);
       return false;
     }
-    if (vie_codec.width != width || vie_codec.height != height)
+    // Likely spurious unless there was some error, but rarely checked
+    if (vie_codec.width != width || vie_codec.height != height ||
+        vie_codec.maxFramerate != mSendingFramerate)
     {
       vie_codec.width = width;
       vie_codec.height = height;
+      vie_codec.maxFramerate = mSendingFramerate;
 
       if ((err = mPtrViECodec->SetSendCodec(mChannel, vie_codec)) != 0)
       {
         CSFLogError(logTag, "%s: SetSendCodec(%ux%u) failed, err %d",
                     __FUNCTION__, width, height, err);
         return false;
       }
-      CSFLogDebug(logTag, "%s: Encoder resolution changed to %ux%u",
-                  __FUNCTION__, width, height);
+      CSFLogDebug(logTag, "%s: Encoder resolution changed to %ux%u @ %ufps, bitrate %u:%u",
+                  __FUNCTION__, width, height, mSendingFramerate,
+                   vie_codec.minBitrate, vie_codec.maxBitrate);
     } // else no change; mSendingWidth likely was 0
   }
   return true;
 }
-// TODO(ruil2@cisco.com):combine SelectSendResolution with SelectSendFrameRate Bug 1132318
-bool
-WebrtcVideoConduit::SelectSendFrameRate(unsigned int framerate)
+
+// Invoked under lock of mCodecMutex!
+unsigned int
+WebrtcVideoConduit::SelectSendFrameRate(unsigned int framerate) const
 {
+  mCodecMutex.AssertCurrentThreadOwns();
+  unsigned int new_framerate = framerate;
+
   // Limit frame rate based on max-mbps
-  mSendingFramerate = framerate;
   if (mCurSendCodecConfig && mCurSendCodecConfig->mMaxMBPS)
   {
     unsigned int cur_fs, mb_width, mb_height, max_fps;
 
     mb_width = (mSendingWidth + 15) >> 4;
     mb_height = (mSendingHeight + 15) >> 4;
 
     cur_fs = mb_width * mb_height;
     max_fps = mCurSendCodecConfig->mMaxMBPS/cur_fs;
     if (max_fps < mSendingFramerate) {
-      mSendingFramerate = max_fps;
+      new_framerate = max_fps;
     }
 
     if (mCurSendCodecConfig->mMaxFrameRate != 0 &&
       mCurSendCodecConfig->mMaxFrameRate < mSendingFramerate) {
-      mSendingFramerate = mCurSendCodecConfig->mMaxFrameRate;
+      new_framerate = mCurSendCodecConfig->mMaxFrameRate;
     }
   }
-  if (mSendingFramerate != framerate)
-  {
-    // Get current vie codec.
-    webrtc::VideoCodec vie_codec;
-    int32_t err;
-
-    if ((err = mPtrViECodec->GetSendCodec(mChannel, vie_codec)) != 0)
-    {
-      CSFLogError(logTag, "%s: GetSendCodec failed, err %d", __FUNCTION__, err);
-      return false;
-    }
-    if (vie_codec.maxFramerate != mSendingFramerate)
-    {
-      vie_codec.maxFramerate = mSendingFramerate;
-      if ((err = mPtrViECodec->SetSendCodec(mChannel, vie_codec)) != 0)
-      {
-        CSFLogError(logTag, "%s: SetSendCodec(%u) failed, err %d",
-                       __FUNCTION__, mSendingFramerate, err);
-        return false;
-      }
-      CSFLogDebug(logTag, "%s: Encoder framerate changed to %u",
-       __FUNCTION__, mSendingFramerate);
-    }
-  }
-  return true;
+  return new_framerate;
 }
 
 MediaConduitErrorCode
 WebrtcVideoConduit::SetExternalSendCodec(VideoCodecConfig* config,
                                          VideoEncoder* encoder) {
   if (!mPtrExtCodec->RegisterExternalSendCodec(mChannel,
                                               config->mType,
                                               static_cast<WebrtcVideoEncoder*>(encoder),
@@ -1088,23 +1086,22 @@ WebrtcVideoConduit::SendVideoFrame(unsig
 
   //Transmission should be enabled before we insert any frames.
   if(!mEngineTransmitting)
   {
     CSFLogError(logTag, "%s Engine not transmitting ", __FUNCTION__);
     return kMediaConduitSessionNotInited;
   }
 
-  if (!SelectSendResolution(width, height))
   {
-    return kMediaConduitCaptureError;
-  }
-  if (!SelectSendFrameRate(mSendingFramerate))
-  {
-    return kMediaConduitCaptureError;
+    MutexAutoLock lock(mCodecMutex);
+    if (!SelectSendResolution(width, height))
+    {
+      return kMediaConduitCaptureError;
+    }
   }
   //insert the frame to video engine in I420 format only
   MOZ_ASSERT(mPtrExtCapture);
   if(mPtrExtCapture->IncomingFrame(video_frame,
                                    video_frame_length,
                                    width, height,
                                    type,
                                    (unsigned long long)capture_time) == -1)
@@ -1476,17 +1473,17 @@ WebrtcVideoConduit::CheckCodecForMatch(c
 }
 
 /**
  * Perform validation on the codecConfig to be applied
  * Verifies if the codec is already applied.
  */
 MediaConduitErrorCode
 WebrtcVideoConduit::ValidateCodecConfig(const VideoCodecConfig* codecInfo,
-                                        bool send) const
+                                        bool send)
 {
   bool codecAppliedAlready = false;
 
   if(!codecInfo)
   {
     CSFLogError(logTag, "%s Null CodecConfig ", __FUNCTION__);
     return kMediaConduitMalformedArgument;
   }
@@ -1496,16 +1493,18 @@ WebrtcVideoConduit::ValidateCodecConfig(
   {
     CSFLogError(logTag, "%s Invalid Payload Name Length ", __FUNCTION__);
     return kMediaConduitMalformedArgument;
   }
 
   //check if we have the same codec already applied
   if(send)
   {
+    MutexAutoLock lock(mCodecMutex);
+
     codecAppliedAlready = CheckCodecsForMatch(mCurSendCodecConfig,codecInfo);
   } else {
     codecAppliedAlready = CheckCodecForMatch(codecInfo);
   }
 
   if(codecAppliedAlready)
   {
     CSFLogDebug(logTag, "%s Codec %s Already Applied  ", __FUNCTION__, codecInfo->mName.c_str());
--- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h
+++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h
@@ -140,19 +140,20 @@ public:
    * @param width, height: dimensions of the frame
    */
   bool SelectSendResolution(unsigned short width,
                             unsigned short height);
 
   /**
    * Function to select and change the encoding frame rate based on incoming frame rate
    * and max-mbps setting.
-   * @param framerate
+   * @param current framerate
+   * @result new framerate
    */
-  bool SelectSendFrameRate(unsigned int framerate);
+  unsigned int SelectSendFrameRate(unsigned int framerate) const;
 
   /**
    * Function to deliver a capture video frame for encoding and transport
    * @param video_frame: pointer to captured video-frame.
    * @param video_frame_length: size of the frame
    * @param width, height: dimensions of the frame
    * @param video_type: Type of the video frame - I420, RAW
    * @param captured_time: timestamp when the frame was captured.
@@ -295,17 +296,17 @@ private:
 
   // Functions to verify if the codec passed is already in
   // conduits database
   bool CheckCodecForMatch(const VideoCodecConfig* codecInfo) const;
   bool CheckCodecsForMatch(const VideoCodecConfig* curCodecConfig,
                            const VideoCodecConfig* codecInfo) const;
 
   //Checks the codec to be applied
-  MediaConduitErrorCode ValidateCodecConfig(const VideoCodecConfig* codecInfo, bool send) const;
+  MediaConduitErrorCode ValidateCodecConfig(const VideoCodecConfig* codecInfo, bool send);
 
   //Utility function to dump recv codec database
   void DumpCodecDB() const;
 
   // Video Latency Test averaging filter
   void VideoLatencyUpdate(uint64_t new_sample);
 
   webrtc::VideoEngine* mVideoEngine;
@@ -326,17 +327,20 @@ private:
 
   // Engine state we are concerned with.
   mozilla::Atomic<bool> mEngineTransmitting; //If true ==> Transmit Sub-system is up and running
   mozilla::Atomic<bool> mEngineReceiving;    // if true ==> Receive Sus-sysmtem up and running
 
   int mChannel; // Video Channel for this conduit
   int mCapId;   // Capturer for this conduit
   RecvCodecList    mRecvCodecList;
-  VideoCodecConfig* mCurSendCodecConfig;
+
+  Mutex mCodecMutex; // protects mCurrSendCodecConfig
+  nsAutoPtr<VideoCodecConfig> mCurSendCodecConfig;
+
   unsigned short mSendingWidth;
   unsigned short mSendingHeight;
   unsigned short mReceivingWidth;
   unsigned short mReceivingHeight;
   unsigned int   mSendingFramerate;
   unsigned short mNumReceivingStreams;
   bool mVideoLatencyTestEnable;
   uint64_t mVideoLatencyAvg;