Add SdpHelper for more reliable codec configuration in WebrtcTransport.

remoting/protocol/webrtc_transport.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/protocol/webrtc_transport.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/base64.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #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"
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
 #include "third_party/webrtc/api/test/fakeconstraints.h"
 #include "third_party/webrtc/libjingle/xmllite/xmlelement.h"
 
 using buzz::QName;
 using buzz::XmlElement;
 
@@ skipping 14 matching lines @@
 // Command line switch used to disable signature verification.
 // TODO(sergeyu): Remove this flag.
 const char kDisableAuthenticationSwitchName[] = "disable-authentication";
 #endif
 
 bool IsValidSessionDescriptionType(const std::string& type) {
   return type == webrtc::SessionDescriptionInterface::kOffer ||
          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
 // results of creating descriptions for this end of the PeerConnection.
 class CreateSessionDescriptionObserver
     : public webrtc::CreateSessionDescriptionObserver {
  public:
   typedef base::Callback<void(
       std::unique_ptr<webrtc::SessionDescriptionInterface> description,
       const std::string& error)>
@@ skipping 244 matching lines @@
     webrtc::PeerConnectionInterface::SignalingState expected_state =
         transport_context_->role() == TransportRole::CLIENT
             ? webrtc::PeerConnectionInterface::kStable
             : webrtc::PeerConnectionInterface::kHaveLocalOffer;
     if (peer_connection()->signaling_state() != expected_state) {
       LOG(ERROR) << "Received unexpected WebRTC session_description.";
       return false;
     }
 
     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)
       ignore_error = base::CommandLine::ForCurrentProcess()->HasSwitch(
           kDisableAuthenticationSwitchName);
 #endif
       if (!ignore_error) {
         Close(AUTHENTICATION_FAILED);
         return true;
       }
     }
 
-    // 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;
       return false;
     }
 
     peer_connection()->SetRemoteDescription(
         SetSessionDescriptionObserver::Create(
             base::Bind(&WebrtcTransport::OnRemoteDescriptionSet,
                        weak_factory_.GetWeakPtr(),
@@ skipping 55 matching lines @@
     Close(CHANNEL_CONNECTION_ERROR);
     return;
   }
 
   std::string description_sdp;
   if (!description->ToString(&description_sdp)) {
     LOG(ERROR) << "Failed to serialize description.";
     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(
       new XmlElement(QName(kTransportNamespace, "transport"), true));
   XmlElement* offer_tag =
       new XmlElement(QName(kTransportNamespace, "session-description"));
   transport_info->AddElement(offer_tag);
   offer_tag->SetAttr(QName(std::string(), "type"), description->type());
   offer_tag->SetBodyText(description_sdp);
 
@@ skipping 217 matching lines @@
   // the stack and so it must be destroyed later.
   base::ThreadTaskRunnerHandle::Get()->DeleteSoon(
       FROM_HERE, peer_connection_wrapper_.release());
 
   if (error != OK)
     event_handler_->OnWebrtcTransportError(error);
 }
 
 }  // namespace protocol
 }  // namespace remoting