Bug 1141779: Fix H264 negotiation when level-asymmetry-allowed is not specified, and some cleanup. draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Tue, 10 Mar 2015 17:35:50 -0700
changeset 250126 a878e20c0582b109e76201bbf8d7202734554b29
parent 249710 bc6aeea7229080e26a7d89b350cb78b0c9234099
child 505632 05b604f8f37ee2ceba21a65089f7e73b3d2f1eef
push id1013
push userbcampen@mozilla.com
push dateThu, 12 Mar 2015 20:18:08 +0000
bugs1141779
milestone39.0a1
Bug 1141779: Fix H264 negotiation when level-asymmetry-allowed is not specified, and some cleanup.
media/webrtc/signaling/src/jsep/JsepCodecDescription.h
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/test/jsep_session_unittest.cpp
--- a/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
+++ b/media/webrtc/signaling/src/jsep/JsepCodecDescription.h
@@ -33,20 +33,19 @@ struct JsepCodecDescription {
         mEnabled(enabled)
   {
   }
   virtual ~JsepCodecDescription() {}
 
   virtual JsepCodecDescription* Clone() const = 0;
   virtual void AddFmtps(SdpFmtpAttributeList& fmtp) const = 0;
   virtual void AddRtcpFbs(SdpRtcpFbAttributeList& rtcpfb) const = 0;
-  virtual bool LoadFmtps(const SdpFmtpAttributeList::Parameters& params) = 0;
-  virtual bool LoadRtcpFbs(
-      const SdpRtcpFbAttributeList::Feedback& feedback) = 0;
 
+  // TODO(bug 1142105): This probably should be a helper function in
+  // /sdp
   static bool
   GetPtAsInt(const std::string& ptString, uint16_t* ptOutparam)
   {
     char* end;
     unsigned long pt = strtoul(ptString.c_str(), &end, 10);
     size_t length = static_cast<size_t>(end - ptString.c_str());
     if ((pt > UINT16_MAX) || (length != ptString.size())) {
       return false;
@@ -86,16 +85,38 @@ struct JsepCodecDescription {
   }
 
   virtual bool
   ParametersMatch(const SdpFmtpAttributeList::Parameters* fmtp) const
   {
     return true;
   }
 
+  UniquePtr<JsepCodecDescription>
+  MakeNegotiatedCodec(const std::string& pt,
+                      const SdpMediaSection& remoteMsection) const
+  {
+    UniquePtr<JsepCodecDescription> negotiated(Clone());
+
+    if (!negotiated->Negotiate(pt, remoteMsection)) {
+      negotiated.reset();
+    }
+
+    return negotiated;
+  }
+
+  virtual bool
+  Negotiate(const std::string& pt, const SdpMediaSection& remoteMsection)
+  {
+    mDefaultPt = pt;
+    return true;
+  }
+
+  // TODO(bug 1142105): This probably should be a helper function in
+  // /sdp
   static const SdpFmtpAttributeList::Parameters*
   FindParameters(const std::string& pt,
                  const mozilla::SdpMediaSection& remoteMsection)
   {
     const SdpAttributeList& attrs = remoteMsection.GetAttributeList();
 
     if (attrs.HasAttribute(SdpAttribute::kFmtpAttribute)) {
       const SdpFmtpAttributeList& fmtps = attrs.GetFmtp();
@@ -103,50 +124,56 @@ struct JsepCodecDescription {
         if (i->format == pt && i->parameters) {
           return i->parameters.get();
         }
       }
     }
     return nullptr;
   }
 
-  virtual JsepCodecDescription*
-  MakeNegotiatedCodec(const mozilla::SdpMediaSection& remoteMsection,
-                      const std::string& pt,
-                      bool sending) const
+  UniquePtr<JsepCodecDescription>
+  MakeSendCodec(const mozilla::SdpMediaSection& remoteMsection,
+                const std::string& pt) const
   {
-    UniquePtr<JsepCodecDescription> negotiated(Clone());
-    negotiated->mDefaultPt = pt;
-
-    const SdpAttributeList& attrs = remoteMsection.GetAttributeList();
+    UniquePtr<JsepCodecDescription> sendCodec(Clone());
+    sendCodec->mDefaultPt = pt;
 
-    if (sending) {
-      auto* parameters = FindParameters(negotiated->mDefaultPt, remoteMsection);
-      if (parameters) {
-        if (!negotiated->LoadFmtps(*parameters)) {
-          // Remote parameters were invalid
-          return nullptr;
-        }
-      }
-    } else {
-      // If a receive track, we need to pay attention to remote end's rtcp-fb
-      if (attrs.HasAttribute(SdpAttribute::kRtcpFbAttribute)) {
-        auto& rtcpfbs = attrs.GetRtcpFb().mFeedbacks;
-        for (auto i = rtcpfbs.begin(); i != rtcpfbs.end(); ++i) {
-          if (i->pt == negotiated->mDefaultPt || i->pt == "*") {
-            if (!negotiated->LoadRtcpFbs(*i)) {
-              // Remote parameters were invalid
-              return nullptr;
-            }
-          }
-        }
-      }
+    if (!sendCodec->LoadSendParameters(remoteMsection, pt)) {
+      sendCodec.reset();
     }
 
-    return negotiated.release();
+    return sendCodec;
+  }
+
+  UniquePtr<JsepCodecDescription>
+  MakeRecvCodec(const mozilla::SdpMediaSection& remoteMsection,
+                const std::string& pt) const
+  {
+    UniquePtr<JsepCodecDescription> recvCodec(Clone());
+    recvCodec->mDefaultPt = pt;
+
+    if (!recvCodec->LoadRecvParameters(remoteMsection, pt)) {
+      recvCodec.reset();
+    }
+
+    return recvCodec;
+  }
+
+  virtual bool LoadSendParameters(
+      const mozilla::SdpMediaSection& remoteMsection,
+      const std::string& pt)
+  {
+    return true;
+  }
+
+  virtual bool LoadRecvParameters(
+      const mozilla::SdpMediaSection& remoteMsection,
+      const std::string& pt)
+  {
+    return true;
   }
 
   virtual void
   AddToMediaSection(SdpMediaSection& msection) const
   {
     if (mEnabled && msection.GetMediaType() == mType) {
       if (mType == SdpMediaSection::kApplication) {
         // Hack: using mChannels for number of streams
@@ -229,30 +256,16 @@ struct JsepAudioCodecDescription : publi
   }
 
   virtual void
   AddRtcpFbs(SdpRtcpFbAttributeList& rtcpfb) const MOZ_OVERRIDE
   {
     // TODO: Do we want to add anything?
   }
 
-  virtual bool
-  LoadFmtps(const SdpFmtpAttributeList::Parameters& params) MOZ_OVERRIDE
-  {
-    // TODO
-    return true;
-  }
-
-  virtual bool
-  LoadRtcpFbs(const SdpRtcpFbAttributeList::Feedback& feedback) MOZ_OVERRIDE
-  {
-    // Nothing to do
-    return true;
-  }
-
   JSEP_CODEC_CLONE(JsepAudioCodecDescription)
 
   uint32_t mPacketSize;
   uint32_t mBitrate;
 };
 
 struct JsepVideoCodecDescription : public JsepCodecDescription {
   JsepVideoCodecDescription(const std::string& defaultPt,
@@ -312,238 +325,157 @@ struct JsepVideoCodecDescription : publi
     rtcpfb.PushEntry(mDefaultPt, SdpRtcpFbAttributeList::kNack);
     rtcpfb.PushEntry(
         mDefaultPt, SdpRtcpFbAttributeList::kNack, SdpRtcpFbAttributeList::pli);
     rtcpfb.PushEntry(
         mDefaultPt, SdpRtcpFbAttributeList::kCcm, SdpRtcpFbAttributeList::fir);
   }
 
   virtual bool
-  LoadFmtps(const SdpFmtpAttributeList::Parameters& params) MOZ_OVERRIDE
+  Negotiate(const std::string& pt,
+            const SdpMediaSection& remoteMsection) MOZ_OVERRIDE
+  {
+    auto* params = FindParameters(pt, remoteMsection);
+    if (params && params->codec_type == SdpRtpmapAttributeList::kH264) {
+      const SdpFmtpAttributeList::H264Parameters& h264Params =
+        static_cast<const SdpFmtpAttributeList::H264Parameters&>(*params);
+
+      if (!h264Params.level_asymmetry_allowed) {
+        NegotiateProfileLevelId(h264Params.profile_level_id);
+      }
+    }
+
+    return JsepCodecDescription::Negotiate(pt, remoteMsection);
+  }
+
+  void
+  NegotiateProfileLevelId(uint32_t remoteProfileLevelId)
+  {
+    uint8_t remoteLevel = remoteProfileLevelId & 0xFF;
+    uint8_t localLevel = mProfileLevelId & 0xFF;
+
+    if (remoteLevel < localLevel) {
+      localLevel = remoteLevel;
+    }
+
+    mProfileLevelId = (remoteProfileLevelId & 0xFFFF00) + localLevel;
+  }
+
+  virtual bool
+  LoadSendParameters(const mozilla::SdpMediaSection& remoteMsection,
+                     const std::string& pt) MOZ_OVERRIDE
   {
-    switch (params.codec_type) {
-      case SdpRtpmapAttributeList::kH264:
-        LoadH264Parameters(params);
-        break;
-      case SdpRtpmapAttributeList::kVP9:
-        // VP8 and VP9 share the same SDP parameters thus far
-      case SdpRtpmapAttributeList::kVP8:
-        LoadVP8Parameters(params);
-        break;
-      case SdpRtpmapAttributeList::kiLBC:
-      case SdpRtpmapAttributeList::kiSAC:
-      case SdpRtpmapAttributeList::kOpus:
-      case SdpRtpmapAttributeList::kG722:
-      case SdpRtpmapAttributeList::kPCMU:
-      case SdpRtpmapAttributeList::kPCMA:
-      case SdpRtpmapAttributeList::kOtherCodec:
-        MOZ_ASSERT(false, "Invalid codec type for video");
+    auto* params = FindParameters(pt, remoteMsection);
+    if (params) {
+      switch (params->codec_type) {
+        case SdpRtpmapAttributeList::kH264:
+          {
+            const SdpFmtpAttributeList::H264Parameters& h264Params =
+              static_cast<const SdpFmtpAttributeList::H264Parameters&>(*params);
+
+            if (!h264Params.level_asymmetry_allowed) {
+              NegotiateProfileLevelId(h264Params.profile_level_id);
+            }
+
+            mMaxFs = h264Params.max_fs;
+            mMaxMbps = h264Params.max_mbps;
+            mMaxCpb = h264Params.max_cpb;
+            mMaxDpb = h264Params.max_dpb;
+            mMaxBr = h264Params.max_br;
+            mSpropParameterSets = h264Params.sprop_parameter_sets;
+          }
+          break;
+        case SdpRtpmapAttributeList::kVP9:
+          // VP8 and VP9 share the same SDP params thus far
+        case SdpRtpmapAttributeList::kVP8:
+          {
+            const SdpFmtpAttributeList::VP8Parameters& vp8Params =
+              static_cast<const SdpFmtpAttributeList::VP8Parameters&>(*params);
+
+            mMaxFs = vp8Params.max_fs;
+            mMaxFr = vp8Params.max_fr;
+          }
+          break;
+        case SdpRtpmapAttributeList::kiLBC:
+        case SdpRtpmapAttributeList::kiSAC:
+        case SdpRtpmapAttributeList::kOpus:
+        case SdpRtpmapAttributeList::kG722:
+        case SdpRtpmapAttributeList::kPCMU:
+        case SdpRtpmapAttributeList::kPCMA:
+        case SdpRtpmapAttributeList::kOtherCodec:
+          MOZ_ASSERT(false, "Invalid codec type for video");
+      }
     }
     return true;
   }
 
   virtual bool
-  LoadRtcpFbs(const SdpRtcpFbAttributeList::Feedback& feedback) MOZ_OVERRIDE
+  LoadRecvParameters(const mozilla::SdpMediaSection& remoteMsection,
+                     const std::string& pt) MOZ_OVERRIDE
   {
-    switch (feedback.type) {
-      case SdpRtcpFbAttributeList::kAck:
-        mAckFbTypes.push_back(feedback.parameter);
-        break;
-      case SdpRtcpFbAttributeList::kCcm:
-        mCcmFbTypes.push_back(feedback.parameter);
-        break;
-      case SdpRtcpFbAttributeList::kNack:
-        mNackFbTypes.push_back(feedback.parameter);
-        break;
-      case SdpRtcpFbAttributeList::kApp:
-      case SdpRtcpFbAttributeList::kTrrInt:
-        // We don't support these, ignore.
-        {}
-    }
-    return true;
-  }
+    const SdpAttributeList& attrs(remoteMsection.GetAttributeList());
 
-  enum Subprofile {
-    kH264ConstrainedBaseline,
-    kH264Baseline,
-    kH264Main,
-    kH264Extended,
-    kH264High,
-    kH264High10,
-    kH264High42,
-    kH264High44,
-    kH264High10I,
-    kH264High42I,
-    kH264High44I,
-    kH264CALVC44,
-    kH264UnknownSubprofile
-  };
-
-  static Subprofile
-  GetSubprofile(uint32_t profileLevelId)
-  {
-    // Based on Table 5 from RFC 6184:
-    //        Profile     profile_idc        profile-iop
-    //                    (hexadecimal)      (binary)
-
-    //        CB          42 (B)             x1xx0000
-    //           same as: 4D (M)             1xxx0000
-    //           same as: 58 (E)             11xx0000
-    //        B           42 (B)             x0xx0000
-    //           same as: 58 (E)             10xx0000
-    //        M           4D (M)             0x0x0000
-    //        E           58                 00xx0000
-    //        H           64                 00000000
-    //        H10         6E                 00000000
-    //        H42         7A                 00000000
-    //        H44         F4                 00000000
-    //        H10I        6E                 00010000
-    //        H42I        7A                 00010000
-    //        H44I        F4                 00010000
-    //        C44I        2C                 00010000
-
-    if ((profileLevelId & 0xFF4F00) == 0x424000) {
-      // 01001111 (mask, 0x4F)
-      // x1xx0000 (from table)
-      // 01000000 (expected value, 0x40)
-      return kH264ConstrainedBaseline;
+    if (attrs.HasAttribute(SdpAttribute::kRtcpFbAttribute)) {
+      auto& rtcpfbs = attrs.GetRtcpFb().mFeedbacks;
+      for (auto i = rtcpfbs.begin(); i != rtcpfbs.end(); ++i) {
+        if (i->pt == mDefaultPt || i->pt == "*") {
+          switch (i->type) {
+            case SdpRtcpFbAttributeList::kAck:
+              mAckFbTypes.push_back(i->parameter);
+              break;
+            case SdpRtcpFbAttributeList::kCcm:
+              mCcmFbTypes.push_back(i->parameter);
+              break;
+            case SdpRtcpFbAttributeList::kNack:
+              mNackFbTypes.push_back(i->parameter);
+              break;
+            case SdpRtcpFbAttributeList::kApp:
+            case SdpRtcpFbAttributeList::kTrrInt:
+              // We don't support these, ignore.
+              {}
+          }
+        }
+      }
     }
 
-    if ((profileLevelId & 0xFF8F00) == 0x4D8000) {
-      // 10001111 (mask, 0x8F)
-      // 1xxx0000 (from table)
-      // 10000000 (expected value, 0x80)
-      return kH264ConstrainedBaseline;
-    }
-
-    if ((profileLevelId & 0xFFCF00) == 0x58C000) {
-      // 11001111 (mask, 0xCF)
-      // 11xx0000 (from table)
-      // 11000000 (expected value, 0xC0)
-      return kH264ConstrainedBaseline;
-    }
+    auto* params = FindParameters(pt, remoteMsection);
+    if (params) {
+      if (params->codec_type == SdpRtpmapAttributeList::kH264) {
+        const SdpFmtpAttributeList::H264Parameters& h264Params =
+          static_cast<const SdpFmtpAttributeList::H264Parameters&>(*params);
 
-    if ((profileLevelId & 0xFF4F00) == 0x420000) {
-      // 01001111 (mask, 0x4F)
-      // x0xx0000 (from table)
-      // 00000000 (expected value)
-      return kH264Baseline;
-    }
-
-    if ((profileLevelId & 0xFFCF00) == 0x588000) {
-      // 11001111 (mask, 0xCF)
-      // 10xx0000 (from table)
-      // 10000000 (expected value, 0x80)
-      return kH264Baseline;
-    }
-
-    if ((profileLevelId & 0xFFAF00) == 0x4D0000) {
-      // 10101111 (mask, 0xAF)
-      // 0x0x0000 (from table)
-      // 00000000 (expected value)
-      return kH264Main;
+        mProfileLevelId = h264Params.profile_level_id;
+      }
     }
 
-    if ((profileLevelId & 0xFF0000) == 0x580000) {
-      // 11001111 (mask, 0xCF)
-      // 00xx0000 (from table)
-      // 00000000 (expected value)
-      return kH264Extended;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x640000) {
-      return kH264High;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x6E0000) {
-      return kH264High10;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x7A0000) {
-      return kH264High42;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0xF40000) {
-      return kH264High44;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x6E1000) {
-      return kH264High10I;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x7A1000) {
-      return kH264High42I;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0xF41000) {
-      return kH264High44I;
-    }
-
-    if ((profileLevelId & 0xFFFF00) == 0x2C1000) {
-      return kH264CALVC44;
-    }
-
-    return kH264UnknownSubprofile;
+    return true;
   }
 
   virtual bool
   ParametersMatch(const SdpFmtpAttributeList::Parameters* fmtp) const
       MOZ_OVERRIDE
   {
     if (mName == "H264") {
-      if (!fmtp) {
-        // No fmtp means that we cannot assume level asymmetry is allowed,
-        // and since we have no way of knowing the profile-level-id, we can't
-        // say that we match.
-        return false;
+      uint32_t packetizationMode = 0;
+
+      if (fmtp) {
+        auto* h264Params =
+            static_cast<const SdpFmtpAttributeList::H264Parameters*>(fmtp);
+
+        packetizationMode = h264Params->packetization_mode;
       }
 
-      auto* h264Params =
-          static_cast<const SdpFmtpAttributeList::H264Parameters*>(fmtp);
-
-      if (!h264Params->level_asymmetry_allowed) {
-        if (GetSubprofile(h264Params->profile_level_id) !=
-            GetSubprofile(mProfileLevelId)) {
-          return false;
-        }
-      }
-
-      if (h264Params->packetization_mode != mPacketizationMode) {
+      if (packetizationMode != mPacketizationMode) {
         return false;
       }
     }
+
     return true;
   }
 
-  void
-  LoadH264Parameters(const SdpFmtpAttributeList::Parameters& params)
-  {
-    const SdpFmtpAttributeList::H264Parameters& h264Params =
-        static_cast<const SdpFmtpAttributeList::H264Parameters&>(params);
-
-    mMaxFs = h264Params.max_fs;
-    mProfileLevelId = h264Params.profile_level_id;
-    mPacketizationMode = h264Params.packetization_mode;
-    mMaxMbps = h264Params.max_mbps;
-    mMaxCpb = h264Params.max_cpb;
-    mMaxDpb = h264Params.max_dpb;
-    mMaxBr = h264Params.max_br;
-    mSpropParameterSets = h264Params.sprop_parameter_sets;
-  }
-
-  void
-  LoadVP8Parameters(const SdpFmtpAttributeList::Parameters& params)
-  {
-    const SdpFmtpAttributeList::VP8Parameters& vp8Params =
-        static_cast<const SdpFmtpAttributeList::VP8Parameters&>(params);
-
-    mMaxFs = vp8Params.max_fs;
-    mMaxFr = vp8Params.max_fr;
-  }
-
   JSEP_CODEC_CLONE(JsepVideoCodecDescription)
 
   std::vector<std::string> mAckFbTypes;
   std::vector<std::string> mNackFbTypes;
   std::vector<std::string> mCcmFbTypes;
 
   uint32_t mMaxFs;
 
@@ -575,30 +507,16 @@ struct JsepApplicationCodecDescription :
   }
 
   virtual void
   AddRtcpFbs(SdpRtcpFbAttributeList& rtcpfb) const MOZ_OVERRIDE
   {
     // Nothing to do here.
   }
 
-  virtual bool
-  LoadFmtps(const SdpFmtpAttributeList::Parameters& params) MOZ_OVERRIDE
-  {
-    // TODO: Is there anything to do here?
-    return true;
-  }
-
-  virtual bool
-  LoadRtcpFbs(const SdpRtcpFbAttributeList::Feedback& feedback) MOZ_OVERRIDE
-  {
-    // Nothing to do
-    return true;
-  }
-
   JSEP_CODEC_CLONE(JsepApplicationCodecDescription)
 
   // Override, uses sctpmap instead of rtpmap
   virtual bool
   Matches(const std::string& fmt,
           const SdpMediaSection& remoteMsection) const MOZ_OVERRIDE
   {
     auto& attrs = remoteMsection.GetAttributeList();
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -880,18 +880,22 @@ void
 JsepSessionImpl::AddCommonCodecs(const SdpMediaSection& remoteMsection,
                                  SdpMediaSection* msection)
 {
   const std::vector<std::string>& formats = remoteMsection.GetFormats();
 
   for (auto fmt = formats.begin(); fmt != formats.end(); ++fmt) {
     JsepCodecDescription* codec = FindMatchingCodec(*fmt, remoteMsection);
     if (codec) {
-      codec->mDefaultPt = *fmt; // Reflect the other side's PT
-      codec->AddToMediaSection(*msection);
+      UniquePtr<JsepCodecDescription> negotiated(
+          codec->MakeNegotiatedCodec(*fmt, remoteMsection));
+      if (negotiated) {
+        negotiated->AddToMediaSection(*msection);
+        codec->mDefaultPt = *fmt; // Remember the other side's PT
+      }
       // TODO(bug 1099351): Once bug 1073475 is fixed on all supported
       // versions, we can remove this limitation.
       break;
     }
   }
 }
 
 void
@@ -1638,42 +1642,58 @@ JsepSessionImpl::NegotiateTrack(const Sd
 
   for (auto fmt = formats.begin(); fmt != formats.end(); ++fmt) {
     JsepCodecDescription* codec = FindMatchingCodec(*fmt, remoteMsection);
 
     if (!codec) {
       continue;
     }
 
-    bool sending = (direction == JsepTrack::kJsepTrackSending);
-
-    // We need to take the remote side's parameters into account so we can
-    // configure our send media.
-    // |codec| is assumed to have the necessary state about our own config
-    // in order to negotiate.
-    JsepCodecDescription* negotiated =
-        codec->MakeNegotiatedCodec(remoteMsection, *fmt, sending);
+    UniquePtr<JsepCodecDescription> negotiated(
+        codec->MakeNegotiatedCodec(*fmt, remoteMsection));
 
     if (!negotiated) {
       continue;
     }
 
+    bool sending = (direction == JsepTrack::kJsepTrackSending);
+
+    // Everywhere else in JsepSessionImpl, a JsepCodecDescription describes
+    // what one side puts in its SDP. However, we don't want that here; we want
+    // a JsepCodecDescription that instead encapsulates all the parameters
+    // that deal with sending (or receiving). Some of these parameters will come
+    // from the local SDP (eg; profile-level-id for H264), others will come from
+    // the remote SDP (eg; max-fps, rtcp-fb stuff).
+    UniquePtr<JsepCodecDescription> sendOrReceiveCodec;
+
+    if (sending) {
+      sendOrReceiveCodec =
+        Move(negotiated->MakeSendCodec(remoteMsection, *fmt));
+    } else {
+      sendOrReceiveCodec =
+        Move(negotiated->MakeRecvCodec(remoteMsection, *fmt));
+    }
+
+    if (!sendOrReceiveCodec) {
+      continue;
+    }
+
     if (remoteMsection.GetMediaType() == SdpMediaSection::kAudio ||
         remoteMsection.GetMediaType() == SdpMediaSection::kVideo) {
       // Sanity-check that payload type can work with RTP
       uint16_t payloadType;
-      if (!negotiated->GetPtAsInt(&payloadType) ||
+      if (!sendOrReceiveCodec->GetPtAsInt(&payloadType) ||
           payloadType > UINT8_MAX) {
         JSEP_SET_ERROR("audio/video payload type is not an 8 bit unsigned int: "
                        << *fmt);
         return NS_ERROR_INVALID_ARG;
       }
     }
 
-    negotiatedDetails->mCodecs.push_back(negotiated);
+    negotiatedDetails->mCodecs.push_back(sendOrReceiveCodec.release());
     break;
   }
 
   if (negotiatedDetails->mCodecs.empty()) {
     JSEP_SET_ERROR("Failed to negotiate codec details for all codecs");
     return NS_ERROR_INVALID_ARG;
   }
 
--- a/media/webrtc/signaling/test/jsep_session_unittest.cpp
+++ b/media/webrtc/signaling/test/jsep_session_unittest.cpp
@@ -2307,16 +2307,227 @@ TEST_F(JsepSessionTest, ValidateAnswered
         fmtps[2].parameters.get());
 
   ASSERT_EQ((uint32_t)0x42a00d, parsed_h264_0_params.profile_level_id);
   ASSERT_TRUE(parsed_h264_0_params.level_asymmetry_allowed);
   ASSERT_EQ(0U, parsed_h264_0_params.packetization_mode);
 #endif
 }
 
+static void
+Replace(const std::string& toReplace,
+        const std::string& with,
+        std::string* in)
+{
+  size_t pos = in->find(toReplace);
+  ASSERT_NE(std::string::npos, pos);
+  in->replace(pos, toReplace.size(), with);
+}
+
+static void
+GetCodec(JsepSession& session,
+         size_t pairIndex,
+         bool sending,
+         size_t codecIndex,
+         const JsepCodecDescription** codecOut)
+{
+  ASSERT_LT(pairIndex, session.GetNegotiatedTrackPairs().size());
+  JsepTrackPair pair(session.GetNegotiatedTrackPairs().front());
+  RefPtr<JsepTrack> track(sending ? pair.mSending : pair.mReceiving);
+  ASSERT_TRUE(track);
+  ASSERT_TRUE(track->GetNegotiatedDetails());
+  ASSERT_LT(codecIndex, track->GetNegotiatedDetails()->GetCodecCount());
+  ASSERT_EQ(NS_OK,
+            track->GetNegotiatedDetails()->GetCodec(codecIndex, codecOut));
+}
+
+TEST_F(JsepSessionTest, TestH264Negotiation)
+{
+  for (JsepCodecDescription* codec : mSessionOff.Codecs()) {
+    if (codec->mName == "H264") {
+      JsepVideoCodecDescription* h264 =
+          static_cast<JsepVideoCodecDescription*>(codec);
+      h264->mProfileLevelId = 0x42a00b;
+    } else {
+      codec->mEnabled = false;
+    }
+  }
+
+  for (JsepCodecDescription* codec : mSessionAns.Codecs()) {
+    if (codec->mName == "H264") {
+      JsepVideoCodecDescription* h264 =
+          static_cast<JsepVideoCodecDescription*>(codec);
+      h264->mProfileLevelId = 0x42e00d;
+    } else {
+      codec->mEnabled = false;
+    }
+  }
+
+  AddTracks(mSessionOff, "video");
+  AddTracks(mSessionAns, "video");
+
+  std::string offer(CreateOffer());
+  SetLocalOffer(offer, CHECK_SUCCESS);
+
+  SetRemoteOffer(offer, CHECK_SUCCESS);
+  std::string answer(CreateAnswer());
+
+  SetRemoteAnswer(answer, CHECK_SUCCESS);
+  SetLocalAnswer(answer, CHECK_SUCCESS);
+
+  const JsepCodecDescription* offererSendCodec;
+  GetCodec(mSessionOff, 0, true, 0, &offererSendCodec);
+  ASSERT_TRUE(offererSendCodec);
+  ASSERT_EQ("H264", offererSendCodec->mName);
+  const JsepVideoCodecDescription* offererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererSendCodec));
+  ASSERT_EQ(0x42a00b, offererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* offererRecvCodec;
+  GetCodec(mSessionOff, 0, false, 0, &offererRecvCodec);
+  ASSERT_EQ("H264", offererRecvCodec->mName);
+  const JsepVideoCodecDescription* offererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererRecvCodec));
+  ASSERT_EQ(0x42e00d, offererVideoRecvCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererSendCodec;
+  GetCodec(mSessionAns, 0, true, 0, &answererSendCodec);
+  ASSERT_TRUE(answererSendCodec);
+  ASSERT_EQ("H264", answererSendCodec->mName);
+  const JsepVideoCodecDescription* answererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererSendCodec));
+  ASSERT_EQ(0x42e00d, answererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererRecvCodec;
+  GetCodec(mSessionAns, 0, false, 0, &answererRecvCodec);
+  ASSERT_EQ("H264", answererRecvCodec->mName);
+  const JsepVideoCodecDescription* answererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererRecvCodec));
+  ASSERT_EQ(0x42a00b, answererVideoRecvCodec->mProfileLevelId);
+}
+
+TEST_F(JsepSessionTest, TestH264LevelAsymmetryDisallowedByOfferer)
+{
+  for (JsepCodecDescription* codec : mSessionOff.Codecs()) {
+    if (codec->mName == "H264") {
+      JsepVideoCodecDescription* h264 =
+          static_cast<JsepVideoCodecDescription*>(codec);
+      h264->mProfileLevelId = 0x42a00b;
+    } else {
+      codec->mEnabled = false;
+    }
+  }
+
+  AddTracks(mSessionOff, "video");
+  AddTracks(mSessionAns, "video");
+
+  std::string offer(CreateOffer());
+  SetLocalOffer(offer, CHECK_SUCCESS);
+
+  Replace("level-asymmetry-allowed=1",
+          "level-asymmetry-allowed=0",
+          &offer);
+
+  SetRemoteOffer(offer, CHECK_SUCCESS);
+  std::string answer(CreateAnswer());
+
+  ASSERT_NE(std::string::npos, answer.find("profile-level-id=42a00b"));
+  SetRemoteAnswer(answer, CHECK_SUCCESS);
+  SetLocalAnswer(answer, CHECK_SUCCESS);
+
+  const JsepCodecDescription* offererSendCodec;
+  GetCodec(mSessionOff, 0, true, 0, &offererSendCodec);
+  ASSERT_TRUE(offererSendCodec);
+  ASSERT_EQ("H264", offererSendCodec->mName);
+  const JsepVideoCodecDescription* offererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererSendCodec));
+  ASSERT_EQ(0x42a00b, offererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* offererRecvCodec;
+  GetCodec(mSessionOff, 0, false, 0, &offererRecvCodec);
+  ASSERT_EQ("H264", offererRecvCodec->mName);
+  const JsepVideoCodecDescription* offererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererRecvCodec));
+  ASSERT_EQ(0x42a00b, offererVideoRecvCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererSendCodec;
+  GetCodec(mSessionAns, 0, true, 0, &answererSendCodec);
+  ASSERT_TRUE(answererSendCodec);
+  ASSERT_EQ("H264", answererSendCodec->mName);
+  const JsepVideoCodecDescription* answererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererSendCodec));
+  ASSERT_EQ(0x42a00b, answererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererRecvCodec;
+  GetCodec(mSessionAns, 0, false, 0, &answererRecvCodec);
+  ASSERT_EQ("H264", answererRecvCodec->mName);
+  const JsepVideoCodecDescription* answererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererRecvCodec));
+  ASSERT_EQ(0x42a00b, answererVideoRecvCodec->mProfileLevelId);
+}
+
+TEST_F(JsepSessionTest, TestH264LevelAsymmetryDisallowedByAnswerer)
+{
+  for (JsepCodecDescription* codec : mSessionAns.Codecs()) {
+    if (codec->mName == "H264") {
+      JsepVideoCodecDescription* h264 =
+          static_cast<JsepVideoCodecDescription*>(codec);
+      h264->mProfileLevelId = 0x42a00b;
+    } else {
+      codec->mEnabled = false;
+    }
+  }
+
+  AddTracks(mSessionOff, "video");
+  AddTracks(mSessionAns, "video");
+
+  std::string offer(CreateOffer());
+  SetLocalOffer(offer, CHECK_SUCCESS);
+  SetRemoteOffer(offer, CHECK_SUCCESS);
+  std::string answer(CreateAnswer());
+
+  Replace("level-asymmetry-allowed=1",
+          "level-asymmetry-allowed=0",
+          &answer);
+
+  ASSERT_NE(std::string::npos, answer.find("profile-level-id=42a00b"));
+  SetRemoteAnswer(answer, CHECK_SUCCESS);
+  SetLocalAnswer(answer, CHECK_SUCCESS);
+
+  const JsepCodecDescription* offererSendCodec;
+  GetCodec(mSessionOff, 0, true, 0, &offererSendCodec);
+  ASSERT_TRUE(offererSendCodec);
+  ASSERT_EQ("H264", offererSendCodec->mName);
+  const JsepVideoCodecDescription* offererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererSendCodec));
+  ASSERT_EQ(0x42a00b, offererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* offererRecvCodec;
+  GetCodec(mSessionOff, 0, false, 0, &offererRecvCodec);
+  ASSERT_EQ("H264", offererRecvCodec->mName);
+  const JsepVideoCodecDescription* offererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(offererRecvCodec));
+  ASSERT_EQ(0x42a00b, offererVideoRecvCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererSendCodec;
+  GetCodec(mSessionAns, 0, true, 0, &answererSendCodec);
+  ASSERT_TRUE(answererSendCodec);
+  ASSERT_EQ("H264", answererSendCodec->mName);
+  const JsepVideoCodecDescription* answererVideoSendCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererSendCodec));
+  ASSERT_EQ(0x42a00b, answererVideoSendCodec->mProfileLevelId);
+
+  const JsepCodecDescription* answererRecvCodec;
+  GetCodec(mSessionAns, 0, false, 0, &answererRecvCodec);
+  ASSERT_EQ("H264", answererRecvCodec->mName);
+  const JsepVideoCodecDescription* answererVideoRecvCodec(
+      static_cast<const JsepVideoCodecDescription*>(answererRecvCodec));
+  ASSERT_EQ(0x42e00d, answererVideoRecvCodec->mProfileLevelId);
+}
+
 TEST_P(JsepSessionTest, TestRejectMline)
 {
   AddTracks(mSessionOff);
   AddTracks(mSessionAns);
 
   switch (types.front()) {
     case SdpMediaSection::kAudio:
       // Sabotage audio