diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 59a47034ce..5ff38612c0 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -999,17 +999,6 @@ class VideoMediaReceiveChannelInterface : public MediaReceiveChannelInterface { absl::optional rtx_time) = 0; }; -// Info about data received in DataMediaChannel. For use in -// DataMediaChannel::SignalDataReceived and in all of the signals that -// signal fires, on up the chain. -struct ReceiveDataParams { - // The in-packet stream identifier. - // SCTP data channels use SIDs. - int sid = 0; - // The type of message (binary, text, or control). - webrtc::DataMessageType type = webrtc::DataMessageType::kText; -}; - 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 0181f7ada5..5dab2c7e57 100644 --- a/media/sctp/dcsctp_transport.cc +++ b/media/sctp/dcsctp_transport.cc @@ -453,8 +453,6 @@ void DcSctpTransport::OnMessageReceived(dcsctp::DcSctpMessage message) { << message.stream_id().value() << ", ppid=" << message.ppid().value() << ", length=" << message.payload().size() << ")."; - cricket::ReceiveDataParams receive_data_params; - receive_data_params.sid = message.stream_id().value(); auto type = ToDataMessageType(message.ppid()); if (!type.has_value()) { RTC_LOG(LS_VERBOSE) << debug_name_ @@ -463,15 +461,14 @@ void DcSctpTransport::OnMessageReceived(dcsctp::DcSctpMessage message) { << " on an SCTP packet. Dropping."; return; } - receive_data_params.type = *type; receive_buffer_.Clear(); if (!IsEmptyPPID(message.ppid())) receive_buffer_.AppendData(message.payload().data(), message.payload().size()); if (data_channel_sink_) { - data_channel_sink_->OnDataReceived( - receive_data_params.sid, receive_data_params.type, receive_buffer_); + data_channel_sink_->OnDataReceived(message.stream_id().value(), *type, + receive_buffer_); } } diff --git a/media/sctp/sctp_transport_internal.h b/media/sctp/sctp_transport_internal.h index fd31176894..7ab42b75b4 100644 --- a/media/sctp/sctp_transport_internal.h +++ b/media/sctp/sctp_transport_internal.h @@ -19,9 +19,6 @@ #include #include "api/transport/data_channel_transport_interface.h" -// For SendDataParams/ReceiveDataParams. -// TODO(deadbeef): Use something else for SCTP. It's confusing that we use an -// SSRC field for SID. #include "media/base/media_channel.h" #include "p2p/base/packet_transport_internal.h" #include "rtc_base/copy_on_write_buffer.h" @@ -121,10 +118,8 @@ class SctpTransportInternal { // initiates it, and both SignalClosingProcedureStartedRemotely and // SignalClosingProcedureComplete on the other side. virtual bool ResetStream(int sid) = 0; - // Send data down this channel (will be wrapped as SCTP packets then given to - // usrsctp that will then post the network interface). + // Send data down this channel. // Returns true iff successful data somewhere on the send-queue/network. - // Uses `params.ssrc` as the SCTP sid. virtual bool SendData(int sid, const webrtc::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc index c8579de953..7ce2fd74f0 100644 --- a/pc/data_channel_controller.cc +++ b/pc/data_channel_controller.cc @@ -93,21 +93,18 @@ void DataChannelController::OnDataReceived( DataMessageType type, const rtc::CopyOnWriteBuffer& buffer) { RTC_DCHECK_RUN_ON(network_thread()); - cricket::ReceiveDataParams params; - params.sid = channel_id; - params.type = type; - if (HandleOpenMessage_n(params, buffer)) + if (HandleOpenMessage_n(channel_id, type, buffer)) return; signaling_thread()->PostTask( - SafeTask(signaling_safety_.flag(), [this, params, buffer] { + SafeTask(signaling_safety_.flag(), [this, channel_id, type, buffer] { RTC_DCHECK_RUN_ON(signaling_thread()); // TODO(bugs.webrtc.org/11547): The data being received should be // delivered on the network thread. for (const auto& channel : sctp_data_channels_) { - if (channel->id() == params.sid) - channel->OnDataReceived(params, buffer); + if (channel->id() == channel_id) + channel->OnDataReceived(type, buffer); } })); } @@ -214,19 +211,20 @@ std::vector DataChannelController::GetDataChannelStats() } bool DataChannelController::HandleOpenMessage_n( - const cricket::ReceiveDataParams& params, + int channel_id, + DataMessageType type, const rtc::CopyOnWriteBuffer& buffer) { - if (params.type != DataMessageType::kControl || !IsOpenMessage(buffer)) + if (type != DataMessageType::kControl || !IsOpenMessage(buffer)) return false; // Received OPEN message; parse and signal that a new data channel should // be created. std::string label; InternalDataChannelInit config; - config.id = params.sid; + config.id = channel_id; if (!ParseDataChannelOpenMessage(buffer, &label, &config)) { RTC_LOG(LS_WARNING) << "Failed to parse the OPEN message for sid " - << params.sid; + << channel_id; } else { config.open_handshake_role = InternalDataChannelInit::kAcker; signaling_thread()->PostTask( diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h index c79192474f..4cca379644 100644 --- a/pc/data_channel_controller.h +++ b/pc/data_channel_controller.h @@ -20,7 +20,6 @@ #include "api/sequence_checker.h" #include "api/task_queue/pending_task_safety_flag.h" #include "api/transport/data_channel_transport_interface.h" -#include "media/base/media_channel.h" #include "pc/data_channel_utils.h" #include "pc/sctp_data_channel.h" #include "rtc_base/checks.h" @@ -114,7 +113,8 @@ class DataChannelController : public SctpDataChannelControllerInterface, // Parses and handles open messages. Returns true if the message is an open // message and should be considered to be handled, false otherwise. - bool HandleOpenMessage_n(const cricket::ReceiveDataParams& params, + bool HandleOpenMessage_n(int channel_id, + DataMessageType type, const rtc::CopyOnWriteBuffer& buffer) RTC_RUN_ON(network_thread()); // Called when a valid data channel OPEN message is received. diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc index e783dce0af..f79228726e 100644 --- a/pc/data_channel_unittest.cc +++ b/pc/data_channel_unittest.cc @@ -352,12 +352,9 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceivesOpenAck) { EXPECT_TRUE(controller_->last_send_data_params().ordered); // Emulates receiving an OPEN_ACK message. - cricket::ReceiveDataParams params; - params.sid = init.id; - params.type = DataMessageType::kControl; rtc::CopyOnWriteBuffer payload; WriteDataChannelOpenAckMessage(&payload); - dc->OnDataReceived(params, payload); + dc->OnDataReceived(DataMessageType::kControl, payload); // Sends another message and verifies it's unordered. ASSERT_TRUE(dc->Send(buffer)); @@ -378,11 +375,8 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceiveData) { EXPECT_EQ_WAIT(DataChannelInterface::kOpen, dc->state(), 1000); // Emulates receiving a DATA message. - cricket::ReceiveDataParams params; - params.sid = init.id; - params.type = DataMessageType::kText; DataBuffer buffer("data"); - dc->OnDataReceived(params, buffer.data); + dc->OnDataReceived(DataMessageType::kText, buffer.data); // Sends a message and verifies it's unordered. ASSERT_TRUE(dc->Send(buffer)); @@ -440,11 +434,8 @@ TEST_F(SctpDataChannelTest, ReceiveDataWithValidId) { AddObserver(); - cricket::ReceiveDataParams params; - params.sid = 1; DataBuffer buffer("abcd"); - - webrtc_data_channel_->OnDataReceived(params, buffer.data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffer.data); EXPECT_EQ(1U, observer_->messages_received()); } @@ -479,17 +470,15 @@ TEST_F(SctpDataChannelTest, VerifyMessagesAndBytesReceived) { }); webrtc_data_channel_->SetSctpSid(StreamId(1)); - cricket::ReceiveDataParams params; - params.sid = 1; // Default values. EXPECT_EQ(0U, webrtc_data_channel_->messages_received()); EXPECT_EQ(0U, webrtc_data_channel_->bytes_received()); // Receive three buffers while data channel isn't open. - webrtc_data_channel_->OnDataReceived(params, buffers[0].data); - webrtc_data_channel_->OnDataReceived(params, buffers[1].data); - webrtc_data_channel_->OnDataReceived(params, buffers[2].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[0].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[1].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[2].data); EXPECT_EQ(0U, observer_->messages_received()); EXPECT_EQ(0U, webrtc_data_channel_->messages_received()); EXPECT_EQ(0U, webrtc_data_channel_->bytes_received()); @@ -503,9 +492,9 @@ TEST_F(SctpDataChannelTest, VerifyMessagesAndBytesReceived) { EXPECT_EQ(bytes_received, webrtc_data_channel_->bytes_received()); // Receive three buffers while open. - webrtc_data_channel_->OnDataReceived(params, buffers[3].data); - webrtc_data_channel_->OnDataReceived(params, buffers[4].data); - webrtc_data_channel_->OnDataReceived(params, buffers[5].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[3].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[4].data); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffers[5].data); bytes_received += buffers[3].size() + buffers[4].size() + buffers[5].size(); EXPECT_EQ(6U, observer_->messages_received()); EXPECT_EQ(6U, webrtc_data_channel_->messages_received()); @@ -589,12 +578,9 @@ TEST_F(SctpDataChannelTest, ClosedWhenReceivedBufferFull) { rtc::CopyOnWriteBuffer buffer(1024); memset(buffer.MutableData(), 0, buffer.size()); - cricket::ReceiveDataParams params; - params.sid = 0; - // Receiving data without having an observer will overflow the buffer. for (size_t i = 0; i < 16 * 1024 + 1; ++i) { - webrtc_data_channel_->OnDataReceived(params, buffer); + webrtc_data_channel_->OnDataReceived(DataMessageType::kText, buffer); } EXPECT_EQ(DataChannelInterface::kClosed, webrtc_data_channel_->state()); EXPECT_FALSE(webrtc_data_channel_->error().ok()); diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc index ccd9c70aee..dad00f2446 100644 --- a/pc/sctp_data_channel.cc +++ b/pc/sctp_data_channel.cc @@ -192,13 +192,13 @@ SctpDataChannel::SctpDataChannel( RTC_DCHECK(config.IsValid()); switch (config.open_handshake_role) { - case webrtc::InternalDataChannelInit::kNone: // pre-negotiated + case InternalDataChannelInit::kNone: // pre-negotiated handshake_state_ = kHandshakeReady; break; - case webrtc::InternalDataChannelInit::kOpener: + case InternalDataChannelInit::kOpener: handshake_state_ = kHandshakeShouldSendOpen; break; - case webrtc::InternalDataChannelInit::kAcker: + case InternalDataChannelInit::kAcker: handshake_state_ = kHandshakeShouldSendAck; break; } @@ -426,37 +426,36 @@ DataChannelStats SctpDataChannel::GetStats() const { return stats; } -void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params, +void SctpDataChannel::OnDataReceived(DataMessageType type, const rtc::CopyOnWriteBuffer& payload) { RTC_DCHECK_RUN_ON(signaling_thread_); - RTC_DCHECK_EQ(id_.stream_id_int(), params.sid); - if (params.type == DataMessageType::kControl) { + if (type == DataMessageType::kControl) { if (handshake_state_ != kHandshakeWaitingForAck) { // Ignore it if we are not expecting an ACK message. RTC_LOG(LS_WARNING) << "DataChannel received unexpected CONTROL message, sid = " - << params.sid; + << id_.stream_id_int(); return; } if (ParseDataChannelOpenAckMessage(payload)) { // We can send unordered as soon as we receive the ACK message. handshake_state_ = kHandshakeReady; RTC_LOG(LS_INFO) << "DataChannel received OPEN_ACK message, sid = " - << params.sid; + << id_.stream_id_int(); } else { RTC_LOG(LS_WARNING) << "DataChannel failed to parse OPEN_ACK message, sid = " - << params.sid; + << id_.stream_id_int(); } return; } - RTC_DCHECK(params.type == DataMessageType::kBinary || - params.type == DataMessageType::kText); + RTC_DCHECK(type == DataMessageType::kBinary || + type == DataMessageType::kText); RTC_LOG(LS_VERBOSE) << "DataChannel received DATA message, sid = " - << params.sid; + << id_.stream_id_int(); // We can send unordered as soon as we receive any DATA message since the // remote side must have received the OPEN (and old clients do not send // OPEN_ACK). @@ -464,7 +463,7 @@ void SctpDataChannel::OnDataReceived(const cricket::ReceiveDataParams& params, handshake_state_ = kHandshakeReady; } - bool binary = (params.type == webrtc::DataMessageType::kBinary); + bool binary = (type == DataMessageType::kBinary); auto buffer = std::make_unique(payload, binary); if (state_ == kOpen && observer_) { ++messages_received_; diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h index 6289801812..be34f3a6ef 100644 --- a/pc/sctp_data_channel.h +++ b/pc/sctp_data_channel.h @@ -23,7 +23,6 @@ #include "api/rtc_error.h" #include "api/scoped_refptr.h" #include "api/transport/data_channel_transport_interface.h" -#include "media/base/media_channel.h" #include "pc/data_channel_utils.h" #include "pc/sctp_utils.h" #include "rtc_base/containers/flat_set.h" @@ -94,7 +93,7 @@ class SctpSidAllocator { void ReleaseSid(const StreamId& sid); private: - webrtc::flat_set used_sids_; + flat_set used_sids_; }; // SctpDataChannel is an implementation of the DataChannelInterface based on @@ -180,7 +179,7 @@ class SctpDataChannel : public DataChannelInterface { // already finished. void OnTransportReady(bool writable); - void OnDataReceived(const cricket::ReceiveDataParams& params, + void OnDataReceived(DataMessageType type, const rtc::CopyOnWriteBuffer& payload); // Sets the SCTP sid and adds to transport layer if not set yet. Should only