Add SdpHelper for more reliable codec configuration in WebrtcTransport.

remoting/protocol/webrtc_transport.cc

Index: remoting/protocol/webrtc_transport.cc
diff --git a/remoting/protocol/webrtc_transport.cc b/remoting/protocol/webrtc_transport.cc
index 805bbbae6f1f67a46d4de24f18cb2dff1f460454..5b4d832546681ae42578e4e360d67d65aa6d580f 100644
--- a/remoting/protocol/webrtc_transport.cc
+++ b/remoting/protocol/webrtc_transport.cc
@@ -22,6 +22,7 @@
 #include "jingle/glue/thread_wrapper.h"
 #include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/port_allocator_factory.h"
+#include "remoting/protocol/sdp_message.h"
 #include "remoting/protocol/stream_message_pipe_adapter.h"
 #include "remoting/protocol/transport_context.h"
 #include "remoting/protocol/webrtc_audio_module.h"
@@ -56,15 +57,38 @@ bool IsValidSessionDescriptionType(const std::string& type) {
          type == webrtc::SessionDescriptionInterface::kAnswer;
 }
 
-// Normalizes the SDP message to make sure the text used for HMAC signatures
-// verifications is the same that was signed on the sending side. This is
-// necessary because WebRTC generates SDP with CRLF line endings which are
-// sometimes converted to LF after passing the signaling channel.
-std::string NormalizeSessionDescription(const std::string& sdp) {
-  // Split SDP lines. The CR symbols is removed by the TRIM_WHITESPACE flag.
-  std::vector<std::string> lines =
-      SplitString(sdp, "\n", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  return base::JoinString(lines, "\n") + "\n";
+void UpdateCodecParameters(SdpMessage* sdp_message, bool incoming) {
+  // Set bitrate range to 1-100 Mbps.
+  //  - Setting min bitrate here enables padding.
+  //  - The default max bitrate is 600 kbps. Setting it to 100 Mbps allows to
+  //    use higher bandwidth when it's available.
+  //
+  // TODO(sergeyu): Padding needs to be enabled to workaround BW estimator not
+  // handling spiky traffic patterns well. This won't be necessary with a
+  // better bandwidth estimator.
+  // TODO(isheriff): The need for this should go away once we have a proper
+  // API to provide max bitrate for the case of handing over encoded
+  // frames to webrtc.
+  if (sdp_message->has_video() &&
+      !sdp_message->AddCodecParameter(
+          "VP8", "x-google-min-bitrate=1000; x-google-max-bitrate=100000")) {
+    if (incoming) {
+      LOG(WARNING) << "VP8 not found in an incoming SDP.";
+    } else {
+      LOG(FATAL) << "VP8 not found in SDP generated by WebRTC.";
+    }
+  }
+
+  // Update SDP format to use stereo for opus codec.
+  if (sdp_message->has_audio() &&
+      !sdp_message->AddCodecParameter("opus",
+                                      "stereo=1; x-google-min-bitrate=160")) {
+    if (incoming) {
+      LOG(WARNING) << "Opus not found in an incoming SDP.";
+    } else {
+      LOG(FATAL) << "Opus not found in SDP generated by WebRTC.";
+    }
+  }
 }
 
 // A webrtc::CreateSessionDescriptionObserver implementation used to receive the
@@ -329,18 +353,20 @@ bool WebrtcTransport::ProcessTransportInfo(XmlElement* transport_info) {
     }
 
     std::string type = session_description->Attr(QName(std::string(), "type"));
-    std::string sdp =
-        NormalizeSessionDescription(session_description->BodyText());
-    if (!IsValidSessionDescriptionType(type) || sdp.empty()) {
+    std::string raw_sdp = session_description->BodyText();
+    if (!IsValidSessionDescriptionType(type) || raw_sdp.empty()) {
       LOG(ERROR) << "Incorrect session description format.";
       return false;
     }
 
+    SdpMessage sdp_message(raw_sdp);
+
     std::string signature_base64 =
         session_description->Attr(QName(std::string(), "signature"));
     std::string signature;
     if (!base::Base64Decode(signature_base64, &signature) ||
-        !handshake_hmac_.Verify(type + " " + sdp, signature)) {
+        !handshake_hmac_.Verify(type + " " + sdp_message.ToString(),
+                                signature)) {
       LOG(WARNING) << "Received session-description with invalid signature.";
       bool ignore_error = false;
 #if !defined(NDEBUG)
@@ -353,27 +379,11 @@ bool WebrtcTransport::ProcessTransportInfo(XmlElement* transport_info) {
       }
     }
 
-    // Set bitrate range to 1-100 Mbps.
-    //  - Setting min bitrate here enables padding.
-    //  - The default max bitrate is 600 kbps. Setting it to 100 Mbps allows to
-    //    use higher bandwidth when it's available.
-    //
-    // TODO(sergeyu): Padding needs to be enabled to workaround BW estimator not
-    // handling spiky traffic patterns well. This won't be necessary with a
-    // better bandwidth estimator.
-    // TODO(isheriff): The need for this should go away once we have a proper
-    // API to provide max bitrate for the case of handing over encoded
-    // frames to webrtc.
-    std::string vp8line = "a=rtpmap:100 VP8/90000\n";
-    std::string vp8line_with_bitrate =
-        vp8line +
-        "a=fmtp:100 x-google-min-bitrate=1000\n"
-        "a=fmtp:100 x-google-max-bitrate=100000\n";
-    base::ReplaceSubstringsAfterOffset(&sdp, 0, vp8line, vp8line_with_bitrate);
+    UpdateCodecParameters(&sdp_message, /*incoming=*/true);
 
     webrtc::SdpParseError error;
     std::unique_ptr<webrtc::SessionDescriptionInterface> session_description(
-        webrtc::CreateSessionDescription(type, sdp, &error));
+        webrtc::CreateSessionDescription(type, sdp_message.ToString(), &error));
     if (!session_description) {
       LOG(ERROR) << "Failed to parse the session description: "
                  << error.description << " line: " << error.line;
@@ -449,14 +459,10 @@ void WebrtcTransport::OnLocalSessionDescriptionCreated(
     Close(CHANNEL_CONNECTION_ERROR);
     return;
   }
-  description_sdp = NormalizeSessionDescription(description_sdp);
 
-  // Update SDP format to use stereo for opus codec.
-  std::string opus_line = "a=rtpmap:111 opus/48000/2\n";
-  std::string opus_line_with_bitrate =
-      opus_line + "a=fmtp:111 stereo=1; x-google-min-bitrate=160\n";
-  base::ReplaceSubstringsAfterOffset(&description_sdp, 0, opus_line,
-                                     opus_line_with_bitrate);
+  SdpMessage sdp_message(description_sdp);
+  UpdateCodecParameters(&sdp_message, /*incoming=*/false);
+  description_sdp = sdp_message.ToString();
 
   // Format and send the session description to the peer.
   std::unique_ptr<XmlElement> transport_info(