Skip to content

Commit

Permalink
Version 2 "Refactoring DataContentDescription class"
Browse files Browse the repository at this point in the history
(substantial changes since version 1)

This CL splits the cricket::DataContentDescription class into
two classes: cricket::RtpDataContentDescription (used for RTP data)
and cricket::SctpDataContentDescription (used for SCTP only).

SctpDataContentDescription no longer inherits from
MediaContentDescriptionImpl, and no longer contains "codecs".

Due to usage of internal interfaces by consumers, shimming the old
DataContentDescription API is needed.

A new cricket::DataContentDescription class is defined, which is
a shim over RtpDataContentDescription and SctpDataContentDescription.
It exposes as little functionality as possible, but supports the
concerned consumer's usage

Design document:
https://docs.google.com/document/d/1H5LfQxJA2ikMWTQ8FZ3_GAmaXM7knfVQWiSz6ph8VQ0/edit#

Version 1 reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132700

Bug: webrtc:10358
Change-Id: Icf95fb7308244d6f2ebfdb403aaffc544e358580
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133900
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27853}
Harald Alvestrand authored and Commit Bot committed May 5, 2019
1 parent 2390a13 commit 14b2758
Showing 21 changed files with 1,344 additions and 486 deletions.
16 changes: 8 additions & 8 deletions media/base/codec.cc
Original file line number Diff line number Diff line change
@@ -334,22 +334,22 @@ bool VideoCodec::ValidateCodecFormat() const {
return true;
}

DataCodec::DataCodec(int id, const std::string& name)
RtpDataCodec::RtpDataCodec(int id, const std::string& name)
: Codec(id, name, kDataCodecClockrate) {}

