From b11b15ef827e1cfc624e49e541c113e64eeb0d20 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Fri, 5 Jun 2015 20:27:38 -0400 Subject: [PATCH] Bug 1132318: merge SelectSendFrameRate with SelectSendResolution r=bwc --- .../src/media-conduit/AudioConduit.cpp | 32 ++--- .../src/media-conduit/AudioConduit.h | 8 +- .../src/media-conduit/VideoConduit.cpp | 119 +++++++++--------- .../src/media-conduit/VideoConduit.h | 15 ++- 4 files changed, 90 insertions(+), 84 deletions(-) diff --git a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp index ca86a85dee86..c39b9803ec18 100755 --- a/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp +++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.cpp @@ -69,7 +69,6 @@ WebrtcAudioConduit::~WebrtcAudioConduit() { delete mRecvCodecList[i]; } - delete mCurSendCodecConfig; // The first one of a pair to be deleted shuts down media for both if(mPtrVoEXmedia) @@ -360,10 +359,12 @@ WebrtcAudioConduit::ConfigureSendMediaCodec(const AudioCodecConfig* codecConfig) 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(); @@ -411,16 +412,17 @@ WebrtcAudioConduit::ConfigureSendMediaCodec(const AudioCodecConfig* codecConfig) return condError; } - //Copy the applied config for future reference. - delete mCurSendCodecConfig; - - mCurSendCodecConfig = new AudioCodecConfig(codecConfig->mType, - codecConfig->mName, - codecConfig->mFreq, - codecConfig->mPacSize, - codecConfig->mChannels, - codecConfig->mRate); + { + MutexAutoLock lock(mCodecMutex); + //Copy the applied config for future reference. + mCurSendCodecConfig = new AudioCodecConfig(codecConfig->mType, + codecConfig->mName, + codecConfig->mFreq, + codecConfig->mPacSize, + codecConfig->mChannels, + codecConfig->mRate); + } return kMediaConduitNoError; } @@ -1018,7 +1020,7 @@ WebrtcAudioConduit::CheckCodecForMatch(const AudioCodecConfig* codecInfo) const */ MediaConduitErrorCode WebrtcAudioConduit::ValidateCodecConfig(const AudioCodecConfig* codecInfo, - bool send) const + bool send) { bool codecAppliedAlready = false; @@ -1045,6 +1047,8 @@ WebrtcAudioConduit::ValidateCodecConfig(const AudioCodecConfig* codecInfo, //check if we have the same codec already applied if(send) { + MutexAutoLock lock(mCodecMutex); + codecAppliedAlready = CheckCodecsForMatch(mCurSendCodecConfig,codecInfo); } else { codecAppliedAlready = CheckCodecForMatch(codecInfo); diff --git a/media/webrtc/signaling/src/media-conduit/AudioConduit.h b/media/webrtc/signaling/src/media-conduit/AudioConduit.h index 507f3d7a67a9..faafc6bc0175 100755 --- a/media/webrtc/signaling/src/media-conduit/AudioConduit.h +++ b/media/webrtc/signaling/src/media-conduit/AudioConduit.h @@ -169,7 +169,7 @@ public: mEngineTransmitting(false), mEngineReceiving(false), mChannel(-1), - mCurSendCodecConfig(nullptr), + mCodecMutex("AudioConduit codec db"), mCaptureDelay(150), #if !defined(MOZILLA_EXTERNAL_LINKAGE) mLastTimestamp(0), @@ -245,7 +245,7 @@ private: 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; @@ -277,7 +277,9 @@ private: int mChannel; RecvCodecList mRecvCodecList; - AudioCodecConfig* mCurSendCodecConfig; + + Mutex mCodecMutex; // protects mCurSendCodecConfig + nsAutoPtr mCurSendCodecConfig; // Current "capture" delay (really output plus input delay) int32_t mCaptureDelay; diff --git a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp index a5782da5e3ed..3ca59d5ecb3a 100755 --- a/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp +++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp @@ -74,7 +74,7 @@ WebrtcVideoConduit::WebrtcVideoConduit(): mEngineReceiving(false), mChannel(-1), mCapId(-1), - mCurSendCodecConfig(nullptr), + mCodecMutex("VideoConduit codec db"), mSendingWidth(0), mSendingHeight(0), mReceivingWidth(640), @@ -101,8 +101,6 @@ WebrtcVideoConduit::~WebrtcVideoConduit() 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) @@ -236,8 +234,9 @@ bool WebrtcVideoConduit::GetVideoEncoderStats(double* framerateMean, // unchanged resolution, but adjust bandwidth limits to match camera fps CSFLogDebug(logTag, "Encoder frame rate changed from %f to %f", (mLastFramerateTenths/10.0), *framerateMean); + MutexAutoLock lock(mCodecMutex); mLastFramerateTenths = *framerateMean * 10; - SelectSendResolution(mSendingWidth, mSendingHeight, true); + SelectSendResolution(mSendingWidth, mSendingHeight); } return true; } @@ -595,6 +594,12 @@ WebrtcVideoConduit::ConfigureCodecMode(webrtc::VideoCodecMode mode) /** * 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) @@ -608,16 +613,12 @@ WebrtcVideoConduit::ConfigureSendMediaCodec(const VideoCodecConfig* codecConfig) 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(); @@ -709,10 +710,12 @@ WebrtcVideoConduit::ConfigureSendMediaCodec(const VideoCodecConfig* codecConfig) 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); @@ -997,11 +1000,12 @@ WebrtcVideoConduit::SelectBandwidth(webrtc::VideoCodec& vie_codec, // 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, - bool force) + 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 @@ -1071,15 +1075,25 @@ WebrtcVideoConduit::SelectSendResolution(unsigned short width, // Adapt to getUserMedia resolution changes // check if we need to reconfigure the sending resolution. - // force tells us to do it regardless, such as when the FPS changes - if (mSendingWidth != width || mSendingHeight != height || force) + 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; @@ -1089,10 +1103,13 @@ WebrtcVideoConduit::SelectSendResolution(unsigned short width, CSFLogError(logTag, "%s: GetSendCodec failed, err %d", __FUNCTION__, err); return false; } - if (vie_codec.width != width || vie_codec.height != height || force) + // 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; SelectBandwidth(vie_codec, width, height); if ((err = mPtrViECodec->SetSendCodec(mChannel, vie_codec)) != 0) @@ -1101,19 +1118,22 @@ WebrtcVideoConduit::SelectSendResolution(unsigned short width, __FUNCTION__, width, height, err); return false; } - CSFLogDebug(logTag, "%s: Encoder resolution changed to %ux%u, bitrate %u:%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; @@ -1124,39 +1144,15 @@ WebrtcVideoConduit::SelectSendFrameRate(unsigned int framerate) 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 @@ -1221,13 +1217,12 @@ WebrtcVideoConduit::SendVideoFrame(unsigned char* video_frame, return kMediaConduitSessionNotInited; } - if (!SelectSendResolution(width, height, false)) { - 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); @@ -1609,7 +1604,7 @@ WebrtcVideoConduit::CheckCodecForMatch(const VideoCodecConfig* codecInfo) const */ MediaConduitErrorCode WebrtcVideoConduit::ValidateCodecConfig(const VideoCodecConfig* codecInfo, - bool send) const + bool send) { bool codecAppliedAlready = false; @@ -1629,6 +1624,8 @@ WebrtcVideoConduit::ValidateCodecConfig(const VideoCodecConfig* codecInfo, //check if we have the same codec already applied if(send) { + MutexAutoLock lock(mCodecMutex); + codecAppliedAlready = CheckCodecsForMatch(mCurSendCodecConfig,codecInfo); } else { codecAppliedAlready = CheckCodecForMatch(codecInfo); diff --git a/media/webrtc/signaling/src/media-conduit/VideoConduit.h b/media/webrtc/signaling/src/media-conduit/VideoConduit.h index 68f5bb50bd7c..2a81aff2fd5c 100755 --- a/media/webrtc/signaling/src/media-conduit/VideoConduit.h +++ b/media/webrtc/signaling/src/media-conduit/VideoConduit.h @@ -145,15 +145,15 @@ public: * @param force: force setting the codec config if framerate may require a bandwidth change */ bool SelectSendResolution(unsigned short width, - unsigned short height, - bool force); + 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 @@ -306,7 +306,7 @@ private: 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; @@ -337,7 +337,10 @@ private: int mChannel; // Video Channel for this conduit int mCapId; // Capturer for this conduit RecvCodecList mRecvCodecList; - VideoCodecConfig* mCurSendCodecConfig; + + Mutex mCodecMutex; // protects mCurrSendCodecConfig + nsAutoPtr mCurSendCodecConfig; + unsigned short mSendingWidth; unsigned short mSendingHeight; unsigned short mReceivingWidth;