Bug 1212908 - Update a=simulcast to match new grammar in 03 draft. r=mt
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 20 Oct 2015 10:31:26 -0500
changeset 269613 32fb2eab97cd5af72959312f42f40810ff7c95a7
parent 269612 8e50c813fa42da9f6e44b4bc8f902641aa82df18
child 269614 ec5247a32592e3a0fd43b59f4109414cd11653c0
push id67149
push usercbook@mozilla.com
push dateTue, 27 Oct 2015 08:11:49 +0000
treeherdermozilla-inbound@ec5247a32592 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssimulcast, mt
bugs1212908
milestone44.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 1212908 - Update a=simulcast to match new grammar in 03 draft. r=mt
media/webrtc/signaling/src/sdp/SdpAttribute.cpp
media/webrtc/signaling/src/sdp/SdpAttribute.h
media/webrtc/signaling/test/sdp_unittests.cpp
--- a/media/webrtc/signaling/src/sdp/SdpAttribute.cpp
+++ b/media/webrtc/signaling/src/sdp/SdpAttribute.cpp
@@ -480,17 +480,16 @@ SdpImageattrAttributeList::PRange::Seria
   os << std::setprecision(4) << std::fixed;
   os << "[" << min << "-" << max << "]";
 }
 
 static std::string ParseToken(std::istream& is,
                               const std::string& delims,
                               std::string* error)
 {
-  is >> std::ws;
   std::string token;
   while (is) {
     unsigned char c = PeekChar(is, error);
     if (!c || (delims.find(c) != std::string::npos)) {
       break;
     }
     token.push_back(std::tolower(is.get()));
   }
@@ -841,16 +840,17 @@ bool
 SdpRidAttributeList::Constraints::Parse(std::istream& is, std::string* error)
 {
   if (!PeekChar(is, error)) {
     // No constraints
     return true;
   }
 
   do {
+    is >> std::ws;
     std::string key = ParseKey(is, error);
     if (key.empty()) {
       return false; // Illegal trailing cruft
     }
 
     // This allows pt= to appear anywhere, instead of only at the beginning, but
     // this ends up being significantly less code.
     if (key == "pt") {
@@ -979,16 +979,17 @@ SdpRidAttributeList::Constraints::Serial
 bool
 SdpRidAttributeList::Rid::Parse(std::istream& is, std::string* error)
 {
   id = ParseToken(is, " ", error);
   if (id.empty()) {
     return false;
   }
 
+  is >> std::ws;
   std::string directionToken = ParseToken(is, " ", error);
   if (directionToken == "send") {
     direction = sdp::kSend;
   } else if (directionToken == "recv") {
     direction = sdp::kRecv;
   } else {
     *error = "Invalid direction, must be either send or recv";
     return false;
@@ -1114,120 +1115,138 @@ SdpSetupAttribute::Serialize(std::ostrea
 {
   os << "a=" << mType << ":" << mRole << CRLF;
 }
 
 void
 SdpSimulcastAttribute::Version::Serialize(std::ostream& os) const
 {
   SkipFirstDelimiter comma(",");
-  for (uint16_t format : choices) {
-    os << comma << format;
+  for (const std::string& choice : choices) {
+    os << comma << choice;
   }
 }
 
 bool
 SdpSimulcastAttribute::Version::Parse(std::istream& is, std::string* error)
 {
   do {
-    uint16_t value;
-    if (!GetUnsigned<uint16_t>(is, 0, UINT16_MAX, &value, error)) {
+    std::string value = ParseToken(is, ",; ", error);
+    if (value.empty()) {
       return false;
     }
     choices.push_back(value);
   } while (SkipChar(is, ',', error));
 
   return true;
 }
 
-void
-SdpSimulcastAttribute::Version::AppendAsStrings(
-    std::vector<std::string>* formats) const
+bool
+SdpSimulcastAttribute::Version::GetChoicesAsFormats(
+    std::vector<uint16_t>* formats) const
 {
-  for (uint16_t pt : choices) {
-    std::ostringstream os;
-    os << pt;
-    formats->push_back(os.str());
-  }
-}
-
-void
-SdpSimulcastAttribute::Version::AddChoice(const std::string& pt)
-{
-  uint16_t ptAsInt;
-  if (!SdpHelper::GetPtAsInt(pt, &ptAsInt)) {
-    MOZ_ASSERT(false);
-    return;
+  for (const std::string& choice : choices) {
+    uint16_t format;
+    if (!SdpHelper::GetPtAsInt(choice, &format) || (format > 127)) {
+      return false;
+    }
+    formats->push_back(format);
   }
 
-  choices.push_back(ptAsInt);
+  return true;
 }
 
 void
 SdpSimulcastAttribute::Versions::Serialize(std::ostream& os) const
 {
+  switch (type) {
+    case kRid:
+      os << "rid=";
+      break;
+    case kPt:
+      os << "pt=";
+      break;
+  }
+
   SkipFirstDelimiter semic(";");
   for (const Version& version : *this) {
     if (!version.IsSet()) {
       continue;
     }
     os << semic;
     version.Serialize(os);
   }
 }
 
 bool
 SdpSimulcastAttribute::Versions::Parse(std::istream& is, std::string* error)
 {
+  std::string rawType = ParseKey(is, error);
+  if (rawType.empty()) {
+    return false;
+  }
+
+  if (rawType == "pt") {
+    type = kPt;
+  } else if (rawType == "rid") {
+    type = kRid;
+  } else {
+    *error = "Unknown simulcast identification type ";
+    error->append(rawType);
+    return false;
+  }
+
   do {
     Version version;
     if (!version.Parse(is, error)) {
       return false;
     }
+
+    if (type == kPt) {
+      std::vector<uint16_t> formats;
+      if (!version.GetChoicesAsFormats(&formats)) {
+        *error = "Invalid payload type";
+        return false;
+      }
+    }
+
     push_back(version);
   } while(SkipChar(is, ';', error));
 
   return true;
 }
 
 void
 SdpSimulcastAttribute::Serialize(std::ostream& os) const
 {
-  MOZ_ASSERT(sendVersions.IsSet() ||
-             recvVersions.IsSet() ||
-             sendrecvVersions.IsSet());
+  MOZ_ASSERT(sendVersions.IsSet() || recvVersions.IsSet());
 
   os << "a=" << mType << ":";
 
   if (sendVersions.IsSet()) {
     os << " send ";
     sendVersions.Serialize(os);
   }
 
   if (recvVersions.IsSet()) {
     os << " recv ";
     recvVersions.Serialize(os);
   }
 
-  if (sendrecvVersions.IsSet()) {
-    os << " sendrecv ";
-    sendrecvVersions.Serialize(os);
-  }
-
   os << CRLF;
 }
 
 bool
 SdpSimulcastAttribute::Parse(std::istream& is, std::string* error)
 {
   bool gotRecv = false;
   bool gotSend = false;
-  bool gotSendrecv = false;
 
   while (true) {
+    is >> std::ws;
     std::string token = ParseToken(is, " \t", error);
     if (token.empty()) {
       break;
     }
 
     if (token == "send") {
       if (gotSend) {
         *error = "Already got a send list";
@@ -1245,34 +1264,23 @@ SdpSimulcastAttribute::Parse(std::istrea
         return false;
       }
       gotRecv = true;
 
       is >> std::ws;
       if (!recvVersions.Parse(is, error)) {
         return false;
       }
-    } else if (token == "sendrecv") {
-      if (gotSendrecv) {
-        *error = "Already got a sendrecv list";
-        return false;
-      }
-      gotSendrecv = true;
-
-      is >> std::ws;
-      if (!sendrecvVersions.Parse(is, error)) {
-        return false;
-      }
     } else {
-      *error = "Type must be either 'send', 'recv', or 'sendrecv'";
+      *error = "Type must be either 'send' or 'recv'";
       return false;
     }
   }
 
-  if (!gotSend && !gotRecv && !gotSendrecv) {
+  if (!gotSend && !gotRecv) {
     *error = "Empty simulcast attribute";
     return false;
   }
 
   return true;
 }
 
 void
--- a/media/webrtc/signaling/src/sdp/SdpAttribute.h
+++ b/media/webrtc/signaling/src/sdp/SdpAttribute.h
@@ -1405,26 +1405,26 @@ inline std::ostream& operator<<(std::ost
       break;
     default:
       MOZ_ASSERT(false);
       os << "?";
   }
   return os;
 }
 
-// Note: This ABNF is buggy since it does not include a ':' after "a=simulcast"
-// The authors have said this will be fixed in a subsequent version.
-// TODO(bug 1191986): Update this ABNF once the new version is out.
-// simulcast-attribute = "a=simulcast" 1*3( WSP sc-dir-list )
-// sc-dir-list         = sc-dir WSP sc-fmt-list *( ";" sc-fmt-list )
-// sc-dir              = "send" / "recv" / "sendrecv"
-// sc-fmt-list         = sc-fmt *( "," sc-fmt )
-// sc-fmt              = fmt
+// sc-attr     = "a=simulcast:" 1*2( WSP sc-str-list ) [WSP sc-pause-list]
+// sc-str-list = sc-dir WSP sc-id-type "=" sc-alt-list *( ";" sc-alt-list )
+// sc-pause-list = "paused=" sc-alt-list
+// sc-dir      = "send" / "recv"
+// sc-id-type  = "pt" / "rid" / token
+// sc-alt-list = sc-id *( "," sc-id )
+// sc-id       = fmt / rid-identifier / token
 // ; WSP defined in [RFC5234]
-// ; fmt defined in [RFC4566]
+// ; fmt, token defined in [RFC4566]
+// ; rid-identifier defined in [I-D.pthatcher-mmusic-rid]
 class SdpSimulcastAttribute : public SdpAttribute
 {
 public:
   SdpSimulcastAttribute() : SdpAttribute(kSimulcastAttribute) {}
 
   void Serialize(std::ostream& os) const override;
   bool Parse(std::istream& is, std::string* error);
 
@@ -1432,46 +1432,52 @@ public:
   {
     public:
       void Serialize(std::ostream& os) const;
       bool IsSet() const
       {
         return !choices.empty();
       }
       bool Parse(std::istream& is, std::string* error);
-      void AppendAsStrings(std::vector<std::string>* formats) const;
-      void AddChoice(const std::string& pt);
+      bool GetChoicesAsFormats(std::vector<uint16_t>* formats) const;
 
-      std::vector<uint16_t> choices;
+      std::vector<std::string> choices;
   };
 
   class Versions : public std::vector<Version>
   {
     public:
+      enum Type {
+        kPt,
+        kRid
+      };
+
+      Versions() : type(kRid) {}
       void Serialize(std::ostream& os) const;
       bool IsSet() const
       {
         if (empty()) {
           return false;
         }
 
         for (const Version& version : *this) {
           if (version.IsSet()) {
             return true;
           }
         }
 
         return false;
       }
+
       bool Parse(std::istream& is, std::string* error);
+      Type type;
   };
 
   Versions sendVersions;
   Versions recvVersions;
-  Versions sendrecvVersions;
 };
 
 ///////////////////////////////////////////////////////////////////////////
 // a=ssrc, RFC5576
 //-------------------------------------------------------------------------
 // ssrc-attr = "ssrc:" ssrc-id SP attribute
 // ; The base definition of "attribute" is in RFC 4566.
 // ; (It is the content of "a=" lines.)
--- a/media/webrtc/signaling/test/sdp_unittests.cpp
+++ b/media/webrtc/signaling/test/sdp_unittests.cpp
@@ -1190,17 +1190,17 @@ const std::string kBasicAudioVideoOffer 
 "a=candidate:3 1 UDP 100401151 162.222.183.171 62935 typ relay raddr 162.222.183.171 rport 62935" CRLF
 "a=candidate:3 2 UDP 100401150 162.222.183.171 61026 typ relay raddr 162.222.183.171 rport 61026" CRLF
 "a=rtcp:61026" CRLF
 "a=end-of-candidates" CRLF
 "a=ssrc:1111 foo" CRLF
 "a=ssrc:1111 foo:bar" CRLF
 "a=imageattr:120 send * recv *" CRLF
 "a=imageattr:121 send [x=640,y=480] recv [x=640,y=480]" CRLF
-"a=simulcast:sendrecv 120;121" CRLF
+"a=simulcast:send pt=120;121" CRLF
 "a=rid:foo send" CRLF
 "a=rid:bar recv pt=96;max-width=800;max-height=600" CRLF
 "m=audio 9 RTP/SAVPF 0" CRLF
 "a=mid:third" CRLF
 "a=rtpmap:0 PCMU/8000" CRLF
 "a=ice-lite" CRLF
 "a=ice-options:foo bar" CRLF
 "a=msid:noappdata" CRLF
@@ -2094,23 +2094,24 @@ TEST_P(NewSdpTest, CheckSimulcast)
   ASSERT_TRUE(mSdp->GetMediaSection(1).GetAttributeList().HasAttribute(
         SdpAttribute::kSimulcastAttribute));
   ASSERT_FALSE(mSdp->GetMediaSection(2).GetAttributeList().HasAttribute(
         SdpAttribute::kSimulcastAttribute));
 
   const SdpSimulcastAttribute& simulcast =
     mSdp->GetMediaSection(1).GetAttributeList().GetSimulcast();
 
-  ASSERT_EQ(0U, simulcast.sendVersions.size());
   ASSERT_EQ(0U, simulcast.recvVersions.size());
-  ASSERT_EQ(2U, simulcast.sendrecvVersions.size());
-  ASSERT_EQ(1U, simulcast.sendrecvVersions[0].choices.size());
-  ASSERT_EQ(120U, simulcast.sendrecvVersions[0].choices[0]);
-  ASSERT_EQ(1U, simulcast.sendrecvVersions[1].choices.size());
-  ASSERT_EQ(121U, simulcast.sendrecvVersions[1].choices[0]);
+  ASSERT_EQ(2U, simulcast.sendVersions.size());
+  ASSERT_EQ(1U, simulcast.sendVersions[0].choices.size());
+  ASSERT_EQ("120", simulcast.sendVersions[0].choices[0]);
+  ASSERT_EQ(1U, simulcast.sendVersions[1].choices.size());
+  ASSERT_EQ("121", simulcast.sendVersions[1].choices[0]);
+  ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+            simulcast.sendVersions.type);
 }
 
 TEST_P(NewSdpTest, CheckSctpmap) {
   ParseSdp(kBasicAudioVideoDataOffer);
   ASSERT_TRUE(!!mSdp) << "Parse failed: " << GetParseErrors();
   ASSERT_EQ(3U, mSdp->GetMediaSectionCount())
     << "Wrong number of media sections";
 
@@ -3413,27 +3414,27 @@ TEST(NewSdpTestNoFixture, CheckImageattr
   os.str("");
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionSerialize)
 {
   std::ostringstream os;
 
   SdpSimulcastAttribute::Version version;
-  version.choices.push_back(8U);
+  version.choices.push_back("8");
   version.Serialize(os);
   ASSERT_EQ("8", os.str());
   os.str("");
 
-  version.choices.push_back(9U);
+  version.choices.push_back("9");
   version.Serialize(os);
   ASSERT_EQ("8,9", os.str());
   os.str("");
 
-  version.choices.push_back(0U);
+  version.choices.push_back("0");
   version.Serialize(os);
   ASSERT_EQ("8,9,0", os.str());
   os.str("");
 }
 
 static SdpSimulcastAttribute::Version
 ParseSimulcastVersion(const std::string& input)
 {
@@ -3447,67 +3448,71 @@ ParseSimulcastVersion(const std::string&
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionValidParse)
 {
   {
     SdpSimulcastAttribute::Version version(
         ParseSimulcastVersion("1"));
     ASSERT_EQ(1U, version.choices.size());
-    ASSERT_EQ(1U, version.choices[0]);
+    ASSERT_EQ("1", version.choices[0]);
   }
 
   {
     SdpSimulcastAttribute::Version version(
         ParseSimulcastVersion("1,2"));
     ASSERT_EQ(2U, version.choices.size());
-    ASSERT_EQ(1U, version.choices[0]);
-    ASSERT_EQ(2U, version.choices[1]);
+    ASSERT_EQ("1", version.choices[0]);
+    ASSERT_EQ("2", version.choices[1]);
   }
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionInvalidParse)
 {
   ParseInvalid<SdpSimulcastAttribute::Version>("", 0);
-  ParseInvalid<SdpSimulcastAttribute::Version>("x", 0);
-  ParseInvalid<SdpSimulcastAttribute::Version>("*", 0);
+  ParseInvalid<SdpSimulcastAttribute::Version>(",", 0);
+  ParseInvalid<SdpSimulcastAttribute::Version>(";", 0);
+  ParseInvalid<SdpSimulcastAttribute::Version>(" ", 0);
   ParseInvalid<SdpSimulcastAttribute::Version>("8,", 2);
-  ParseInvalid<SdpSimulcastAttribute::Version>("8,x", 2);
-  ParseInvalid<SdpSimulcastAttribute::Version>("8,*", 2);
-  ParseInvalid<SdpSimulcastAttribute::Version>("-1", 0);
-  ParseInvalid<SdpSimulcastAttribute::Version>("8,-1", 2);
-  ParseInvalid<SdpSimulcastAttribute::Version>("99999", 5);
-  ParseInvalid<SdpSimulcastAttribute::Version>("8,99999", 7);
+  ParseInvalid<SdpSimulcastAttribute::Version>("8, ", 2);
+  ParseInvalid<SdpSimulcastAttribute::Version>("8,,", 2);
+  ParseInvalid<SdpSimulcastAttribute::Version>("8,;", 2);
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionsSerialize)
 {
   std::ostringstream os;
 
   SdpSimulcastAttribute::Versions versions;
+  versions.type = SdpSimulcastAttribute::Versions::kPt;
   versions.push_back(SdpSimulcastAttribute::Version());
-  versions.back().choices.push_back(8U);
+  versions.back().choices.push_back("8");
   versions.Serialize(os);
-  ASSERT_EQ("8", os.str());
+  ASSERT_EQ("pt=8", os.str());
+  os.str("");
+
+  versions.type = SdpSimulcastAttribute::Versions::kRid;
+  versions.Serialize(os);
+  ASSERT_EQ("rid=8", os.str());
   os.str("");
 
   versions.push_back(SdpSimulcastAttribute::Version());
   versions.Serialize(os);
-  ASSERT_EQ("8", os.str());
+  ASSERT_EQ("rid=8", os.str());
   os.str("");
 
-  versions.back().choices.push_back(9U);
+  versions.back().choices.push_back("9");
   versions.Serialize(os);
-  ASSERT_EQ("8;9", os.str());
+  ASSERT_EQ("rid=8;9", os.str());
   os.str("");
 
   versions.push_back(SdpSimulcastAttribute::Version());
-  versions.back().choices.push_back(0U);
+  versions.back().choices.push_back("0");
   versions.Serialize(os);
-  ASSERT_EQ("8;9;0", os.str());
+  ASSERT_EQ("rid=8;9;0", os.str());
   os.str("");
 }
 
 static SdpSimulcastAttribute::Versions
 ParseSimulcastVersions(const std::string& input)
 {
   std::istringstream is(input + " ");
   std::string error;
@@ -3517,74 +3522,85 @@ ParseSimulcastVersions(const std::string
   EXPECT_EQ(EOF, is.get());
   return list;
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionsValidParse)
 {
   {
     SdpSimulcastAttribute::Versions versions(
-        ParseSimulcastVersions("8"));
+        ParseSimulcastVersions("pt=8"));
     ASSERT_EQ(1U, versions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt, versions.type);
     ASSERT_EQ(1U, versions[0].choices.size());
-    ASSERT_EQ(8U, versions[0].choices[0]);
+    ASSERT_EQ("8", versions[0].choices[0]);
   }
 
   {
     SdpSimulcastAttribute::Versions versions(
-        ParseSimulcastVersions("8,9"));
+        ParseSimulcastVersions("rid=8"));
     ASSERT_EQ(1U, versions.size());
-    ASSERT_EQ(2U, versions[0].choices.size());
-    ASSERT_EQ(8U, versions[0].choices[0]);
-    ASSERT_EQ(9U, versions[0].choices[1]);
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kRid, versions.type);
+    ASSERT_EQ(1U, versions[0].choices.size());
+    ASSERT_EQ("8", versions[0].choices[0]);
   }
 
   {
     SdpSimulcastAttribute::Versions versions(
-        ParseSimulcastVersions("8,9;10"));
+        ParseSimulcastVersions("pt=8,9"));
+    ASSERT_EQ(1U, versions.size());
+    ASSERT_EQ(2U, versions[0].choices.size());
+    ASSERT_EQ("8", versions[0].choices[0]);
+    ASSERT_EQ("9", versions[0].choices[1]);
+  }
+
+  {
+    SdpSimulcastAttribute::Versions versions(
+        ParseSimulcastVersions("pt=8,9;10"));
     ASSERT_EQ(2U, versions.size());
     ASSERT_EQ(2U, versions[0].choices.size());
-    ASSERT_EQ(8U, versions[0].choices[0]);
-    ASSERT_EQ(9U, versions[0].choices[1]);
+    ASSERT_EQ("8", versions[0].choices[0]);
+    ASSERT_EQ("9", versions[0].choices[1]);
     ASSERT_EQ(1U, versions[1].choices.size());
-    ASSERT_EQ(10U, versions[1].choices[0]);
+    ASSERT_EQ("10", versions[1].choices[0]);
   }
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastVersionsInvalidParse)
 {
   ParseInvalid<SdpSimulcastAttribute::Versions>("", 0);
-  ParseInvalid<SdpSimulcastAttribute::Versions>("x", 0);
-  ParseInvalid<SdpSimulcastAttribute::Versions>(";", 0);
-  ParseInvalid<SdpSimulcastAttribute::Versions>("8;", 2);
-  ParseInvalid<SdpSimulcastAttribute::Versions>("8;x", 2);
-  ParseInvalid<SdpSimulcastAttribute::Versions>("8;;", 2);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("x", 1);
+  ParseInvalid<SdpSimulcastAttribute::Versions>(";", 1);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("8", 1);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("foo=", 4);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("foo=8", 4);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=9999", 7);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=-1", 5);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=x", 4);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=8;", 5);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=8;x", 6);
+  ParseInvalid<SdpSimulcastAttribute::Versions>("pt=8;;", 5);
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastSerialize)
 {
   std::ostringstream os;
 
   SdpSimulcastAttribute simulcast;
+  simulcast.recvVersions.type = SdpSimulcastAttribute::Versions::kPt;
   simulcast.recvVersions.push_back(SdpSimulcastAttribute::Version());
-  simulcast.recvVersions.back().choices.push_back(8U);
+  simulcast.recvVersions.back().choices.push_back("8");
   simulcast.Serialize(os);
-  ASSERT_EQ("a=simulcast: recv 8" CRLF, os.str());
+  ASSERT_EQ("a=simulcast: recv pt=8" CRLF, os.str());
   os.str("");
 
   simulcast.sendVersions.push_back(SdpSimulcastAttribute::Version());
-  simulcast.sendVersions.back().choices.push_back(9U);
+  simulcast.sendVersions.back().choices.push_back("9");
   simulcast.Serialize(os);
-  ASSERT_EQ("a=simulcast: send 9 recv 8" CRLF, os.str());
-  os.str("");
-
-  simulcast.sendrecvVersions.push_back(SdpSimulcastAttribute::Version());
-  simulcast.sendrecvVersions.back().choices.push_back(0U);
-  simulcast.Serialize(os);
-  ASSERT_EQ("a=simulcast: send 9 recv 8 sendrecv 0" CRLF, os.str());
+  ASSERT_EQ("a=simulcast: send rid=9 recv pt=8" CRLF, os.str());
   os.str("");
 }
 
 static SdpSimulcastAttribute
 ParseSimulcast(const std::string& input)
 {
   std::istringstream is(input);
   std::string error;
@@ -3592,90 +3608,81 @@ ParseSimulcast(const std::string& input)
   EXPECT_TRUE(simulcast.Parse(is, &error)) << error;
   EXPECT_TRUE(is.eof());
   return simulcast;
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastValidParse)
 {
   {
-    SdpSimulcastAttribute simulcast(ParseSimulcast(" send 8"));
+    SdpSimulcastAttribute simulcast(ParseSimulcast(" send pt=8"));
     ASSERT_EQ(1U, simulcast.sendVersions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+              simulcast.sendVersions.type);
     ASSERT_EQ(1U, simulcast.sendVersions[0].choices.size());
-    ASSERT_EQ(8U, simulcast.sendVersions[0].choices[0]);
+    ASSERT_EQ("8", simulcast.sendVersions[0].choices[0]);
     ASSERT_EQ(0U, simulcast.recvVersions.size());
-    ASSERT_EQ(0U, simulcast.sendrecvVersions.size());
   }
 
   {
-    SdpSimulcastAttribute simulcast(ParseSimulcast(" SEND 8"));
+    SdpSimulcastAttribute simulcast(ParseSimulcast(" SEND pt=8"));
     ASSERT_EQ(1U, simulcast.sendVersions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+              simulcast.sendVersions.type);
     ASSERT_EQ(1U, simulcast.sendVersions[0].choices.size());
-    ASSERT_EQ(8U, simulcast.sendVersions[0].choices[0]);
-    ASSERT_EQ(0U, simulcast.recvVersions.size());
-    ASSERT_EQ(0U, simulcast.sendrecvVersions.size());
-  }
-
-  {
-    SdpSimulcastAttribute simulcast(ParseSimulcast(" recv 8"));
-    ASSERT_EQ(1U, simulcast.recvVersions.size());
-    ASSERT_EQ(1U, simulcast.recvVersions[0].choices.size());
-    ASSERT_EQ(8U, simulcast.recvVersions[0].choices[0]);
-    ASSERT_EQ(0U, simulcast.sendVersions.size());
-    ASSERT_EQ(0U, simulcast.sendrecvVersions.size());
-  }
-
-  {
-    SdpSimulcastAttribute simulcast(ParseSimulcast(" sendrecv 8"));
-    ASSERT_EQ(1U, simulcast.sendrecvVersions.size());
-    ASSERT_EQ(1U, simulcast.sendrecvVersions[0].choices.size());
-    ASSERT_EQ(8U, simulcast.sendrecvVersions[0].choices[0]);
-    ASSERT_EQ(0U, simulcast.sendVersions.size());
+    ASSERT_EQ("8", simulcast.sendVersions[0].choices[0]);
     ASSERT_EQ(0U, simulcast.recvVersions.size());
   }
 
   {
+    SdpSimulcastAttribute simulcast(ParseSimulcast(" recv pt=8"));
+    ASSERT_EQ(1U, simulcast.recvVersions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+              simulcast.recvVersions.type);
+    ASSERT_EQ(1U, simulcast.recvVersions[0].choices.size());
+    ASSERT_EQ("8", simulcast.recvVersions[0].choices[0]);
+    ASSERT_EQ(0U, simulcast.sendVersions.size());
+  }
+
+  {
     SdpSimulcastAttribute simulcast(
-        ParseSimulcast(" send 8,9;101;97,98 recv 101,120;97 sendrecv 101;97"));
+        ParseSimulcast(
+          " send pt=8,9;101;97,98 recv pt=101,120;97"));
     ASSERT_EQ(3U, simulcast.sendVersions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+              simulcast.sendVersions.type);
     ASSERT_EQ(2U, simulcast.sendVersions[0].choices.size());
-    ASSERT_EQ(8U, simulcast.sendVersions[0].choices[0]);
-    ASSERT_EQ(9U, simulcast.sendVersions[0].choices[1]);
+    ASSERT_EQ("8", simulcast.sendVersions[0].choices[0]);
+    ASSERT_EQ("9", simulcast.sendVersions[0].choices[1]);
     ASSERT_EQ(1U, simulcast.sendVersions[1].choices.size());
-    ASSERT_EQ(101U, simulcast.sendVersions[1].choices[0]);
+    ASSERT_EQ("101", simulcast.sendVersions[1].choices[0]);
     ASSERT_EQ(2U, simulcast.sendVersions[2].choices.size());
-    ASSERT_EQ(97U, simulcast.sendVersions[2].choices[0]);
-    ASSERT_EQ(98U, simulcast.sendVersions[2].choices[1]);
+    ASSERT_EQ("97", simulcast.sendVersions[2].choices[0]);
+    ASSERT_EQ("98", simulcast.sendVersions[2].choices[1]);
 
     ASSERT_EQ(2U, simulcast.recvVersions.size());
+    ASSERT_EQ(SdpSimulcastAttribute::Versions::kPt,
+              simulcast.recvVersions.type);
     ASSERT_EQ(2U, simulcast.recvVersions[0].choices.size());
-    ASSERT_EQ(101U, simulcast.recvVersions[0].choices[0]);
-    ASSERT_EQ(120U, simulcast.recvVersions[0].choices[1]);
+    ASSERT_EQ("101", simulcast.recvVersions[0].choices[0]);
+    ASSERT_EQ("120", simulcast.recvVersions[0].choices[1]);
     ASSERT_EQ(1U, simulcast.recvVersions[1].choices.size());
-    ASSERT_EQ(97U, simulcast.recvVersions[1].choices[0]);
-
-    ASSERT_EQ(2U, simulcast.sendrecvVersions.size());
-    ASSERT_EQ(1U, simulcast.sendrecvVersions[0].choices.size());
-    ASSERT_EQ(101U, simulcast.sendrecvVersions[0].choices[0]);
-    ASSERT_EQ(1U, simulcast.sendrecvVersions[1].choices.size());
-    ASSERT_EQ(97U, simulcast.sendrecvVersions[1].choices[0]);
+    ASSERT_EQ("97", simulcast.recvVersions[1].choices[0]);
   }
 }
 
 TEST(NewSdpTestNoFixture, CheckSimulcastInvalidParse)
 {
   ParseInvalid<SdpSimulcastAttribute>("", 0);
   ParseInvalid<SdpSimulcastAttribute>(" ", 1);
   ParseInvalid<SdpSimulcastAttribute>("vcer ", 4);
-  ParseInvalid<SdpSimulcastAttribute>(" send x", 6);
-  ParseInvalid<SdpSimulcastAttribute>(" recv x", 6);
-  ParseInvalid<SdpSimulcastAttribute>(" sendrecv x", 10);
-  ParseInvalid<SdpSimulcastAttribute>(" send 8 send ", 12);
-  ParseInvalid<SdpSimulcastAttribute>(" recv 8 recv ", 12);
-  ParseInvalid<SdpSimulcastAttribute>(" sendrecv 8 sendrecv ", 20);
+  ParseInvalid<SdpSimulcastAttribute>(" send x", 7);
+  ParseInvalid<SdpSimulcastAttribute>(" recv x", 7);
+  ParseInvalid<SdpSimulcastAttribute>(" send pt=8 send ", 15);
+  ParseInvalid<SdpSimulcastAttribute>(" recv pt=8 recv ", 15);
 }
 
 static SdpRidAttributeList::Constraints
 ParseRidConstraints(const std::string& input)
 {
   std::istringstream is(input);
   std::string error;
   SdpRidAttributeList::Constraints constraints;
@@ -4119,17 +4126,17 @@ TEST(NewSdpTestNoFixture, CheckRidValidP
     ASSERT_EQ(0U, rid.constraints.dependIds.size());
   }
 
 }
 
 TEST(NewSdpTestNoFixture, CheckRidInvalidParse)
 {
   ParseInvalid<SdpRidAttributeList::Rid>("", 0);
-  ParseInvalid<SdpRidAttributeList::Rid>(" ", 1);
+  ParseInvalid<SdpRidAttributeList::Rid>(" ", 0);
   ParseInvalid<SdpRidAttributeList::Rid>("foo", 3);
   ParseInvalid<SdpRidAttributeList::Rid>("foo ", 4);
   ParseInvalid<SdpRidAttributeList::Rid>("foo  ", 5);
   ParseInvalid<SdpRidAttributeList::Rid>("foo bar", 7);
   ParseInvalid<SdpRidAttributeList::Rid>("foo recv ", 9);
   ParseInvalid<SdpRidAttributeList::Rid>("foo recv pt=", 12);
 }