Introduce webrtc::SdpType, the chosen enum for offer/pranswer/answer
This change introduces a new method |GetType()| in
SessionDescriptionInterface which returns an enum for the SDP type
rather than a string. Additionally, new overloads were added for
CreateSessionDescription to take SdpType instead of a type string.
Bug: webrtc:8613
Change-Id: I52b342f12155daf8d623646b0c21b7562f69d101
Reviewed-on: https://webrtc-review.googlesource.com/29380
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21100}diff --git a/api/jsep.h b/api/jsep.h
index acfffd4..c05f382 100644
--- a/api/jsep.h
+++ b/api/jsep.h
@@ -22,9 +22,11 @@
#include <stddef.h>
+#include <memory>
#include <string>
#include <vector>
+#include "api/optional.h"
#include "rtc_base/refcount.h"
namespace cricket {
@@ -55,7 +57,7 @@
// m= section in SDP, which identifies the m= section.
virtual std::string sdp_mid() const = 0;
// This indicates the index (starting at zero) of m= section this candidate
- // is assocated with. Needed when an endpoint doesn't support MIDs.
+ // is associated with. Needed when an endpoint doesn't support MIDs.
virtual int sdp_mline_index() const = 0;
// Only for use internally.
virtual const cricket::Candidate& candidate() const = 0;
@@ -86,6 +88,26 @@
virtual const IceCandidateInterface* at(size_t index) const = 0;
};
+// Enum that describes the type of the SessionDescriptionInterface.
+// Corresponds to RTCSdpType in the WebRTC specification.
+// https://w3c.github.io/webrtc-pc/#dom-rtcsdptype
+enum class SdpType {
+ kOffer, // Description must be treated as an SDP offer.
+ kPrAnswer, // Description must be treated as an SDP answer, but not a final
+ // answer.
+ kAnswer // Description must be treated as an SDP final answer, and the offer-
+ // answer exchange must be considered complete after receiving this.
+};
+
+// Returns the string form of the given SDP type. String forms are defined in
+// SessionDescriptionInterface.
+const char* SdpTypeToString(SdpType type);
+
+// Returns the SdpType from its string form. The string form can be one of the
+// constants defined in SessionDescriptionInterface. Passing in any other string
+// results in nullopt.
+rtc::Optional<SdpType> SdpTypeFromString(const std::string& type_str);
+
// Class representation of an SDP session description.
//
// An instance of this interface is supposed to be owned by one class at a time
@@ -94,7 +116,7 @@
// An instance can be created by CreateSessionDescription.
class SessionDescriptionInterface {
public:
- // Supported types:
+ // String representations of the supported SDP types.
static const char kOffer[];
static const char kPrAnswer[];
static const char kAnswer[];
@@ -110,7 +132,14 @@
virtual std::string session_id() const = 0;
virtual std::string session_version() const = 0;
+ // Returns the type of this session description as an SdpType. Descriptions of
+ // the various types are found in the SdpType documentation.
+ // TODO(steveanton): Remove default implementation once Chromium has been
+ // updated.
+ virtual SdpType GetType() const;
+
// kOffer/kPrAnswer/kAnswer
+ // TODO(steveanton): Remove this in favor of |GetType| that returns SdpType.
virtual std::string type() const = 0;
// Adds the specified candidate to the description.
@@ -143,10 +172,24 @@
// Creates a SessionDescriptionInterface based on the SDP string and the type.
// Returns null if the sdp string can't be parsed or the type is unsupported.
// |error| may be null.
+// TODO(steveanton): This function is deprecated. Please use the functions below
+// which take an SdpType enum instead. Remove this once it is no longer used.
SessionDescriptionInterface* CreateSessionDescription(const std::string& type,
const std::string& sdp,
SdpParseError* error);
+// Creates a SessionDescriptionInterface based on the SDP string and the type.
+// Returns null if the SDP string cannot be parsed.
+// If using the signature with |error_out|, details of the parsing error may be
+// written to |error_out| if it is not null.
+std::unique_ptr<SessionDescriptionInterface> CreateSessionDescription(
+ SdpType type,
+ const std::string& sdp);
+std::unique_ptr<SessionDescriptionInterface> CreateSessionDescription(
+ SdpType type,
+ const std::string& sdp,
+ SdpParseError* error_out);
+
// CreateOffer and CreateAnswer callback interface.
class CreateSessionDescriptionObserver : public rtc::RefCountInterface {
public:
diff --git a/api/jsepsessiondescription.h b/api/jsepsessiondescription.h
index 6b115ee..70bb277 100644
--- a/api/jsepsessiondescription.h
+++ b/api/jsepsessiondescription.h
@@ -32,6 +32,8 @@
// Implementation of SessionDescriptionInterface.
class JsepSessionDescription : public SessionDescriptionInterface {
public:
+ explicit JsepSessionDescription(SdpType type);
+ // TODO(steveanton): Remove this once callers have switched to SdpType.
explicit JsepSessionDescription(const std::string& type);
virtual ~JsepSessionDescription();
@@ -54,11 +56,9 @@
virtual std::string session_version() const {
return session_version_;
}
- virtual std::string type() const {
- return type_;
- }
+ virtual SdpType GetType() const { return type_; }
+ virtual std::string type() const { return SdpTypeToString(type_); }
// Allows changing the type. Used for testing.
- void set_type(const std::string& type) { type_ = type; }
virtual bool AddCandidate(const IceCandidateInterface* candidate);
virtual size_t RemoveCandidates(
const std::vector<cricket::Candidate>& candidates);
@@ -74,7 +74,7 @@
std::unique_ptr<cricket::SessionDescription> description_;
std::string session_id_;
std::string session_version_;
- std::string type_;
+ SdpType type_;
std::vector<JsepCandidateCollection> candidate_collection_;
bool GetMediasectionIndex(const IceCandidateInterface* candidate,
diff --git a/pc/jsepsessiondescription.cc b/pc/jsepsessiondescription.cc
index e6b6d32..5e6a1cb 100644
--- a/pc/jsepsessiondescription.cc
+++ b/pc/jsepsessiondescription.cc
@@ -12,46 +12,31 @@
#include <memory>
+#include "p2p/base/port.h"
#include "pc/mediasession.h"
#include "pc/webrtcsdp.h"
-#include "p2p/base/port.h"
#include "rtc_base/arraysize.h"
+#include "rtc_base/ptr_util.h"
#include "rtc_base/stringencode.h"
using cricket::SessionDescription;
namespace webrtc {
-
-static const char* kSupportedTypes[] = {
- JsepSessionDescription::kOffer,
- JsepSessionDescription::kPrAnswer,
- JsepSessionDescription::kAnswer
-};
-
-static bool IsTypeSupported(const std::string& type) {
- bool type_supported = false;
- for (size_t i = 0; i < arraysize(kSupportedTypes); ++i) {
- if (kSupportedTypes[i] == type) {
- type_supported = true;
- break;
- }
- }
- return type_supported;
-}
+namespace {
// RFC 5245
// It is RECOMMENDED that default candidates be chosen based on the
// likelihood of those candidates to work with the peer that is being
// contacted. It is RECOMMENDED that relayed > reflexive > host.
-static const int kPreferenceUnknown = 0;
-static const int kPreferenceHost = 1;
-static const int kPreferenceReflexive = 2;
-static const int kPreferenceRelayed = 3;
+constexpr int kPreferenceUnknown = 0;
+constexpr int kPreferenceHost = 1;
+constexpr int kPreferenceReflexive = 2;
+constexpr int kPreferenceRelayed = 3;
-static const char kDummyAddress[] = "0.0.0.0";
-static const int kDummyPort = 9;
+constexpr char kDummyAddress[] = "0.0.0.0";
+constexpr int kDummyPort = 9;
-static int GetCandidatePreferenceFromType(const std::string& type) {
+int GetCandidatePreferenceFromType(const std::string& type) {
int preference = kPreferenceUnknown;
if (type == cricket::LOCAL_PORT_TYPE) {
preference = kPreferenceHost;
@@ -67,7 +52,7 @@
// Update the connection address for the MediaContentDescription based on the
// candidates.
-static void UpdateConnectionAddress(
+void UpdateConnectionAddress(
const JsepCandidateCollection& candidate_collection,
cricket::ContentDescription* content_description) {
int port = kDummyPort;
@@ -107,6 +92,8 @@
->set_connection_address(connection_addr);
}
+} // namespace
+
const char SessionDescriptionInterface::kOffer[] = "offer";
const char SessionDescriptionInterface::kPrAnswer[] = "pranswer";
const char SessionDescriptionInterface::kAnswer[] = "answer";
@@ -114,23 +101,85 @@
const int JsepSessionDescription::kDefaultVideoCodecId = 100;
const char JsepSessionDescription::kDefaultVideoCodecName[] = "VP8";
+const char* SdpTypeToString(SdpType type) {
+ switch (type) {
+ case SdpType::kOffer:
+ return SessionDescriptionInterface::kOffer;
+ case SdpType::kPrAnswer:
+ return SessionDescriptionInterface::kPrAnswer;
+ case SdpType::kAnswer:
+ return SessionDescriptionInterface::kAnswer;
+ }
+ return "";
+}
+
+rtc::Optional<SdpType> SdpTypeFromString(const std::string& type_str) {
+ if (type_str == SessionDescriptionInterface::kOffer) {
+ return SdpType::kOffer;
+ } else if (type_str == SessionDescriptionInterface::kPrAnswer) {
+ return SdpType::kPrAnswer;
+ } else if (type_str == SessionDescriptionInterface::kAnswer) {
+ return SdpType::kAnswer;
+ } else {
+ return rtc::nullopt;
+ }
+}
+
+// TODO(steveanton): Remove this default implementation once Chromium has been
+// updated.
+SdpType SessionDescriptionInterface::GetType() const {
+ rtc::Optional<SdpType> maybe_type = SdpTypeFromString(type());
+ if (maybe_type) {
+ return *maybe_type;
+ } else {
+ RTC_LOG(LS_WARNING) << "Default implementation of "
+ "SessionDescriptionInterface::GetType does not "
+ "recognize the result from type(), returning "
+ "kOffer.";
+ return SdpType::kOffer;
+ }
+}
+
SessionDescriptionInterface* CreateSessionDescription(const std::string& type,
const std::string& sdp,
SdpParseError* error) {
- if (!IsTypeSupported(type)) {
- return NULL;
+ rtc::Optional<SdpType> maybe_type = SdpTypeFromString(type);
+ if (!maybe_type) {
+ return nullptr;
}
- JsepSessionDescription* jsep_desc = new JsepSessionDescription(type);
- if (!SdpDeserialize(sdp, jsep_desc, error)) {
- delete jsep_desc;
- return NULL;
- }
- return jsep_desc;
+ return CreateSessionDescription(*maybe_type, sdp, error).release();
}
-JsepSessionDescription::JsepSessionDescription(const std::string& type)
- : type_(type) {
+std::unique_ptr<SessionDescriptionInterface> CreateSessionDescription(
+ SdpType type,
+ const std::string& sdp) {
+ return CreateSessionDescription(type, sdp, nullptr);
+}
+
+std::unique_ptr<SessionDescriptionInterface> CreateSessionDescription(
+ SdpType type,
+ const std::string& sdp,
+ SdpParseError* error_out) {
+ auto jsep_desc = rtc::MakeUnique<JsepSessionDescription>(type);
+ if (!SdpDeserialize(sdp, jsep_desc.get(), error_out)) {
+ return nullptr;
+ }
+ return std::move(jsep_desc);
+}
+
+JsepSessionDescription::JsepSessionDescription(SdpType type) : type_(type) {}
+
+JsepSessionDescription::JsepSessionDescription(const std::string& type) {
+ rtc::Optional<SdpType> maybe_type = SdpTypeFromString(type);
+ if (maybe_type) {
+ type_ = *maybe_type;
+ } else {
+ RTC_LOG(LS_WARNING)
+ << "JsepSessionDescription constructed with invalid type string: "
+ << type << ". Assuming it is an offer.";
+ type_ = SdpType::kOffer;
+ }
}
JsepSessionDescription::~JsepSessionDescription() {}
diff --git a/pc/jsepsessiondescription_unittest.cc b/pc/jsepsessiondescription_unittest.cc
index be24dcd..27b2cf1 100644
--- a/pc/jsepsessiondescription_unittest.cc
+++ b/pc/jsepsessiondescription_unittest.cc
@@ -22,10 +22,12 @@
#include "rtc_base/gunit.h"
#include "rtc_base/stringencode.h"
+using ::testing::Values;
using webrtc::IceCandidateCollection;
using webrtc::IceCandidateInterface;
using webrtc::JsepIceCandidate;
using webrtc::JsepSessionDescription;
+using webrtc::SdpType;
using webrtc::SessionDescriptionInterface;
static const char kCandidateUfrag[] = "ufrag";
@@ -405,3 +407,20 @@
ASSERT_TRUE(jsep_desc_->RemoveCandidates(candidates));
EXPECT_EQ("0.0.0.0:9", media_desc->connection_address().ToString());
}
+
+class EnumerateAllSdpTypesTest : public ::testing::Test,
+ public ::testing::WithParamInterface<SdpType> {
+};
+
+TEST_P(EnumerateAllSdpTypesTest, TestIdentity) {
+ SdpType type = GetParam();
+
+ const char* str = webrtc::SdpTypeToString(type);
+ EXPECT_EQ(type, webrtc::SdpTypeFromString(str));
+}
+
+INSTANTIATE_TEST_CASE_P(JsepSessionDescriptionTest,
+ EnumerateAllSdpTypesTest,
+ Values(SdpType::kOffer,
+ SdpType::kPrAnswer,
+ SdpType::kAnswer));
diff --git a/pc/peerconnection_crypto_unittest.cc b/pc/peerconnection_crypto_unittest.cc
index 5d5296b..ab341f5 100644
--- a/pc/peerconnection_crypto_unittest.cc
+++ b/pc/peerconnection_crypto_unittest.cc
@@ -513,19 +513,6 @@
// CreateOffer/CreateAnswer calls are made while waiting for the certificate,
// they all finish after the certificate is generated.
-// Whether the test will call CreateOffer or CreateAnswer.
-enum class SdpType { kOffer, kAnswer };
-std::ostream& operator<<(std::ostream& out, SdpType value) {
- switch (value) {
- case SdpType::kOffer:
- return out << "offer";
- case SdpType::kAnswer:
- return out << "answer";
- default:
- return out << "unknown";
- }
-}
-
// Whether the certificate will be generated before calling CreateOffer or
// while CreateOffer is executing.
enum class CertGenTime { kBefore, kDuring };
