diff --git a/media/BUILD.gn b/media/BUILD.gn index 9d254b7c99..c85a037e67 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -81,6 +81,7 @@ rtc_library("rtc_media_base") { "../api/crypto:frame_decryptor_interface", "../api/crypto:frame_encryptor_interface", "../api/crypto:options", + "../api/transport:datagram_transport_interface", "../api/transport:stun_types", "../api/transport:webrtc_key_value_config", "../api/transport/rtp:rtp_source", @@ -381,6 +382,7 @@ rtc_library("rtc_media_engine_defaults") { rtc_source_set("rtc_data_sctp_transport_internal") { sources = [ "sctp/sctp_transport_internal.h" ] deps = [ + "../api/transport:datagram_transport_interface", "../media:rtc_media_base", "../p2p:rtc_p2p", "../rtc_base:rtc_base_approved", diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 8ebaddbfd8..a4a925e912 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -26,6 +26,7 @@ #include "api/media_stream_interface.h" #include "api/rtc_error.h" #include "api/rtp_parameters.h" +#include "api/transport/data_channel_transport_interface.h" #include "api/transport/rtp/rtp_source.h" #include "api/video/video_content_type.h" #include "api/video/video_sink_interface.h" @@ -892,15 +893,6 @@ class VideoMediaChannel : public MediaChannel, public Delayable { virtual std::vector GetSources(uint32_t ssrc) const = 0; }; -enum DataMessageType { - // Chrome-Internal use only. See SctpDataMediaChannel for the actual PPID - // values. - DMT_NONE = 0, - DMT_CONTROL = 1, - DMT_BINARY = 2, - DMT_TEXT = 3, -}; - // Info about data received in DataMediaChannel. For use in // DataMediaChannel::SignalDataReceived and in all of the signals that // signal fires, on up the chain. @@ -909,28 +901,11 @@ struct ReceiveDataParams { // SCTP data channels use SIDs. int sid = 0; // The type of message (binary, text, or control). - DataMessageType type = DMT_TEXT; + webrtc::DataMessageType type = webrtc::DataMessageType::kText; // A per-stream value incremented per packet in the stream. int seq_num = 0; }; -struct SendDataParams { - // The in-packet stream indentifier. - int sid = 0; - // The type of message (binary, text, or control). - DataMessageType type = DMT_TEXT; - - // For SCTP, whether to send messages flagged as ordered or not. - // If false, messages can be received out of order. - bool ordered = false; - // Provide partial reliability by resending up to this many times. Either - // count or millis is supported, not both at the same time. - absl::optional max_rtx_count; - // Provide partial reliability by resending for up to this many milliseconds. - // Either count or millis is supported, not both at the same time. - absl::optional max_rtx_ms; -}; - enum SendDataResult { SDR_SUCCESS, SDR_ERROR, SDR_BLOCK }; } // namespace cricket diff --git a/media/sctp/dcsctp_transport.cc b/media/sctp/dcsctp_transport.cc index 15d1742eb0..687e7503e6 100644 --- a/media/sctp/dcsctp_transport.cc +++ b/media/sctp/dcsctp_transport.cc @@ -32,7 +32,6 @@ namespace webrtc { 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 @@ enum class WebrtcPPID : dcsctp::PPID::UnderlyingType { 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 ToDataMessageType(dcsctp::PPID ppid) { +absl::optional ToDataMessageType(dcsctp::PPID ppid) { switch (static_cast(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 @@ bool DcSctpTransport::ResetStream(int sid) { 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(params.type) << ", length=" << payload.size() << ")."; if (!socket_) { @@ -216,7 +211,7 @@ bool DcSctpTransport::SendData(const cricket::SendDataParams& params, } dcsctp::DcSctpMessage message( - dcsctp::StreamID(static_cast(params.sid)), + dcsctp::StreamID(static_cast(sid)), dcsctp::PPID(static_cast(ToPPID(params.type, payload.size()))), std::move(message_payload)); diff --git a/media/sctp/dcsctp_transport.h b/media/sctp/dcsctp_transport.h index 8e104da302..f154c44928 100644 --- a/media/sctp/dcsctp_transport.h +++ b/media/sctp/dcsctp_transport.h @@ -47,7 +47,8 @@ class DcSctpTransport : public cricket::SctpTransportInternal, 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 dc8ac4558d..96c35ffb93 100644 --- a/media/sctp/sctp_transport_internal.h +++ b/media/sctp/sctp_transport_internal.h @@ -18,6 +18,7 @@ #include #include +#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 @@ class SctpTransportInternal { // 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 cfd5d6f7f5..d43c017207 100644 --- a/media/sctp/usrsctp_transport.cc +++ b/media/sctp/usrsctp_transport.cc @@ -129,46 +129,37 @@ void DebugSctpPrintf(const char* format, ...) { } // 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 @@ void VerboseLogPacket(const void* data, size_t length, int direction) { // 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 @@ bool UsrsctpTransport::ResetStream(int sid) { 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 @@ bool UsrsctpTransport::SendData(const SendDataParams& params, } // 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 @@ bool UsrsctpTransport::SendData(const SendDataParams& params, } 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 @@ SendDataResult UsrsctpTransport::SendMessageInternal(OutgoingMessage* message) { 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 @@ SendDataResult UsrsctpTransport::SendMessageInternal(OutgoingMessage* message) { } // 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 @@ bool UsrsctpTransport::SendQueuedStreamResets() { // 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(stream.first)); }; // Figure out how many streams need to be reset. We need to do this so we can @@ -1158,7 +1151,7 @@ bool UsrsctpTransport::SendBufferedMessage() { } 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 @@ void UsrsctpTransport::OnDataOrNotificationFromSctp(const void* data, << ", 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 de018b924e..5dcf57b243 100644 --- a/media/sctp/usrsctp_transport.h +++ b/media/sctp/usrsctp_transport.h @@ -81,7 +81,8 @@ class UsrsctpTransport : public SctpTransportInternal, 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 UsrsctpTransport : public SctpTransportInternal, 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 @@ class UsrsctpTransport : public SctpTransportInternal, 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 407dd8d134..104e320398 100644 --- a/media/sctp/usrsctp_transport_reliability_unittest.cc +++ b/media/sctp/usrsctp_transport_reliability_unittest.cc @@ -133,23 +133,19 @@ class SimulatedPacketTransport final : public rtc::PacketTransportInternal { }; /** - * 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 @@ class SctpDataSender final { << 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 @@ class SctpDataSender final { 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 started_ ATOMIC_VAR_INIT(false); @@ -329,7 +323,7 @@ class SctpPingPong final { 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 @@ class SctpPingPong final { 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 @@ TEST_F(UsrSctpReliabilityTest, 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 @@ TEST_F(UsrSctpReliabilityTest, 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 @@ TEST_F(UsrSctpReliabilityTest, 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::max(); send_params.max_rtx_ms = std::numeric_limits::max(); @@ -750,8 +741,7 @@ TEST_F(UsrSctpReliabilityTest, 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 f75cb4a25d..59e9c59b3d 100644 --- a/media/sctp/usrsctp_transport_unittest.cc +++ b/media/sctp/usrsctp_transport_unittest.cc @@ -185,12 +185,11 @@ class SctpTransportTest : public ::testing::Test, public sigslot::has_slots<> { 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 @@ TEST_P(SctpTransportTestWithOrdered, SendDataBlocked) { SetupConnectedTransportsWithTwoStreams(); SendDataResult result; - SendDataParams params; - params.sid = 1; + webrtc::SendDataParams params; params.ordered = GetParam(); std::vector 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 @@ TEST_P(SctpTransportTestWithOrdered, SignalReadyToSendDataAfterBlocked) { 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; } } diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index 7e6dc4b0ae..d8e6b39895 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc @@ -31,11 +31,12 @@ bool DataChannelController::HasDataChannels() const { return !sctp_data_channels_.empty(); } -bool DataChannelController::SendData(const cricket::SendDataParams& params, +bool DataChannelController::SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { if (data_channel_transport()) - return DataChannelSendData(params, payload, result); + return DataChannelSendData(sid, params, payload, result); RTC_LOG(LS_ERROR) << "SendData called before transport is ready"; return false; } @@ -106,7 +107,7 @@ void DataChannelController::OnDataReceived( RTC_DCHECK_RUN_ON(network_thread()); cricket::ReceiveDataParams params; params.sid = channel_id; - params.type = ToCricketDataMessageType(type); + params.type = type; signaling_thread()->PostTask( ToQueuedTask([self = weak_factory_.GetWeakPtr(), params, buffer] { if (self) { @@ -222,7 +223,7 @@ std::vector DataChannelController::GetDataChannelStats() bool DataChannelController::HandleOpenMessage_s( const cricket::ReceiveDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { - if (params.type == cricket::DMT_CONTROL && IsOpenMessage(buffer)) { + if (params.type == DataMessageType::kControl && IsOpenMessage(buffer)) { // Received OPEN message; parse and signal that a new data channel should // be created. std::string label; @@ -386,7 +387,8 @@ void DataChannelController::set_data_channel_transport( } bool DataChannelController::DataChannelSendData( - const cricket::SendDataParams& params, + int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { // TODO(bugs.webrtc.org/11547): Expect method to be called on the network @@ -395,16 +397,9 @@ bool DataChannelController::DataChannelSendData( RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK(data_channel_transport()); - SendDataParams send_params; - send_params.type = ToWebrtcDataMessageType(params.type); - send_params.ordered = params.ordered; - send_params.max_rtx_count = params.max_rtx_count; - send_params.max_rtx_ms = params.max_rtx_ms; - RTCError error = network_thread()->Invoke( - RTC_FROM_HERE, [this, params, send_params, payload] { - return data_channel_transport()->SendData(params.sid, send_params, - payload); + RTC_FROM_HERE, [this, sid, params, payload] { + return data_channel_transport()->SendData(sid, params, payload); }); if (error.ok()) { diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h index 4c42b8a345..05fcff0e03 100644 --- a/pc/data_channel_controller.h +++ b/pc/data_channel_controller.h @@ -53,7 +53,8 @@ class DataChannelController : public SctpDataChannelProviderInterface, // Implements // SctpDataChannelProviderInterface. - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) override; bool ConnectDataChannel(SctpDataChannel* webrtc_data_channel) override; @@ -131,7 +132,8 @@ class DataChannelController : public SctpDataChannelProviderInterface, RTC_RUN_ON(signaling_thread()); // Called from SendData when data_channel_transport() is true. - bool DataChannelSendData(const cricket::SendDataParams& params, + bool DataChannelSendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result); diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc index 2dc003fd11..98c44f26fe 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc @@ -286,8 +286,9 @@ TEST_F(SctpDataChannelTest, OpenMessageSent) { SetChannelReady(); EXPECT_GE(webrtc_data_channel_->id(), 0); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); - EXPECT_EQ(provider_->last_send_data_params().sid, webrtc_data_channel_->id()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); + EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id()); } TEST_F(SctpDataChannelTest, QueuedOpenMessageSent) { @@ -295,8 +296,9 @@ TEST_F(SctpDataChannelTest, QueuedOpenMessageSent) { SetChannelReady(); provider_->set_send_blocked(false); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); - EXPECT_EQ(provider_->last_send_data_params().sid, webrtc_data_channel_->id()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); + EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id()); } // Tests that the DataChannel created after transport gets ready can enter OPEN @@ -333,7 +335,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceivesOpenAck) { // Emulates receiving an OPEN_ACK message. cricket::ReceiveDataParams params; params.sid = init.id; - params.type = cricket::DMT_CONTROL; + params.type = webrtc::DataMessageType::kControl; rtc::CopyOnWriteBuffer payload; webrtc::WriteDataChannelOpenAckMessage(&payload); dc->OnDataReceived(params, payload); @@ -359,7 +361,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceiveData) { // Emulates receiving a DATA message. cricket::ReceiveDataParams params; params.sid = init.id; - params.type = cricket::DMT_TEXT; + params.type = webrtc::DataMessageType::kText; webrtc::DataBuffer buffer("data"); dc->OnDataReceived(params, buffer.data); @@ -380,7 +382,8 @@ TEST_F(SctpDataChannelTest, OpenWaitsForOpenMesssage) { provider_->set_send_blocked(false); EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, webrtc_data_channel_->state(), 1000); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); } // Tests that close first makes sure all queued data gets sent. @@ -401,7 +404,8 @@ TEST_F(SctpDataChannelTest, QueuedCloseFlushes) { EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed, webrtc_data_channel_->state(), 1000); EXPECT_TRUE(webrtc_data_channel_->error().ok()); - EXPECT_EQ(cricket::DMT_TEXT, provider_->last_send_data_params().type); + EXPECT_EQ(webrtc::DataMessageType::kText, + provider_->last_send_data_params().type); } // Tests that messages are sent with the right id. @@ -410,7 +414,7 @@ TEST_F(SctpDataChannelTest, SendDataId) { SetChannelReady(); webrtc::DataBuffer buffer("data"); EXPECT_TRUE(webrtc_data_channel_->Send(buffer)); - EXPECT_EQ(1, provider_->last_send_data_params().sid); + EXPECT_EQ(1, provider_->last_sid()); } // Tests that the incoming messages with wrong ids are rejected. @@ -457,7 +461,7 @@ TEST_F(SctpDataChannelTest, NoMsgSentIfNegotiatedAndNotFromOpenMsg) { rtc::Thread::Current(), rtc::Thread::Current()); EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000); - EXPECT_EQ(0, provider_->last_send_data_params().sid); + EXPECT_EQ(0, provider_->last_sid()); } // Tests that DataChannel::messages_received() and DataChannel::bytes_received() @@ -522,8 +526,9 @@ TEST_F(SctpDataChannelTest, OpenAckSentIfCreatedFromOpenMessage) { EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000); - EXPECT_EQ(config.id, provider_->last_send_data_params().sid); - EXPECT_EQ(cricket::DMT_CONTROL, provider_->last_send_data_params().type); + EXPECT_EQ(config.id, provider_->last_sid()); + EXPECT_EQ(webrtc::DataMessageType::kControl, + provider_->last_send_data_params().type); } // Tests the OPEN_ACK role assigned by InternalDataChannelInit. diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index 2c9066155c..682d76829c 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -406,7 +406,7 @@ void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params, return; } - if (params.type == cricket::DMT_CONTROL) { + if (params.type == DataMessageType::kControl) { if (handshake_state_ != kHandshakeWaitingForAck) { // Ignore it if we are not expecting an ACK message. RTC_LOG(LS_WARNING) @@ -427,8 +427,8 @@ void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params, return; } - RTC_DCHECK(params.type == cricket::DMT_BINARY || - params.type == cricket::DMT_TEXT); + RTC_DCHECK(params.type == DataMessageType::kBinary || + params.type == DataMessageType::kText); RTC_LOG(LS_VERBOSE) << "DataChannel received DATA message, sid = " << params.sid; @@ -439,7 +439,7 @@ void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params, handshake_state_ = kHandshakeReady; } - bool binary = (params.type == cricket::DMT_BINARY); + bool binary = (params.type == webrtc::DataMessageType::kBinary); auto buffer = std::make_unique(payload, binary); if (state_ == kOpen && observer_) { ++messages_received_; @@ -620,7 +620,7 @@ void SctpDataChannel::SendQueuedDataMessages() { bool SctpDataChannel::SendDataMessage(const DataBuffer& buffer, bool queue_if_blocked) { RTC_DCHECK_RUN_ON(signaling_thread_); - cricket::SendDataParams send_params; + SendDataParams send_params; send_params.ordered = config_.ordered; // Send as ordered if it is still going through OPEN/ACK signaling. @@ -633,11 +633,12 @@ bool SctpDataChannel::SendDataMessage(const DataBuffer& buffer, send_params.max_rtx_count = config_.maxRetransmits; send_params.max_rtx_ms = config_.maxRetransmitTime; - send_params.sid = config_.id; - send_params.type = buffer.binary ? cricket::DMT_BINARY : cricket::DMT_TEXT; + send_params.type = + buffer.binary ? DataMessageType::kBinary : DataMessageType::kText; cricket::SendDataResult send_result = cricket::SDR_SUCCESS; - bool success = provider_->SendData(send_params, buffer.data, &send_result); + bool success = + provider_->SendData(config_.id, send_params, buffer.data, &send_result); if (success) { ++messages_sent_; @@ -703,16 +704,16 @@ bool SctpDataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) { bool is_open_message = handshake_state_ == kHandshakeShouldSendOpen; RTC_DCHECK(!is_open_message || !config_.negotiated); - cricket::SendDataParams send_params; - send_params.sid = config_.id; + SendDataParams send_params; // Send data as ordered before we receive any message from the remote peer to // make sure the remote peer will not receive any data before it receives the // OPEN message. send_params.ordered = config_.ordered || is_open_message; - send_params.type = cricket::DMT_CONTROL; + send_params.type = DataMessageType::kControl; cricket::SendDataResult send_result = cricket::SDR_SUCCESS; - bool retval = provider_->SendData(send_params, buffer, &send_result); + bool retval = + provider_->SendData(config_.id, send_params, buffer, &send_result); if (retval) { RTC_LOG(LS_VERBOSE) << "Sent CONTROL message on channel " << config_.id; diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h index ddb8565ff7..1d7a3c73f4 100644 --- a/pc/sctp_data_channel.h +++ b/pc/sctp_data_channel.h @@ -40,7 +40,8 @@ class SctpDataChannel; class SctpDataChannelProviderInterface { public: // Sends the data to the transport. - virtual bool SendData(const cricket::SendDataParams& params, + virtual bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) = 0; // Connects to the transport signals. diff --git a/pc/sctp_data_channel_transport.cc b/pc/sctp_data_channel_transport.cc index 786931c609..bb81156a23 100644 --- a/pc/sctp_data_channel_transport.cc +++ b/pc/sctp_data_channel_transport.cc @@ -39,17 +39,8 @@ RTCError SctpDataChannelTransport::SendData( int channel_id, const SendDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { - // Map webrtc::SendDataParams to cricket::SendDataParams. - // TODO(mellem): See about unifying these structs. - cricket::SendDataParams sd_params; - sd_params.sid = channel_id; - sd_params.type = ToCricketDataMessageType(params.type); - sd_params.ordered = params.ordered; - sd_params.max_rtx_count = params.max_rtx_count; - sd_params.max_rtx_ms = params.max_rtx_ms; - cricket::SendDataResult result; - sctp_transport_->SendData(sd_params, buffer, &result); + sctp_transport_->SendData(channel_id, params, buffer, &result); // TODO(mellem): See about changing the interfaces to not require mapping // SendDataResult to RTCError and back again. @@ -94,8 +85,7 @@ void SctpDataChannelTransport::OnDataReceived( const cricket::ReceiveDataParams& params, const rtc::CopyOnWriteBuffer& buffer) { if (sink_) { - sink_->OnDataReceived(params.sid, ToWebrtcDataMessageType(params.type), - buffer); + sink_->OnDataReceived(params.sid, params.type, buffer); } } diff --git a/pc/sctp_transport_unittest.cc b/pc/sctp_transport_unittest.cc index 8ab4482b1a..b4618edbff 100644 --- a/pc/sctp_transport_unittest.cc +++ b/pc/sctp_transport_unittest.cc @@ -38,7 +38,8 @@ class FakeCricketSctpTransport : public cricket::SctpTransportInternal { } bool OpenStream(int sid) override { return true; } bool ResetStream(int sid) override { return true; } - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result = nullptr) override { return true; diff --git a/pc/sctp_utils.cc b/pc/sctp_utils.cc index 9d46cc4319..f7458405ea 100644 --- a/pc/sctp_utils.cc +++ b/pc/sctp_utils.cc @@ -230,33 +230,4 @@ void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload) { payload->SetData(&data, sizeof(data)); } -cricket::DataMessageType ToCricketDataMessageType(DataMessageType type) { - switch (type) { - case DataMessageType::kText: - return cricket::DMT_TEXT; - case DataMessageType::kBinary: - return cricket::DMT_BINARY; - case DataMessageType::kControl: - return cricket::DMT_CONTROL; - default: - return cricket::DMT_NONE; - } - return cricket::DMT_NONE; -} - -DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type) { - switch (type) { - case cricket::DMT_TEXT: - return DataMessageType::kText; - case cricket::DMT_BINARY: - return DataMessageType::kBinary; - case cricket::DMT_CONTROL: - return DataMessageType::kControl; - case cricket::DMT_NONE: - default: - RTC_NOTREACHED(); - } - return DataMessageType::kControl; -} - } // namespace webrtc diff --git a/pc/sctp_utils.h b/pc/sctp_utils.h index 44225cfe3e..da854458f4 100644 --- a/pc/sctp_utils.h +++ b/pc/sctp_utils.h @@ -40,10 +40,6 @@ bool WriteDataChannelOpenMessage(const std::string& label, void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload); -cricket::DataMessageType ToCricketDataMessageType(DataMessageType type); - -DataMessageType ToWebrtcDataMessageType(cricket::DataMessageType type); - } // namespace webrtc #endif // PC_SCTP_UTILS_H_ diff --git a/pc/test/fake_data_channel_provider.h b/pc/test/fake_data_channel_provider.h index 6a063f8da7..f9e9e91d48 100644 --- a/pc/test/fake_data_channel_provider.h +++ b/pc/test/fake_data_channel_provider.h @@ -26,7 +26,8 @@ class FakeDataChannelProvider transport_error_(false) {} virtual ~FakeDataChannelProvider() {} - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const webrtc::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) override { RTC_CHECK(ready_to_send_); @@ -41,6 +42,7 @@ class FakeDataChannelProvider return false; } + last_sid_ = sid; last_send_data_params_ = params; return true; } @@ -127,7 +129,8 @@ class FakeDataChannelProvider void set_transport_error() { transport_error_ = true; } - cricket::SendDataParams last_send_data_params() const { + int last_sid() const { return last_sid_; } + const webrtc::SendDataParams& last_send_data_params() const { return last_send_data_params_; } @@ -144,7 +147,8 @@ class FakeDataChannelProvider } private: - cricket::SendDataParams last_send_data_params_; + int last_sid_; + webrtc::SendDataParams last_send_data_params_; bool send_blocked_; bool transport_available_; bool ready_to_send_; diff --git a/test/pc/sctp/fake_sctp_transport.h b/test/pc/sctp/fake_sctp_transport.h index fa4cdc4920..42b978a900 100644 --- a/test/pc/sctp/fake_sctp_transport.h +++ b/test/pc/sctp/fake_sctp_transport.h @@ -29,7 +29,8 @@ class FakeSctpTransport : public cricket::SctpTransportInternal { } bool OpenStream(int sid) override { return true; } bool ResetStream(int sid) override { return true; } - bool SendData(const cricket::SendDataParams& params, + bool SendData(int sid, + const webrtc::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result = nullptr) override { return true;