Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 26 Feb 2016 10:47:03 -0600
changeset 286148 2c4b0bfe69aaab7299a6edc814a04fa1c78a5254
parent 286147 95703fdc2175444a69dfb154f7d1bad74f19cf79
child 286149 d37744e8ec76bdb6507e73959cb8b46b03a89b1f
push id30039
push usercbook@mozilla.com
push dateTue, 01 Mar 2016 11:02:11 +0000
treeherdermozilla-central@5cafa6f3019b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs818618
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 818618: Honor (and emit) opus stereo fmtp param. r=jesup MozReview-Commit-ID: IgA305eiu1s
media/webrtc/signaling/src/jsep/JsepCodecDescription.h
media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
media/webrtc/signaling/src/sdp/SdpAttribute.h
media/webrtc/signaling/src/sdp/SipccSdpAttributeList.cpp
media/webrtc/signaling/test/jsep_track_unittest.cpp
media/webrtc/signaling/test/sdp_unittests.cpp
--- a/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
+++ b/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
@@ -126,17 +126,18 @@ class JsepAudioCodecDescription : public
                             uint32_t channels,
                             uint32_t packetSize,
                             uint32_t bitRate,
                             bool enabled = true)
       : JsepCodecDescription(mozilla::SdpMediaSection::kAudio, defaultPt, name,
                              clock, channels, enabled),
         mPacketSize(packetSize),
         mBitrate(bitRate),
-        mMaxPlaybackRate(0)
+        mMaxPlaybackRate(0),
+        mForceMono(false)
   {
   }
 
   JSEP_CODEC_CLONE(JsepAudioCodecDescription)
 
   SdpFmtpAttributeList::OpusParameters
   GetOpusParameters(const std::string& pt,
                     const SdpMediaSection& msection) const
@@ -155,42 +156,50 @@ class JsepAudioCodecDescription : public
 
   void
   AddParametersToMSection(SdpMediaSection& msection) const override
   {
     if (mDirection == sdp::kSend) {
       return;
     }
 
-    if (mName == "opus" && mMaxPlaybackRate) {
+    if (mName == "opus") {
       SdpFmtpAttributeList::OpusParameters opusParams(
           GetOpusParameters(mDefaultPt, msection));
-      opusParams.maxplaybackrate = mMaxPlaybackRate;
+      if (mMaxPlaybackRate) {
+        opusParams.maxplaybackrate = mMaxPlaybackRate;
+      }
+      if (mChannels == 2 && !mForceMono) {
+        // We prefer to receive stereo, if available.
+        opusParams.stereo = 1;
+      }
       msection.SetFmtp(SdpFmtpAttributeList::Fmtp(mDefaultPt, opusParams));
     }
   }
 
   bool
   Negotiate(const std::string& pt,
             const SdpMediaSection& remoteMsection) override
   {
     JsepCodecDescription::Negotiate(pt, remoteMsection);
     if (mName == "opus" && mDirection == sdp::kSend) {
       SdpFmtpAttributeList::OpusParameters opusParams(
           GetOpusParameters(mDefaultPt, remoteMsection));
 
       mMaxPlaybackRate = opusParams.maxplaybackrate;
+      mForceMono = !opusParams.stereo;
     }
 
     return true;
   }
 
   uint32_t mPacketSize;
   uint32_t mBitrate;
   uint32_t mMaxPlaybackRate;
+  bool mForceMono;
 };
 
 class JsepVideoCodecDescription : public JsepCodecDescription {
  public:
   JsepVideoCodecDescription(const std::string& defaultPt,
                             const std::string& name,
                             uint32_t clock,
                             bool enabled = true)
--- a/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
+++ b/media/webrtc/signaling/src/peerconnection/MediaPipelineFactory.cpp
@@ -67,17 +67,17 @@ JsepCodecDescToCodecConfig(const JsepCod
     MOZ_MTLOG(ML_ERROR, "Invalid payload type: " << desc.mDefaultPt);
     return NS_ERROR_INVALID_ARG;
   }
 
   *aConfig = new AudioCodecConfig(pt,
                                   desc.mName,
                                   desc.mClock,
                                   desc.mPacketSize,
-                                  desc.mChannels,
+                                  desc.mForceMono ? 1 : desc.mChannels,
                                   desc.mBitrate);
   (*aConfig)->mMaxPlaybackRate = desc.mMaxPlaybackRate;
 
   return NS_OK;
 }
 
 static std::vector<JsepCodecDescription*>
 GetCodecs(const JsepTrackNegotiatedDetails& aDetails)
