Revert of Use CopyOnWriteBuffer instead of Buffer to avoid unnecessary copies. (patchset #4 id:60001 of https://codereview.webrtc.org/1785713005/ )
Reason for revert:
I'm really sorry for having to revert this but it seems this hit an unexpected compile error downstream:
webrtc/media/sctp/sctpdataengine.cc: In function 'void cricket::VerboseLogPacket(const void*, size_t, int)':
webrtc/media/sctp/sctpdataengine.cc:172:37: error: invalid conversion from 'const void*' to 'void*' [-fpermissive]
data, length, direction)) != NULL) {
^
In file included from webrtc/media/sctp/sctpdataengine.cc:20:0:
third_party/usrsctp/usrsctplib/usrsctp.h:964:1: error: initializing argument 1 of 'char* usrsctp_dumppacket(void*, size_t, int)' [-fpermissive]
usrsctp_dumppacket(void *, size_t, int);
^
I'm sure you can fix this easily and just re-land this CL, while I'm going to look into how to add this warning at the public bots (on Monday).
Original issue's description:
> Use CopyOnWriteBuffer instead of Buffer to avoid unnecessary copies.
>
> This CL removes copy and assign support from Buffer and changes various
> parameters from Buffer to CopyOnWriteBuffer so they can be passed along
> and copied without actually copying the underlying data.
>
> With this changed some parameters to be "const" and fixed an issue when
> creating a CopyOnWriteBuffer with empty data.
>
> BUG=webrtc:5155
>
> Committed: https://crrev.com/944c39006f1c52aee20919676002dac7a42b1c05
> Cr-Commit-Position: refs/heads/master@{#12058}
TBR=kwiberg@webrtc.org,tkchin@webrtc.org,tommi@webrtc.org,pthatcher@webrtc.org,jbauch@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5155
Review URL: https://codereview.webrtc.org/1817753003
Cr-Commit-Position: refs/heads/master@{#12060}
diff --git a/webrtc/api/datachannel.cc b/webrtc/api/datachannel.cc
index 612d7e0..b4dc5d8 100644
--- a/webrtc/api/datachannel.cc
+++ b/webrtc/api/datachannel.cc
@@ -324,7 +324,7 @@
void DataChannel::OnDataReceived(cricket::DataChannel* channel,
const cricket::ReceiveDataParams& params,
- const rtc::CopyOnWriteBuffer& payload) {
+ const rtc::Buffer& payload) {
uint32_t expected_ssrc =
(data_channel_type_ == cricket::DCT_RTP) ? receive_ssrc_ : config_.id;
if (params.ssrc != expected_ssrc) {
@@ -422,11 +422,11 @@
}
if (connected_to_provider_) {
if (handshake_state_ == kHandshakeShouldSendOpen) {
- rtc::CopyOnWriteBuffer payload;
+ rtc::Buffer payload;
WriteDataChannelOpenMessage(label_, config_, &payload);
SendControlMessage(payload);
} else if (handshake_state_ == kHandshakeShouldSendAck) {
- rtc::CopyOnWriteBuffer payload;
+ rtc::Buffer payload;
WriteDataChannelOpenAckMessage(&payload);
SendControlMessage(payload);
}
@@ -595,11 +595,11 @@
}
}
-void DataChannel::QueueControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
+void DataChannel::QueueControlMessage(const rtc::Buffer& buffer) {
queued_control_data_.Push(new DataBuffer(buffer, true));
}
-bool DataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
+bool DataChannel::SendControlMessage(const rtc::Buffer& buffer) {
bool is_open_message = handshake_state_ == kHandshakeShouldSendOpen;
ASSERT(data_channel_type_ == cricket::DCT_SCTP &&
diff --git a/webrtc/api/datachannel.h b/webrtc/api/datachannel.h
index b8830be..62e3eaf 100644
--- a/webrtc/api/datachannel.h
+++ b/webrtc/api/datachannel.h
@@ -31,7 +31,7 @@
public:
// Sends the data to the transport.
virtual bool SendData(const cricket::SendDataParams& params,
- const rtc::CopyOnWriteBuffer& payload,
+ const rtc::Buffer& payload,
cricket::SendDataResult* result) = 0;
// Connects to the transport signals.
virtual bool ConnectDataChannel(DataChannel* data_channel) = 0;
@@ -143,7 +143,7 @@
// Sigslots from cricket::DataChannel
void OnDataReceived(cricket::DataChannel* channel,
const cricket::ReceiveDataParams& params,
- const rtc::CopyOnWriteBuffer& payload);
+ const rtc::Buffer& payload);
void OnStreamClosedRemotely(uint32_t sid);
// The remote peer request that this channel should be closed.
@@ -236,8 +236,8 @@
bool QueueSendDataMessage(const DataBuffer& buffer);
void SendQueuedControlMessages();
- void QueueControlMessage(const rtc::CopyOnWriteBuffer& buffer);
- bool SendControlMessage(const rtc::CopyOnWriteBuffer& buffer);
+ void QueueControlMessage(const rtc::Buffer& buffer);
+ bool SendControlMessage(const rtc::Buffer& buffer);
std::string label_;
InternalDataChannelInit config_;
diff --git a/webrtc/api/datachannel_unittest.cc b/webrtc/api/datachannel_unittest.cc
index 5958ec0..880c2aa 100644
--- a/webrtc/api/datachannel_unittest.cc
+++ b/webrtc/api/datachannel_unittest.cc
@@ -246,7 +246,7 @@
cricket::ReceiveDataParams params;
params.ssrc = init.id;
params.type = cricket::DMT_CONTROL;
- rtc::CopyOnWriteBuffer payload;
+ rtc::Buffer payload;
webrtc::WriteDataChannelOpenAckMessage(&payload);
dc->OnDataReceived(NULL, params, payload);
@@ -404,7 +404,7 @@
TEST_F(SctpDataChannelTest, ClosedWhenSendBufferFull) {
SetChannelReady();
- rtc::CopyOnWriteBuffer buffer(1024);
+ rtc::Buffer buffer(1024);
memset(buffer.data(), 0, buffer.size());
webrtc::DataBuffer packet(buffer, true);
@@ -457,7 +457,7 @@
// Tests that the DataChannel is closed if the received buffer is full.
TEST_F(SctpDataChannelTest, ClosedWhenReceivedBufferFull) {
SetChannelReady();
- rtc::CopyOnWriteBuffer buffer(1024);
+ rtc::Buffer buffer(1024);
memset(buffer.data(), 0, buffer.size());
cricket::ReceiveDataParams params;
diff --git a/webrtc/api/datachannelinterface.h b/webrtc/api/datachannelinterface.h
index 53c11d4..3d6f711 100644
--- a/webrtc/api/datachannelinterface.h
+++ b/webrtc/api/datachannelinterface.h
@@ -17,8 +17,8 @@
#include <string>
#include "webrtc/base/basictypes.h"
+#include "webrtc/base/buffer.h"
#include "webrtc/base/checks.h"
-#include "webrtc/base/copyonwritebuffer.h"
#include "webrtc/base/refcount.h"
@@ -50,7 +50,7 @@
};
struct DataBuffer {
- DataBuffer(const rtc::CopyOnWriteBuffer& data, bool binary)
+ DataBuffer(const rtc::Buffer& data, bool binary)
: data(data),
binary(binary) {
}
@@ -61,7 +61,7 @@
}
size_t size() const { return data.size(); }
- rtc::CopyOnWriteBuffer data;
+ rtc::Buffer data;
// Indicates if the received data contains UTF-8 or binary data.
// Note that the upper layers are left to verify the UTF-8 encoding.
// TODO(jiayl): prefer to use an enum instead of a bool.
diff --git a/webrtc/api/java/jni/peerconnection_jni.cc b/webrtc/api/java/jni/peerconnection_jni.cc
index 6c8192d..d614ed6 100644
--- a/webrtc/api/java/jni/peerconnection_jni.cc
+++ b/webrtc/api/java/jni/peerconnection_jni.cc
@@ -893,7 +893,7 @@
jbyteArray data, jboolean binary) {
jbyte* bytes = jni->GetByteArrayElements(data, NULL);
bool ret = ExtractNativeDC(jni, j_dc)->Send(DataBuffer(
- rtc::CopyOnWriteBuffer(bytes, jni->GetArrayLength(data)),
+ rtc::Buffer(bytes, jni->GetArrayLength(data)),
binary));
jni->ReleaseByteArrayElements(data, bytes, JNI_ABORT);
return ret;
diff --git a/webrtc/api/objc/RTCDataChannel.mm b/webrtc/api/objc/RTCDataChannel.mm
index 9859905..5778cb5 100644
--- a/webrtc/api/objc/RTCDataChannel.mm
+++ b/webrtc/api/objc/RTCDataChannel.mm
@@ -53,8 +53,8 @@
- (instancetype)initWithData:(NSData *)data isBinary:(BOOL)isBinary {
NSParameterAssert(data);
if (self = [super init]) {
- rtc::CopyOnWriteBuffer buffer(
- reinterpret_cast<const uint8_t*>(data.bytes), data.length);
+ rtc::Buffer buffer(reinterpret_cast<const uint8_t*>(data.bytes),
+ data.length);
_dataBuffer.reset(new webrtc::DataBuffer(buffer, isBinary));
}
return self;
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index d4678ba..6bff11a 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -1400,7 +1400,7 @@
EXPECT_EQ(DataChannelInterface::kOpen, data1->state());
EXPECT_EQ(DataChannelInterface::kOpen, data2->state());
- rtc::CopyOnWriteBuffer buffer("test", 4);
+ rtc::Buffer buffer("test", 4);
EXPECT_FALSE(data1->Send(DataBuffer(buffer, true)));
}
diff --git a/webrtc/api/sctputils.cc b/webrtc/api/sctputils.cc
index f2d1b0f..1a2c282 100644
--- a/webrtc/api/sctputils.cc
+++ b/webrtc/api/sctputils.cc
@@ -10,8 +10,8 @@
#include "webrtc/api/sctputils.h"
+#include "webrtc/base/buffer.h"
#include "webrtc/base/bytebuffer.h"
-#include "webrtc/base/copyonwritebuffer.h"
#include "webrtc/base/logging.h"
namespace webrtc {
@@ -31,27 +31,26 @@
DCOMCT_UNORDERED_PARTIAL_TIME = 0x82,
};
-bool IsOpenMessage(const rtc::CopyOnWriteBuffer& payload) {
+bool IsOpenMessage(const rtc::Buffer& payload) {
// Format defined at
// http://tools.ietf.org/html/draft-jesup-rtcweb-data-protocol-04
- if (payload.size() < 1) {
+
+ rtc::ByteBuffer buffer(payload);
+ uint8_t message_type;
+ if (!buffer.ReadUInt8(&message_type)) {
LOG(LS_WARNING) << "Could not read OPEN message type.";
return false;
}
-
- uint8_t message_type = payload[0];
return message_type == DATA_CHANNEL_OPEN_MESSAGE_TYPE;
}
-bool ParseDataChannelOpenMessage(const rtc::CopyOnWriteBuffer& payload,
+bool ParseDataChannelOpenMessage(const rtc::Buffer& payload,
std::string* label,
DataChannelInit* config) {
// Format defined at
// http://tools.ietf.org/html/draft-jesup-rtcweb-data-protocol-04
- // TODO(jbauch): avoid copying the payload data into the ByteBuffer, see
- // https://bugs.chromium.org/p/webrtc/issues/detail?id=5670
- rtc::ByteBuffer buffer(payload.data<char>(), payload.size());
+ rtc::ByteBuffer buffer(payload);
uint8_t message_type;
if (!buffer.ReadUInt8(&message_type)) {
LOG(LS_WARNING) << "Could not read OPEN message type.";
@@ -121,13 +120,13 @@
return true;
}
-bool ParseDataChannelOpenAckMessage(const rtc::CopyOnWriteBuffer& payload) {
- if (payload.size() < 1) {
+bool ParseDataChannelOpenAckMessage(const rtc::Buffer& payload) {
+ rtc::ByteBuffer buffer(payload);
+ uint8_t message_type;
+ if (!buffer.ReadUInt8(&message_type)) {
LOG(LS_WARNING) << "Could not read OPEN_ACK message type.";
return false;
}
-
- uint8_t message_type = payload[0];
if (message_type != DATA_CHANNEL_OPEN_ACK_MESSAGE_TYPE) {
LOG(LS_WARNING) << "Data Channel OPEN_ACK message of unexpected type: "
<< message_type;
@@ -138,7 +137,7 @@
bool WriteDataChannelOpenMessage(const std::string& label,
const DataChannelInit& config,
- rtc::CopyOnWriteBuffer* payload) {
+ rtc::Buffer* payload) {
// Format defined at
// http://tools.ietf.org/html/draft-ietf-rtcweb-data-protocol-00#section-6.1
uint8_t channel_type = 0;
@@ -169,7 +168,6 @@
rtc::ByteBuffer buffer(
NULL, 20 + label.length() + config.protocol.length(),
rtc::ByteBuffer::ORDER_NETWORK);
- // TODO(tommi): Add error handling and check resulting length.
buffer.WriteUInt8(DATA_CHANNEL_OPEN_MESSAGE_TYPE);
buffer.WriteUInt8(channel_type);
buffer.WriteUInt16(priority);
@@ -182,9 +180,9 @@
return true;
}
-void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload) {
- uint8_t data = DATA_CHANNEL_OPEN_ACK_MESSAGE_TYPE;
- payload->SetData(&data, sizeof(data));
+void WriteDataChannelOpenAckMessage(rtc::Buffer* payload) {
+ rtc::ByteBuffer buffer(rtc::ByteBuffer::ORDER_NETWORK);
+ buffer.WriteUInt8(DATA_CHANNEL_OPEN_ACK_MESSAGE_TYPE);
+ payload->SetData(buffer.Data(), buffer.Length());
}
-
} // namespace webrtc
diff --git a/webrtc/api/sctputils.h b/webrtc/api/sctputils.h
index 2fb1943..ffbade2 100644
--- a/webrtc/api/sctputils.h
+++ b/webrtc/api/sctputils.h
@@ -16,26 +16,26 @@
#include "webrtc/api/datachannelinterface.h"
namespace rtc {
-class CopyOnWriteBuffer;
+class Buffer;
} // namespace rtc
namespace webrtc {
struct DataChannelInit;
// Read the message type and return true if it's an OPEN message.
-bool IsOpenMessage(const rtc::CopyOnWriteBuffer& payload);
+bool IsOpenMessage(const rtc::Buffer& payload);
-bool ParseDataChannelOpenMessage(const rtc::CopyOnWriteBuffer& payload,
+bool ParseDataChannelOpenMessage(const rtc::Buffer& payload,
std::string* label,
DataChannelInit* config);
-bool ParseDataChannelOpenAckMessage(const rtc::CopyOnWriteBuffer& payload);
+bool ParseDataChannelOpenAckMessage(const rtc::Buffer& payload);
bool WriteDataChannelOpenMessage(const std::string& label,
const DataChannelInit& config,
- rtc::CopyOnWriteBuffer* payload);
+ rtc::Buffer* payload);
-void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload);
+void WriteDataChannelOpenAckMessage(rtc::Buffer* payload);
} // namespace webrtc
#endif // WEBRTC_API_SCTPUTILS_H_
diff --git a/webrtc/api/test/fakedatachannelprovider.h b/webrtc/api/test/fakedatachannelprovider.h
index 3404ac1..7899c31 100644
--- a/webrtc/api/test/fakedatachannelprovider.h
+++ b/webrtc/api/test/fakedatachannelprovider.h
@@ -23,7 +23,7 @@
virtual ~FakeDataChannelProvider() {}
bool SendData(const cricket::SendDataParams& params,
- const rtc::CopyOnWriteBuffer& payload,
+ const rtc::Buffer& payload,
cricket::SendDataResult* result) override {
ASSERT(ready_to_send_ && transport_available_);
if (send_blocked_) {
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 19f5aa4..593d429 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -1374,7 +1374,7 @@
}
bool WebRtcSession::SendData(const cricket::SendDataParams& params,
- const rtc::CopyOnWriteBuffer& payload,
+ const rtc::Buffer& payload,
cricket::SendDataResult* result) {
if (!data_channel_) {
LOG(LS_ERROR) << "SendData called when data_channel_ is NULL.";
@@ -1854,7 +1854,7 @@
void WebRtcSession::OnDataChannelMessageReceived(
cricket::DataChannel* channel,
const cricket::ReceiveDataParams& params,
- const rtc::CopyOnWriteBuffer& payload) {
+ const rtc::Buffer& payload) {
RTC_DCHECK(data_channel_type_ == cricket::DCT_SCTP);
if (params.type == cricket::DMT_CONTROL && IsOpenMessage(payload)) {
// Received OPEN message; parse and signal that a new data channel should
diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h
index 6164731..e884678 100644
--- a/webrtc/api/webrtcsession.h
+++ b/webrtc/api/webrtcsession.h
@@ -271,7 +271,7 @@
// Implements DataChannelProviderInterface.
bool SendData(const cricket::SendDataParams& params,
- const rtc::CopyOnWriteBuffer& payload,
+ const rtc::Buffer& payload,
cricket::SendDataResult* result) override;
bool ConnectDataChannel(DataChannel* webrtc_data_channel) override;
void DisconnectDataChannel(DataChannel* webrtc_data_channel) override;
@@ -413,7 +413,7 @@
// messages.
void OnDataChannelMessageReceived(cricket::DataChannel* channel,
const cricket::ReceiveDataParams& params,
- const rtc::CopyOnWriteBuffer& payload);
+ const rtc::Buffer& payload);
std::string BadStateErrMsg(State state);
void SetIceConnectionState(PeerConnectionInterface::IceConnectionState state);
diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc
index 18c1a95..77c4854 100644
--- a/webrtc/api/webrtcsession_unittest.cc
+++ b/webrtc/api/webrtcsession_unittest.cc
@@ -4054,7 +4054,7 @@
webrtc::DataChannelInit config;
config.id = 1;
- rtc::CopyOnWriteBuffer payload;
+ rtc::Buffer payload;
webrtc::WriteDataChannelOpenMessage("a", config, &payload);
cricket::ReceiveDataParams params;
params.ssrc = config.id;