Bug 1406941 - Remove unused fields from JsepAudioCodecDescription; r=bwc
authorDan Minor <dminor@mozilla.com>
Mon, 19 Nov 2018 17:44:16 +0000
changeset 447034 d08645c751752d0601a660bf491959dc3e2efe1e
parent 447033 4a979a79984c0d57b4139c9d965f2da9cb5b585d
child 447035 c99a8cc72471893a46b15798439eba7549cf7300
push id35065
push userrmaries@mozilla.com
push dateMon, 19 Nov 2018 21:56:32 +0000
treeherdermozilla-central@bd4cebdbed4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1406941
milestone65.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 1406941 - Remove unused fields from JsepAudioCodecDescription; r=bwc Packet size and rate are no longer configured inside AudioConduit, so there is no reason to continue to define them here. We now take the defaults provided by webrtc.org. Depends on D12012 Differential Revision: https://phabricator.services.mozilla.com/D12013
media/webrtc/signaling/gtest/jsep_track_unittest.cpp
media/webrtc/signaling/src/jsep/JsepCodecDescription.h
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
--- a/media/webrtc/signaling/gtest/jsep_track_unittest.cpp
+++ b/media/webrtc/signaling/gtest/jsep_track_unittest.cpp
@@ -25,23 +25,23 @@ class JsepTrackTest : public ::testing::
 
     std::vector<UniquePtr<JsepCodecDescription>>
     MakeCodecs(bool addFecCodecs = false,
                bool preferRed = false,
                bool addDtmfCodec = false) const
     {
       std::vector<UniquePtr<JsepCodecDescription>> results;
       results.emplace_back(
-          new JsepAudioCodecDescription("1", "opus", 48000, 2, 960, 40000));
+          new JsepAudioCodecDescription("1", "opus", 48000, 2));
       results.emplace_back(
-          new JsepAudioCodecDescription("9", "G722", 8000, 1, 320, 64000));
+          new JsepAudioCodecDescription("9", "G722", 8000, 1));
       if (addDtmfCodec) {
         results.emplace_back(
             new JsepAudioCodecDescription("101", "telephone-event",
-                                          8000, 1, 0, 0));
+                                          8000, 1));
       }
 
       if (addFecCodecs && preferRed) {
         results.emplace_back(
             new JsepVideoCodecDescription("122", "red", 90000));
       }
 
       JsepVideoCodecDescription* vp8 =
--- a/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
+++ b/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
@@ -116,23 +116,19 @@ class JsepCodecDescription {
 };
 
 class JsepAudioCodecDescription : public JsepCodecDescription {
  public:
   JsepAudioCodecDescription(const std::string& defaultPt,
                             const std::string& name,
                             uint32_t clock,
                             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),
         mForceMono(false),
         mFECEnabled(false),
         mDtmfEnabled(false)
   {
   }
 
   JSEP_CODEC_CLONE(JsepAudioCodecDescription)
@@ -212,18 +208,16 @@ class JsepAudioCodecDescription : public
       // at the received side is declarative and can be negotiated
       // separately for either media direction.
       mFECEnabled = opusParams.useInBandFec;
     }
 
     return true;
   }
 
-  uint32_t mPacketSize;
-  uint32_t mBitrate;
   uint32_t mMaxPlaybackRate;
   bool mForceMono;
   bool mFECEnabled;
   bool mDtmfEnabled;
 };
 
 class JsepVideoCodecDescription : public JsepCodecDescription {
  public:
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -2079,60 +2079,43 @@ JsepSessionImpl::SetupDefaultCodecs()
   // If we reduce bitrate enough Opus will low-pass us; 16000 will kill a
   // 9KHz tone.  This should be adaptive when we're at the low-end of video
   // bandwidth (say <100Kbps), and if we're audio-only, down to 8 or
   // 12Kbps.
   mSupportedCodecs.emplace_back(new JsepAudioCodecDescription(
       "109",
       "opus",
       48000,
-      2,
-      960,
-      // TODO Move this elsewhere to be adaptive to rate - Bug 1207925
-      40000
-      ));
+      2));
 
   mSupportedCodecs.emplace_back(new JsepAudioCodecDescription(
       "9",
       "G722",
       8000,
-      1,
-      320,
-      64000));
+      1));
 
-  // packet size and bitrate values below copied from sipcc.
-  // May need reevaluation from a media expert.
   mSupportedCodecs.emplace_back(
       new JsepAudioCodecDescription("0",
                                     "PCMU",
                                     8000,
-                                    1,
-                                    8000 / 50,   // frequency / 50
-                                    8 * 8000 * 1 // 8 * frequency * channels
+                                    1
                                     ));
 
   mSupportedCodecs.emplace_back(
       new JsepAudioCodecDescription("8",
                                     "PCMA",
                                     8000,
-                                    1,
-                                    8000 / 50,   // frequency / 50
-                                    8 * 8000 * 1 // 8 * frequency * channels
+                                    1
                                     ));
 
-  // note: because telephone-event is effectively a marker codec that indicates
-  // that dtmf rtp packets may be passed, the packetSize and bitRate fields
-  // don't make sense here.  For now, use zero. (mjf)
   mSupportedCodecs.emplace_back(
       new JsepAudioCodecDescription("101",
                                     "telephone-event",
                                     8000,
-                                    1,
-                                    0, // packetSize doesn't make sense here
-                                    0  // bitRate doesn't make sense here
+                                    1
                                     ));
 
   // Supported video codecs.
   // Note: order here implies priority for building offers!
   UniquePtr<JsepVideoCodecDescription> vp8(new JsepVideoCodecDescription(
       "120",
       "VP8",
       90000