--- a/media/webrtc/signaling/src/sdp/SdpAttribute.h
+++ b/media/webrtc/signaling/src/sdp/SdpAttribute.h
@@ -1234,35 +1234,39 @@ public:
 
     unsigned int max_fs;
     unsigned int max_fr;
   };
 
   class OpusParameters : public Parameters
   {
   public:
-    enum { kDefaultMaxPlaybackRate = 48000 };
+    enum { kDefaultMaxPlaybackRate = 48000,
+           kDefaultStereo = 0 };
     OpusParameters() :
       Parameters(SdpRtpmapAttributeList::kOpus),
-      maxplaybackrate(kDefaultMaxPlaybackRate)
+      maxplaybackrate(kDefaultMaxPlaybackRate),
+      stereo(kDefaultStereo)
     {}
 
     Parameters*
     Clone() const override
     {
       return new OpusParameters(*this);
     }
 
     void
     Serialize(std::ostream& os) const override
     {
-      os << "maxplaybackrate=" << maxplaybackrate;
+      os << "maxplaybackrate=" << maxplaybackrate << ";"
+         << "stereo=" << stereo;
     }
 
     unsigned int maxplaybackrate;
+    unsigned int stereo;
   };
 
   class Fmtp
   {
   public:
     Fmtp(const std::string& aFormat, UniquePtr<Parameters> aParameters)
         : format(aFormat),
           parameters(Move(aParameters))
--- a/media/webrtc/signaling/src/sdp/SipccSdpAttributeList.cpp
+++ b/media/webrtc/signaling/src/sdp/SipccSdpAttributeList.cpp
@@ -719,16 +719,17 @@ SipccSdpAttributeList::LoadFmtp(sdp_t* s
         vp8Parameters->max_fr = fmtp->max_fr;
 
         parameters.reset(vp8Parameters);
       } break;
       case RTP_OPUS: {
         SdpFmtpAttributeList::OpusParameters* opusParameters(
             new SdpFmtpAttributeList::OpusParameters);
         opusParameters->maxplaybackrate = fmtp->maxplaybackrate;
+        opusParameters->stereo = fmtp->stereo;
         parameters.reset(opusParameters);
       } break;
       default: {
       }
     }
 
     fmtps->PushEntry(osPayloadType.str(), Move(parameters));
   }
--- a/media/webrtc/signaling/test/jsep_track_unittest.cpp
+++ b/media/webrtc/signaling/test/jsep_track_unittest.cpp
@@ -476,48 +476,70 @@ TEST_F(JsepTrackTest, SimulcastAnswerer)
     if (codec->mName == "opus") {  \
       JsepAudioCodecDescription* audioCodec =  \
         static_cast<JsepAudioCodecDescription*>(codec);  \
       ASSERT_EQ((expectedRate), audioCodec->mMaxPlaybackRate);  \
     }  \
   };  \
 }
 
+#define VERIFY_OPUS_FORCE_MONO(track, expected) \
+{ \
+  JsepTrack& copy(track); \
+  ASSERT_TRUE(copy.GetNegotiatedDetails());  \
+  ASSERT_TRUE(copy.GetNegotiatedDetails()->GetEncodingCount());  \
+  for (auto codec : copy.GetNegotiatedDetails()->GetEncoding(0).GetCodecs()) {\
+    if (codec->mName == "opus") {  \
+      JsepAudioCodecDescription* audioCodec =  \
+        static_cast<JsepAudioCodecDescription*>(codec);  \
+      /* gtest has some compiler warnings when using ASSERT_EQ with booleans. */ \
+      ASSERT_EQ((int)(expected), (int)audioCodec->mForceMono);  \
+    }  \
+  };  \
+}
+
 TEST_F(JsepTrackTest, DefaultOpusParameters)
 {
   Init(SdpMediaSection::kAudio);
   OfferAnswer();
 
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mSendOff,
       SdpFmtpAttributeList::OpusParameters::kDefaultMaxPlaybackRate);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mSendAns,
       SdpFmtpAttributeList::OpusParameters::kDefaultMaxPlaybackRate);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mRecvOff, 0U);
