Remove cricket::ReceiveDataParams

This struct only contains two member variables now and there isn't
much value added by having it.

Low-Coverage-Reason: No change in coverage, CL modifies uncovered RTC_LOG lines.
Bug: none
Change-Id: I924d450f4c8f8e49b1cfeabaebee9fd5235a90cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297360
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39563}
This commit is contained in:
Tommi 2023-03-15 12:36:20 +01:00 committed by WebRTC LUCI CQ
parent 2f7071a57a
commit 4e1c9570ed
8 changed files with 38 additions and 75 deletions

View File

@ -999,17 +999,6 @@ class VideoMediaReceiveChannelInterface : public MediaReceiveChannelInterface {
absl::optional<int> 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

View File

@ -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_);
}
}

View File

@ -19,9 +19,6 @@
#include <vector>
#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,

View File

@ -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<DataChannelStats> 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(

View File

@ -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.

View File

@ -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());

View File

@ -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<DataBuffer>(payload, binary);
if (state_ == kOpen && observer_) {
++messages_received_;

View File

@ -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<StreamId> used_sids_;
flat_set<StreamId> 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