Fixed DataChannel shifting on double offer

This commit is contained in:
Paul-Louis Ageneau
2021-04-06 18:27:26 +02:00
parent b5884c84fc
commit 791e6b1e32
3 changed files with 17 additions and 17 deletions

View File

@ -262,6 +262,9 @@ shared_ptr<SctpTransport> PeerConnection::initSctpTransport() {
if (!remote || !remote->application())
throw std::logic_error("Starting SCTP transport without application description");
// This is the last occasion to ensure the stream numbers are coherent with the role
shiftDataChannels();
uint16_t sctpPort = remote->application()->sctpPort().value_or(DEFAULT_SCTP_PORT);
auto lower = std::atomic_load(&mDtlsTransport);
auto transport = std::make_shared<SctpTransport>(
@ -549,8 +552,7 @@ void PeerConnection::forwardBufferedAmount(uint16_t stream, size_t amount) {
channel->triggerBufferedAmount(amount);
}
shared_ptr<DataChannel> PeerConnection::emplaceDataChannel(Description::Role role, string label,
DataChannelInit init) {
shared_ptr<DataChannel> PeerConnection::emplaceDataChannel(string label, DataChannelInit init) {
std::unique_lock lock(mDataChannelsMutex); // we are going to emplace
uint16_t stream;
if (init.id) {
@ -558,6 +560,13 @@ shared_ptr<DataChannel> PeerConnection::emplaceDataChannel(Description::Role rol
if (stream == 65535)
throw std::invalid_argument("Invalid DataChannel id");
} else {
// RFC 5763: The answerer MUST use either a setup attribute value of setup:active or
// setup:passive. [...] Thus, setup:active is RECOMMENDED.
// See https://tools.ietf.org/html/rfc5763#section-5
// Therefore, we assume passive role if we are the offerer.
auto iceTransport = getIceTransport();
auto role = iceTransport ? iceTransport->role() : Description::Role::Passive;
// RFC 8832: The peer that initiates opening a data channel selects a stream identifier for
// which the corresponding incoming and outgoing streams are unused. If the side is acting
// as the DTLS client, it MUST choose an even stream identifier; if the side is acting as
@ -907,6 +916,10 @@ void PeerConnection::processRemoteDescription(Description description) {
auto iceTransport = initIceTransport();
iceTransport->setRemoteDescription(std::move(description));
// Since we assumed passive role during DataChannel creation, we might need to shift the stream
// numbers from odd to even.
shiftDataChannels();
if (description.hasApplication()) {
auto dtlsTransport = std::atomic_load(&mDtlsTransport);
auto sctpTransport = std::atomic_load(&mSctpTransport);

View File

@ -65,8 +65,7 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
void forwardBufferedAmount(uint16_t stream, size_t amount);
optional<string> getMidFromSsrc(uint32_t ssrc);
shared_ptr<DataChannel> emplaceDataChannel(Description::Role role, string label,
DataChannelInit init);
shared_ptr<DataChannel> emplaceDataChannel(string label, DataChannelInit init);
shared_ptr<DataChannel> findDataChannel(uint16_t stream);
void shiftDataChannels();
void iterateDataChannels(std::function<void(shared_ptr<DataChannel> channel)> func);

View File

@ -223,11 +223,6 @@ void PeerConnection::setRemoteDescription(Description description) {
// This is an offer, we need to answer
if (!impl()->config.disableAutoNegotiation)
setLocalDescription(Description::Type::Answer);
} else {
// This is an answer
// Since we assumed passive role during DataChannel creation, we need to shift the
// stream numbers by one to shift them from odd to even.
impl()->shiftDataChannels();
}
for (const auto &candidate : remoteCandidates)
@ -250,14 +245,7 @@ optional<string> PeerConnection::remoteAddress() const {
}
shared_ptr<DataChannel> PeerConnection::createDataChannel(string label, DataChannelInit init) {
// RFC 5763: The answerer MUST use either a setup attribute value of setup:active or
// setup:passive. [...] Thus, setup:active is RECOMMENDED.
// See https://tools.ietf.org/html/rfc5763#section-5
// Therefore, we assume passive role when we are the offerer.
auto iceTransport = impl()->getIceTransport();
auto role = iceTransport ? iceTransport->role() : Description::Role::Passive;
auto channelImpl = impl()->emplaceDataChannel(role, std::move(label), std::move(init));
auto channelImpl = impl()->emplaceDataChannel(std::move(label), std::move(init));
auto channel = std::make_shared<DataChannel>(channelImpl);
if (auto transport = impl()->getSctpTransport())