+  VERIFY_OPUS_FORCE_MONO(*mRecvOff, false);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mRecvAns, 0U);
+  VERIFY_OPUS_FORCE_MONO(*mRecvAns, false);
 }
 
 TEST_F(JsepTrackTest, NonDefaultOpusParameters)
 {
   InitCodecs();
   for (auto& codec : mAnsCodecs.values) {
     if (codec->mName == "opus") {
       JsepAudioCodecDescription* audioCodec =
         static_cast<JsepAudioCodecDescription*>(codec);
       audioCodec->mMaxPlaybackRate = 16000;
+      audioCodec->mForceMono = true;
     }
   }
   InitTracks(SdpMediaSection::kAudio);
   InitSdp(SdpMediaSection::kAudio);
   OfferAnswer();
 
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mSendOff, 16000U);
+  VERIFY_OPUS_FORCE_MONO(*mSendOff, true);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mSendAns,
       SdpFmtpAttributeList::OpusParameters::kDefaultMaxPlaybackRate);
+  VERIFY_OPUS_FORCE_MONO(*mSendAns, false);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mRecvOff, 0U);
+  VERIFY_OPUS_FORCE_MONO(*mRecvOff, false);
   VERIFY_OPUS_MAX_PLAYBACK_RATE(*mRecvAns, 16000U);
+  VERIFY_OPUS_FORCE_MONO(*mRecvAns, true);
 }
 
 } // namespace mozilla
 
 int
 main(int argc, char** argv)
 {
   // Prevents some log spew
--- a/media/webrtc/signaling/test/sdp_unittests.cpp
+++ b/media/webrtc/signaling/test/sdp_unittests.cpp
@@ -1138,17 +1138,17 @@ const std::string kBasicAudioVideoOffer 
 "a=identity:blahblahblah foo;bar" CRLF
 "a=group:BUNDLE first second" CRLF
 "a=group:BUNDLE third" CRLF
 "a=group:LS first third" CRLF
 "m=audio 9 RTP/SAVPF 109 9 0 8 101" CRLF
 "c=IN IP4 0.0.0.0" CRLF
 "a=mid:first" CRLF
 "a=rtpmap:109 opus/48000/2" CRLF
-"a=fmtp:109 maxplaybackrate=32000" CRLF
+"a=fmtp:109 maxplaybackrate=32000;stereo=1" CRLF
 "a=ptime:20" CRLF
 "a=maxptime:20" CRLF
 "a=rtpmap:9 G722/8000" CRLF
 "a=rtpmap:0 PCMU/8000" CRLF
 "a=rtpmap:8 PCMA/8000" CRLF
 "a=rtpmap:101 telephone-event/8000" CRLF
 "a=fmtp:101 0-15" CRLF
 "a=ice-ufrag:00000000" CRLF
@@ -1500,17 +1500,17 @@ const std::string kH264AudioVideoOffer =
 "a=mid:first" CRLF
 "a=rtpmap:109 opus/48000/2" CRLF
 "a=ptime:20" CRLF
 "a=maxptime:20" CRLF
 "a=rtpmap:9 G722/8000" CRLF
 "a=rtpmap:0 PCMU/8000" CRLF
 "a=rtpmap:8 PCMA/8000" CRLF
 "a=rtpmap:101 telephone-event/8000" CRLF
-"a=fmtp:109 maxplaybackrate=32000" CRLF
+"a=fmtp:109 maxplaybackrate=32000;stereo=1" CRLF
 "a=ice-ufrag:00000000" CRLF
 "a=ice-pwd:0000000000000000000000000000000" CRLF
 "a=sendonly" CRLF
 "a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level" CRLF
 "a=setup:actpass" CRLF
 "a=rtcp-mux" CRLF
 "a=msid:stream track" CRLF
 "a=candidate:0 1 UDP 2130379007 10.0.0.36 62453 typ host" CRLF
@@ -1561,16 +1561,17 @@ TEST_P(NewSdpTest, CheckFormatParameters
       mSdp->GetMediaSection(0).GetAttributeList().GetFmtp().mFmtps;
   ASSERT_EQ(1U, audio_format_params.size());
   ASSERT_EQ("109", audio_format_params[0].format);
   ASSERT_TRUE(!!audio_format_params[0].parameters);
   const SdpFmtpAttributeList::OpusParameters* opus_parameters =
     static_cast<SdpFmtpAttributeList::OpusParameters*>(
         audio_format_params[0].parameters.get());
   ASSERT_EQ(32000U, opus_parameters->maxplaybackrate);
+  ASSERT_EQ(1U, opus_parameters->stereo);
 
   ASSERT_TRUE(mSdp->GetMediaSection(1).GetAttributeList().HasAttribute(
       SdpAttribute::kFmtpAttribute));
   auto video_format_params =
       mSdp->GetMediaSection(1).GetAttributeList().GetFmtp().mFmtps;
   ASSERT_EQ(3U, video_format_params.size());
   ASSERT_EQ("97", video_format_params[0].format);
   ASSERT_TRUE(!!video_format_params[0].parameters);