DataCodec::DataCodec() : Codec() {
RtpDataCodec::RtpDataCodec() : Codec() {
clockrate = kDataCodecClockrate;
}

DataCodec::DataCodec(const DataCodec& c) = default;
DataCodec::DataCodec(DataCodec&& c) = default;
DataCodec& DataCodec::operator=(const DataCodec& c) = default;
DataCodec& DataCodec::operator=(DataCodec&& c) = default;
RtpDataCodec::RtpDataCodec(const RtpDataCodec& c) = default;
RtpDataCodec::RtpDataCodec(RtpDataCodec&& c) = default;
RtpDataCodec& RtpDataCodec::operator=(const RtpDataCodec& c) = default;
RtpDataCodec& RtpDataCodec::operator=(RtpDataCodec&& c) = default;

std::string DataCodec::ToString() const {
std::string RtpDataCodec::ToString() const {
char buf[256];
rtc::SimpleStringBuilder sb(buf);
sb << "DataCodec[" << id << ":" << name << "]";
sb << "RtpDataCodec[" << id << ":" << name << "]";
return sb.str();
}

20 changes: 12 additions & 8 deletions media/base/codec.h
Original file line number Diff line number Diff line change
@@ -192,19 +192,23 @@ struct RTC_EXPORT VideoCodec : public Codec {
void SetDefaultParameters();
};

struct DataCodec : public Codec {
DataCodec(int id, const std::string& name);
DataCodec();
DataCodec(const DataCodec& c);
DataCodec(DataCodec&& c);
~DataCodec() override = default;
struct RtpDataCodec : public Codec {
RtpDataCodec(int id, const std::string& name);
RtpDataCodec();
RtpDataCodec(const RtpDataCodec& c);
RtpDataCodec(RtpDataCodec&& c);
~RtpDataCodec() override = default;

DataCodec& operator=(const DataCodec& c);
DataCodec& operator=(DataCodec&& c);
RtpDataCodec& operator=(const RtpDataCodec& c);
RtpDataCodec& operator=(RtpDataCodec&& c);

std::string ToString() const;
};

// For backwards compatibility
// TODO(bugs.webrtc.org/10597): Remove when no longer needed.
typedef RtpDataCodec DataCodec;

// Get the codec setting associated with |payload_type|. If there
// is no codec associated with that payload type it returns nullptr.
template <class Codec>
3 changes: 1 addition & 2 deletions media/base/rtp_data_engine.h
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
#include <string>
#include <vector>

#include "media/base/codec.h"
#include "media/base/media_channel.h"
#include "media/base/media_constants.h"
#include "media/base/media_engine.h"
@@ -26,8 +27,6 @@ class DataRateLimiter;

namespace cricket {

struct DataCodec;

class RtpDataEngine : public DataEngineInterface {
public:
RtpDataEngine();
8 changes: 8 additions & 0 deletions pc/BUILD.gn
Original file line number Diff line number Diff line change
@@ -72,6 +72,7 @@ rtc_static_library("rtc_pc_base") {
]

deps = [
":media_protocol_names",
"../api:array_view",
"../api:audio_options_api",
"../api:call_api",
@@ -121,6 +122,13 @@ rtc_source_set("rtc_pc") {
]
}

rtc_source_set("media_protocol_names") {
sources = [
"media_protocol_names.cc",
"media_protocol_names.h",
]
}

rtc_static_library("peerconnection") {
visibility = [ "*" ]
cflags = []
11 changes: 8 additions & 3 deletions pc/channel.cc
Original file line number Diff line number Diff line change
@@ -1143,7 +1143,7 @@ bool RtpDataChannel::SendData(const SendDataParams& params,
}

bool RtpDataChannel::CheckDataChannelTypeFromContent(
const DataContentDescription* content,
const RtpDataContentDescription* content,
std::string* error_desc) {
bool is_sctp = ((content->protocol() == kMediaProtocolSctp) ||
(content->protocol() == kMediaProtocolDtlsSctp));
@@ -1169,7 +1169,7 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content,
return false;
}

const DataContentDescription* data = content->as_data();
const RtpDataContentDescription* data = content->as_rtp_data();

if (!CheckDataChannelTypeFromContent(data, error_desc)) {
return false;
@@ -1223,7 +1223,12 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content,
return false;
}

const DataContentDescription* data = content->as_data();
const RtpDataContentDescription* data = content->as_rtp_data();

if (!data) {
RTC_LOG(LS_INFO) << "Accepting and ignoring non-RTP content description";
return true;
}

// If the remote data doesn't have codecs, it must be empty, so ignore it.
if (!data->has_codecs()) {
2 changes: 1 addition & 1 deletion pc/channel.h
Original file line number Diff line number Diff line change
@@ -518,7 +518,7 @@ class RtpDataChannel : public BaseChannel {

// overrides from BaseChannel
// Checks that data channel type is RTP.
bool CheckDataChannelTypeFromContent(const DataContentDescription* content,
bool CheckDataChannelTypeFromContent(const RtpDataContentDescription* content,
std::string* error_desc);
bool SetLocalContent_w(const MediaContentDescription* content,
webrtc::SdpType type,
12 changes: 6 additions & 6 deletions pc/channel_unittest.cc
Original file line number Diff line number Diff line change
@@ -94,8 +94,8 @@ class VideoTraits : public Traits<cricket::VideoChannel,

class DataTraits : public Traits<cricket::RtpDataChannel,
cricket::FakeDataMediaChannel,
cricket::DataContentDescription,
cricket::DataCodec,
cricket::RtpDataContentDescription,
cricket::RtpDataCodec,
cricket::DataMediaInfo,
cricket::DataOptions> {};

@@ -2308,15 +2308,15 @@ void ChannelTest<DataTraits>::CreateContent(
int flags,
const cricket::AudioCodec& audio_codec,
const cricket::VideoCodec& video_codec,
cricket::DataContentDescription* data) {
cricket::RtpDataContentDescription* data) {
data->AddCodec(kGoogleDataCodec);
data->set_rtcp_mux((flags & RTCP_MUX) != 0);
}

template <>
void ChannelTest<DataTraits>::CopyContent(
const cricket::DataContentDescription& source,
cricket::DataContentDescription* data) {
const cricket::RtpDataContentDescription& source,
cricket::RtpDataContentDescription* data) {
*data = source;
}

@@ -2330,7 +2330,7 @@ template <>
void ChannelTest<DataTraits>::AddLegacyStreamInContent(
uint32_t ssrc,
int flags,
cricket::DataContentDescription* data) {
cricket::RtpDataContentDescription* data) {
data->AddLegacyStream(ssrc);
}

5 changes: 3 additions & 2 deletions pc/jsep_transport_controller_unittest.cc
Original file line number Diff line number Diff line change
@@ -175,8 +175,9 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
cricket::IceMode ice_mode,
cricket::ConnectionRole conn_role,
rtc::scoped_refptr<rtc::RTCCertificate> cert) {
std::unique_ptr<cricket::DataContentDescription> data(
new cricket::DataContentDescription());
RTC_CHECK(protocol_type == cricket::MediaProtocolType::kSctp);
std::unique_ptr<cricket::SctpDataContentDescription> data(
new cricket::SctpDataContentDescription());
data->set_rtcp_mux(true);
description->AddContent(mid, protocol_type,
/*rejected=*/false, data.release());
41 changes: 41 additions & 0 deletions pc/media_protocol_names.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2019 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/

#include "pc/media_protocol_names.h"

namespace cricket {

const char kMediaProtocolRtpPrefix[] = "RTP/";

const char kMediaProtocolSctp[] = "SCTP";
const char kMediaProtocolDtlsSctp[] = "DTLS/SCTP";
const char kMediaProtocolUdpDtlsSctp[] = "UDP/DTLS/SCTP";
const char kMediaProtocolTcpDtlsSctp[] = "TCP/DTLS/SCTP";

bool IsDtlsSctp(const std::string& protocol) {
return protocol == kMediaProtocolDtlsSctp ||
protocol == kMediaProtocolUdpDtlsSctp ||
protocol == kMediaProtocolTcpDtlsSctp;
}

bool IsPlainSctp(const std::string& protocol) {
return protocol == kMediaProtocolSctp;
}

bool IsRtpProtocol(const std::string& protocol) {
return protocol.empty() ||
(protocol.find(cricket::kMediaProtocolRtpPrefix) != std::string::npos);
}

bool IsSctpProtocol(const std::string& protocol) {
return IsPlainSctp(protocol) || IsDtlsSctp(protocol);
}

} // namespace cricket
35 changes: 35 additions & 0 deletions pc/media_protocol_names.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright 2019 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/

#ifndef PC_MEDIA_PROTOCOL_NAMES_H_
#define PC_MEDIA_PROTOCOL_NAMES_H_

#include <string>

namespace cricket {

// Names or name prefixes of protocols as defined by SDP specifications.
extern const char kMediaProtocolRtpPrefix[];
extern const char kMediaProtocolSctp[];
extern const char kMediaProtocolDtlsSctp[];
extern const char kMediaProtocolUdpDtlsSctp[];
extern const char kMediaProtocolTcpDtlsSctp[];

bool IsDtlsSctp(const std::string& protocol);
bool IsPlainSctp(const std::string& protocol);

// Returns true if the given media section protocol indicates use of RTP.
bool IsRtpProtocol(const std::string& protocol);
// Returns true if the given media section protocol indicates use of SCTP.
bool IsSctpProtocol(const std::string& protocol);

} // namespace cricket

#endif // PC_MEDIA_PROTOCOL_NAMES_H_
Loading

0 comments on commit 14b2758

Please sign in to comment.