datachannel: Merge SendDataParams and DMT types with webrtc equivalent
cricket::SendDataParams is replaced by webrtc::SendDataParams.
cricket::DataMessageType is replaced by webrtc::DataMessageType.
The sid member from cricket::SendDataParams is now passed as an argument
to functions that used one when necessary.
Bug: webrtc:7484
Change-Id: Ia4a89c9651fb54ab9a084a6098d49130b6319e1b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217761
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33966}
diff --git a/media/sctp/dcsctp_transport.cc b/media/sctp/dcsctp_transport.cc
index 15d1742..687e750 100644
--- a/media/sctp/dcsctp_transport.cc
+++ b/media/sctp/dcsctp_transport.cc
@@ -32,7 +32,6 @@
namespace {
enum class WebrtcPPID : dcsctp::PPID::UnderlyingType {
- kNone = 0, // No protocol is specified.
// https://www.rfc-editor.org/rfc/rfc8832.html#section-8.1
kDCEP = 50,
// https://www.rfc-editor.org/rfc/rfc8831.html#section-8
@@ -44,34 +43,29 @@
kBinaryEmpty = 57,
};
-WebrtcPPID ToPPID(cricket::DataMessageType message_type, size_t size) {
+WebrtcPPID ToPPID(DataMessageType message_type, size_t size) {
switch (message_type) {
- case cricket::DMT_CONTROL:
+ case webrtc::DataMessageType::kControl:
return WebrtcPPID::kDCEP;
- case cricket::DMT_TEXT:
+ case webrtc::DataMessageType::kText:
return size > 0 ? WebrtcPPID::kString : WebrtcPPID::kStringEmpty;
- case cricket::DMT_BINARY:
+ case webrtc::DataMessageType::kBinary:
return size > 0 ? WebrtcPPID::kBinary : WebrtcPPID::kBinaryEmpty;
- default:
- RTC_NOTREACHED();
}
- return WebrtcPPID::kNone;
}
-absl::optional<cricket::DataMessageType> ToDataMessageType(dcsctp::PPID ppid) {
+absl::optional<DataMessageType> ToDataMessageType(dcsctp::PPID ppid) {
switch (static_cast<WebrtcPPID>(ppid.value())) {
- case WebrtcPPID::kNone:
- return cricket::DMT_NONE;
case WebrtcPPID::kDCEP:
- return cricket::DMT_CONTROL;
+ return webrtc::DataMessageType::kControl;
case WebrtcPPID::kString:
case WebrtcPPID::kStringPartial:
case WebrtcPPID::kStringEmpty:
- return cricket::DMT_TEXT;
+ return webrtc::DataMessageType::kText;
case WebrtcPPID::kBinary:
case WebrtcPPID::kBinaryPartial:
case WebrtcPPID::kBinaryEmpty:
- return cricket::DMT_BINARY;
+ return webrtc::DataMessageType::kBinary;
}
return absl::nullopt;
}
@@ -177,13 +171,14 @@
return true;
}
-bool DcSctpTransport::SendData(const cricket::SendDataParams& params,
+bool DcSctpTransport::SendData(int sid,
+ const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) {
RTC_DCHECK_RUN_ON(network_thread_);
- RTC_LOG(LS_VERBOSE) << debug_name_ << "->SendData(sid=" << params.sid
- << ", type=" << params.type
+ RTC_LOG(LS_VERBOSE) << debug_name_ << "->SendData(sid=" << sid
+ << ", type=" << static_cast<int>(params.type)
<< ", length=" << payload.size() << ").";
if (!socket_) {
@@ -216,7 +211,7 @@
}
dcsctp::DcSctpMessage message(
- dcsctp::StreamID(static_cast<uint16_t>(params.sid)),
+ dcsctp::StreamID(static_cast<uint16_t>(sid)),
dcsctp::PPID(static_cast<uint16_t>(ToPPID(params.type, payload.size()))),
std::move(message_payload));
diff --git a/media/sctp/dcsctp_transport.h b/media/sctp/dcsctp_transport.h
index 8e104da..f154c44 100644
--- a/media/sctp/dcsctp_transport.h
+++ b/media/sctp/dcsctp_transport.h
@@ -47,7 +47,8 @@
int max_message_size) override;
bool OpenStream(int sid) override;
bool ResetStream(int sid) override;
- bool SendData(const cricket::SendDataParams& params,
+ bool SendData(int sid,
+ const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result = nullptr) override;
bool ReadyToSendData() override;
diff --git a/media/sctp/sctp_transport_internal.h b/media/sctp/sctp_transport_internal.h
index dc8ac45..96c35ff 100644
--- a/media/sctp/sctp_transport_internal.h
+++ b/media/sctp/sctp_transport_internal.h
@@ -18,6 +18,7 @@
#include <string>
#include <vector>
+#include "api/transport/data_channel_transport_interface.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/thread.h"
// For SendDataParams/ReceiveDataParams.
@@ -101,7 +102,8 @@
// usrsctp that will then post the network interface).
// Returns true iff successful data somewhere on the send-queue/network.
// Uses |params.ssrc| as the SCTP sid.
- virtual bool SendData(const SendDataParams& params,
+ virtual bool SendData(int sid,
+ const webrtc::SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
SendDataResult* result = nullptr) = 0;
diff --git a/media/sctp/usrsctp_transport.cc b/media/sctp/usrsctp_transport.cc
index cfd5d6f..d43c017 100644
--- a/media/sctp/usrsctp_transport.cc
+++ b/media/sctp/usrsctp_transport.cc
@@ -129,46 +129,37 @@
}
// Get the PPID to use for the terminating fragment of this type.
-uint32_t GetPpid(cricket::DataMessageType type, size_t size) {
+uint32_t GetPpid(webrtc::DataMessageType type, size_t size) {
switch (type) {
- default:
- case cricket::DMT_NONE:
- return PPID_NONE;
- case cricket::DMT_CONTROL:
+ case webrtc::DataMessageType::kControl:
return PPID_CONTROL;
- case cricket::DMT_BINARY:
+ case webrtc::DataMessageType::kBinary:
return size > 0 ? PPID_BINARY_LAST : PPID_BINARY_EMPTY;
- case cricket::DMT_TEXT:
+ case webrtc::DataMessageType::kText:
return size > 0 ? PPID_TEXT_LAST : PPID_TEXT_EMPTY;
}
}
-bool GetDataMediaType(uint32_t ppid, cricket::DataMessageType* dest) {
+bool GetDataMediaType(uint32_t ppid, webrtc::DataMessageType* dest) {
RTC_DCHECK(dest != NULL);
switch (ppid) {
case PPID_BINARY_PARTIAL:
case PPID_BINARY_LAST:
case PPID_BINARY_EMPTY:
- *dest = cricket::DMT_BINARY;
+ *dest = webrtc::DataMessageType::kBinary;
return true;
case PPID_TEXT_PARTIAL:
case PPID_TEXT_LAST:
case PPID_TEXT_EMPTY:
- *dest = cricket::DMT_TEXT;
+ *dest = webrtc::DataMessageType::kText;
return true;
case PPID_CONTROL:
- *dest = cricket::DMT_CONTROL;
+ *dest = webrtc::DataMessageType::kControl;
return true;
-
- case PPID_NONE:
- *dest = cricket::DMT_NONE;
- return true;
-
- default:
- return false;
}
+ return false;
}
bool IsEmptyPPID(uint32_t ppid) {
@@ -212,11 +203,12 @@
// Creates the sctp_sendv_spa struct used for setting flags in the
// sctp_sendv() call.
-sctp_sendv_spa CreateSctpSendParams(const cricket::SendDataParams& params,
+sctp_sendv_spa CreateSctpSendParams(int sid,
+ const webrtc::SendDataParams& params,
size_t size) {
struct sctp_sendv_spa spa = {0};
spa.sendv_flags |= SCTP_SEND_SNDINFO_VALID;
- spa.sendv_sndinfo.snd_sid = params.sid;
+ spa.sendv_sndinfo.snd_sid = sid;
spa.sendv_sndinfo.snd_ppid = rtc::HostToNetwork32(GetPpid(params.type, size));
// Explicitly marking the EOR flag turns the usrsctp_sendv call below into a
// non atomic operation. This means that the sctp lib might only accept the
@@ -724,7 +716,8 @@
return true;
}
-bool UsrsctpTransport::SendData(const SendDataParams& params,
+bool UsrsctpTransport::SendData(int sid,
+ const webrtc::SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
SendDataResult* result) {
RTC_DCHECK_RUN_ON(network_thread_);
@@ -739,13 +732,13 @@
}
// Do not queue data to send on a closing stream.
- auto it = stream_status_by_sid_.find(params.sid);
+ auto it = stream_status_by_sid_.find(sid);
if (it == stream_status_by_sid_.end() || !it->second.is_open()) {
RTC_LOG(LS_WARNING)
<< debug_name_
<< "->SendData(...): "
"Not sending data because sid is unknown or closing: "
- << params.sid;
+ << sid;
if (result) {
*result = SDR_ERROR;
}
@@ -753,7 +746,7 @@
}
size_t payload_size = payload.size();
- OutgoingMessage message(payload, params);
+ OutgoingMessage message(payload, sid, params);
SendDataResult send_message_result = SendMessageInternal(&message);
if (result) {
*result = send_message_result;
@@ -782,17 +775,17 @@
RTC_LOG(LS_WARNING) << debug_name_
<< "->SendMessageInternal(...): "
"Not sending packet with sid="
- << message->send_params().sid
- << " len=" << message->size() << " before Start().";
+ << message->sid() << " len=" << message->size()
+ << " before Start().";
return SDR_ERROR;
}
- if (message->send_params().type != DMT_CONTROL) {
- auto it = stream_status_by_sid_.find(message->send_params().sid);
+ if (message->send_params().type != webrtc::DataMessageType::kControl) {
+ auto it = stream_status_by_sid_.find(message->sid());
if (it == stream_status_by_sid_.end()) {
RTC_LOG(LS_WARNING) << debug_name_
<< "->SendMessageInternal(...): "
"Not sending data because sid is unknown: "
- << message->send_params().sid;
+ << message->sid();
return SDR_ERROR;
}
}
@@ -804,8 +797,8 @@
}
// Send data using SCTP.
- sctp_sendv_spa spa =
- CreateSctpSendParams(message->send_params(), message->size());
+ sctp_sendv_spa spa = CreateSctpSendParams(
+ message->sid(), message->send_params(), message->size());
const void* data = message->data();
size_t data_length = message->size();
if (message->size() == 0) {
@@ -1081,7 +1074,7 @@
// https://w3c.github.io/webrtc-pc/#closing-procedure
return stream.second.need_outgoing_reset() &&
(!partial_outgoing_message_.has_value() ||
- partial_outgoing_message_.value().send_params().sid !=
+ partial_outgoing_message_.value().sid() !=
static_cast<int>(stream.first));
};
// Figure out how many streams need to be reset. We need to do this so we can
@@ -1158,7 +1151,7 @@
}
RTC_DCHECK_EQ(0u, partial_outgoing_message_->size());
- int sid = partial_outgoing_message_->send_params().sid;
+ int sid = partial_outgoing_message_->sid();
partial_outgoing_message_.reset();
// Send the queued stream reset if it was pending for this stream.
@@ -1314,7 +1307,7 @@
<< ", eor=" << ((flags & MSG_EOR) ? "y" : "n");
// Validate payload protocol identifier
- DataMessageType type = DMT_NONE;
+ webrtc::DataMessageType type;
if (!GetDataMediaType(ppid, &type)) {
// Unexpected PPID, dropping
RTC_LOG(LS_ERROR) << "Received an unknown PPID " << ppid
diff --git a/media/sctp/usrsctp_transport.h b/media/sctp/usrsctp_transport.h
index de018b9..5dcf57b 100644
--- a/media/sctp/usrsctp_transport.h
+++ b/media/sctp/usrsctp_transport.h
@@ -81,7 +81,8 @@
bool Start(int local_port, int remote_port, int max_message_size) override;
bool OpenStream(int sid) override;
bool ResetStream(int sid) override;
- bool SendData(const SendDataParams& params,
+ bool SendData(int sid,
+ const webrtc::SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
SendDataResult* result = nullptr) override;
bool ReadyToSendData() override;
@@ -113,8 +114,9 @@
class OutgoingMessage {
public:
OutgoingMessage(const rtc::CopyOnWriteBuffer& buffer,
- const SendDataParams& send_params)
- : buffer_(buffer), send_params_(send_params) {}
+ int sid,
+ const webrtc::SendDataParams& send_params)
+ : buffer_(buffer), sid_(sid), send_params_(send_params) {}
// Advances the buffer by the incremented amount. Must not advance further
// than the current data size.
@@ -127,11 +129,13 @@
const void* data() const { return buffer_.data() + offset_; }
- SendDataParams send_params() const { return send_params_; }
+ int sid() const { return sid_; }
+ webrtc::SendDataParams send_params() const { return send_params_; }
private:
const rtc::CopyOnWriteBuffer buffer_;
- const SendDataParams send_params_;
+ int sid_;
+ const webrtc::SendDataParams send_params_;
size_t offset_ = 0;
};
diff --git a/media/sctp/usrsctp_transport_reliability_unittest.cc b/media/sctp/usrsctp_transport_reliability_unittest.cc
index 407dd8d..104e320 100644
--- a/media/sctp/usrsctp_transport_reliability_unittest.cc
+++ b/media/sctp/usrsctp_transport_reliability_unittest.cc
@@ -133,23 +133,19 @@
};
/**
- * A helper class to send specified number of messages
- * over UsrsctpTransport with SCTP reliability settings
- * provided by user. The reliability settings are specified
- * by passing a template instance of SendDataParams.
- * When .sid field inside SendDataParams is specified to
- * negative value it means that actual .sid will be
- * assigned by sender itself, .sid will be assigned from
- * range [cricket::kMinSctpSid; cricket::kMaxSctpSid].
- * The wide range of sids are used to possibly trigger
- * more execution paths inside usrsctp.
+ * A helper class to send specified number of messages over UsrsctpTransport
+ * with SCTP reliability settings provided by user. The reliability settings are
+ * specified by passing a template instance of SendDataParams. The sid will be
+ * assigned by sender itself and will be assigned from range
+ * [cricket::kMinSctpSid; cricket::kMaxSctpSid]. The wide range of sids are used
+ * to possibly trigger more execution paths inside usrsctp.
*/
class SctpDataSender final {
public:
SctpDataSender(rtc::Thread* thread,
cricket::UsrsctpTransport* transport,
uint64_t target_messages_count,
- cricket::SendDataParams send_params,
+ webrtc::SendDataParams send_params,
uint32_t sender_id)
: thread_(thread),
transport_(transport),
@@ -200,14 +196,12 @@
<< target_messages_count_;
}
- cricket::SendDataParams params(send_params_);
- if (params.sid < 0) {
- params.sid = cricket::kMinSctpSid +
- (num_messages_sent_ % cricket::kMaxSctpStreams);
- }
+ webrtc::SendDataParams params(send_params_);
+ int sid =
+ cricket::kMinSctpSid + (num_messages_sent_ % cricket::kMaxSctpStreams);
cricket::SendDataResult result;
- transport_->SendData(params, payload_, &result);
+ transport_->SendData(sid, params, payload_, &result);
switch (result) {
case cricket::SDR_BLOCK:
// retry after timeout
@@ -233,7 +227,7 @@
rtc::Thread* const thread_;
cricket::UsrsctpTransport* const transport_;
const uint64_t target_messages_count_;
- const cricket::SendDataParams send_params_;
+ const webrtc::SendDataParams send_params_;
const uint32_t sender_id_;
rtc::CopyOnWriteBuffer payload_{std::string(1400, '.').c_str(), 1400};
std::atomic<bool> started_ ATOMIC_VAR_INIT(false);
@@ -329,7 +323,7 @@
uint32_t messages_count,
uint8_t packet_loss_percents,
uint16_t avg_send_delay_millis,
- cricket::SendDataParams send_params)
+ webrtc::SendDataParams send_params)
: id_(id),
port1_(port1),
port2_(port2),
@@ -582,7 +576,7 @@
const uint32_t messages_count_;
const uint8_t packet_loss_percents_;
const uint16_t avg_send_delay_millis_;
- const cricket::SendDataParams send_params_;
+ const webrtc::SendDataParams send_params_;
RTC_DISALLOW_COPY_AND_ASSIGN(SctpPingPong);
};
@@ -643,8 +637,7 @@
static_assert(wait_timeout > 0,
"Timeout computation must produce positive value");
- cricket::SendDataParams send_params;
- send_params.sid = -1;
+ webrtc::SendDataParams send_params;
send_params.ordered = true;
SctpPingPong test(1, kTransport1Port, kTransport2Port, thread1.get(),
@@ -678,8 +671,7 @@
static_assert(wait_timeout > 0,
"Timeout computation must produce positive value");
- cricket::SendDataParams send_params;
- send_params.sid = -1;
+ webrtc::SendDataParams send_params;
send_params.ordered = true;
SctpPingPong test(1, kTransport1Port, kTransport2Port, thread1.get(),
@@ -714,8 +706,7 @@
static_assert(wait_timeout > 0,
"Timeout computation must produce positive value");
- cricket::SendDataParams send_params;
- send_params.sid = -1;
+ webrtc::SendDataParams send_params;
send_params.ordered = false;
send_params.max_rtx_count = std::numeric_limits<uint16_t>::max();
send_params.max_rtx_ms = std::numeric_limits<uint16_t>::max();
@@ -750,8 +741,7 @@
DISABLED_AllMessagesAreDeliveredOverLossyConnectionConcurrentTests) {
ThreadPool pool(16);
- cricket::SendDataParams send_params;
- send_params.sid = -1;
+ webrtc::SendDataParams send_params;
send_params.ordered = true;
constexpr uint32_t base_sctp_port = 5000;
diff --git a/media/sctp/usrsctp_transport_unittest.cc b/media/sctp/usrsctp_transport_unittest.cc
index f75cb4a..59e9c59 100644
--- a/media/sctp/usrsctp_transport_unittest.cc
+++ b/media/sctp/usrsctp_transport_unittest.cc
@@ -185,12 +185,11 @@
const std::string& msg,
SendDataResult* result,
bool ordered = false) {
- SendDataParams params;
- params.sid = sid;
+ webrtc::SendDataParams params;
params.ordered = ordered;
- return chan->SendData(params, rtc::CopyOnWriteBuffer(&msg[0], msg.length()),
- result);
+ return chan->SendData(
+ sid, params, rtc::CopyOnWriteBuffer(&msg[0], msg.length()), result);
}
bool ReceivedData(const SctpFakeDataReceiver* recv,
@@ -599,15 +598,14 @@
SetupConnectedTransportsWithTwoStreams();
SendDataResult result;
- SendDataParams params;
- params.sid = 1;
+ webrtc::SendDataParams params;
params.ordered = GetParam();
std::vector<char> buffer(1024 * 64, 0);
for (size_t i = 0; i < 100; ++i) {
transport1()->SendData(
- params, rtc::CopyOnWriteBuffer(&buffer[0], buffer.size()), &result);
+ 1, params, rtc::CopyOnWriteBuffer(&buffer[0], buffer.size()), &result);
if (result == SDR_BLOCK)
break;
}
@@ -626,15 +624,15 @@
fake_dtls1()->SetWritable(false);
// Send messages until we get EWOULDBLOCK.
static const size_t kMaxMessages = 1024;
- SendDataParams params;
- params.sid = 1;
+ webrtc::SendDataParams params;
params.ordered = GetParam();
rtc::CopyOnWriteBuffer buf(1024);
memset(buf.MutableData(), 0, 1024);
SendDataResult result;
size_t message_count = 0;
for (; message_count < kMaxMessages; ++message_count) {
- if (!transport1()->SendData(params, buf, &result) && result == SDR_BLOCK) {
+ if (!transport1()->SendData(1, params, buf, &result) &&
+ result == SDR_BLOCK) {
break;
}
}
