zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enable payload type based demuxing with multiple tracks when applicable.

Browse Source 
This fixes regressions caused by:
https://webrtc-review.googlesource.com/c/src/+/183120

... which disabled payload type demuxing when multiple video tracks are
present, to avoid one channel creating a default track intended for
another channel.

However, this isn't an issue when not bundling, as each track will be
delivered on separate transport.

And it's also not an issue when each track uses a distinct set of
payload types (e.g., VP8 is mapped to PT 96 in one m= section, and PT 97
in another).

This CL addresses both of those cases; PT demuxing is only disabled
when two bundled m= sections have overlapping payload types.

Bug: chromium:1139052, webrtc:12029
Change-Id: Ied844bffac2a5fac29147c11b56a5f83a95ecb36
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187560
Commit-Queue: Taylor <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32419}
This commit is contained in:
Taylor Brandstetter
2020-10-15 18:22:57 -07:00
committed by Commit Bot
parent fa9520e752
commit d3ef499418
7 changed files with 207 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
pc/channel.cc
View File

@ -290,9 +290,9 @@ bool BaseChannel::SetRemoteContent(const MediaContentDescription* content,
Bind(&BaseChannel::SetRemoteContent_w, this, content, type, error_desc));
}
void BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) {
bool BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) {
TRACE_EVENT0("webrtc", "BaseChannel::SetPayloadTypeDemuxingEnabled");
InvokeOnWorker<void>(
return InvokeOnWorker<bool>(
RTC_FROM_HERE,
Bind(&BaseChannel::SetPayloadTypeDemuxingEnabled_w, this, enabled));
}
@ -595,11 +595,12 @@ void BaseChannel::ResetUnsignaledRecvStream_w() {
media_channel()->ResetUnsignaledRecvStream();
}
void BaseChannel::SetPayloadTypeDemuxingEnabled_w(bool enabled) {
bool BaseChannel::SetPayloadTypeDemuxingEnabled_w(bool enabled) {
RTC_DCHECK_RUN_ON(worker_thread());
if (enabled == payload_type_demuxing_enabled_) {
return;
return true;
}
payload_type_demuxing_enabled_ = enabled;
if (!enabled) {
// TODO(crbug.com/11477): This will remove *all* unsignaled streams (those
// without an explicitly signaled SSRC), which may include streams that
@ -607,10 +608,22 @@ void BaseChannel::SetPayloadTypeDemuxingEnabled_w(bool enabled) {
// streams that were matched based on payload type alone, but currently
// there is no straightforward way to identify those streams.
media_channel()->ResetUnsignaledRecvStream();
ClearHandledPayloadTypes();
RegisterRtpDemuxerSink();
demuxer_criteria_.payload_types.clear();
if (!RegisterRtpDemuxerSink()) {
RTC_LOG(LS_ERROR) << "Failed to disable payload type demuxing for "
<< ToString();
return false;
}
} else if (!payload_types_.empty()) {
demuxer_criteria_.payload_types.insert(payload_types_.begin(),
payload_types_.end());
if (!RegisterRtpDemuxerSink()) {
RTC_LOG(LS_ERROR) << "Failed to enable payload type demuxing for "
<< ToString();
return false;
}
}
payload_type_demuxing_enabled_ = enabled;
return true;
}
bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
@ -754,6 +767,7 @@ bool BaseChannel::UpdateRemoteStreams_w(
// Re-register the sink to update the receiving ssrcs.
if (!RegisterRtpDemuxerSink()) {
RTC_LOG(LS_ERROR) << "Failed to set up demuxing for " << ToString();
ret = false;
}
remote_streams_ = streams;
return ret;
@ -799,10 +813,14 @@ void BaseChannel::MaybeAddHandledPayloadType(int payload_type) {
if (payload_type_demuxing_enabled_) {
demuxer_criteria_.payload_types.insert(static_cast<uint8_t>(payload_type));
}
// Even if payload type demuxing is currently disabled, we need to remember
// the payload types in case it's re-enabled later.
payload_types_.insert(static_cast<uint8_t>(payload_type));
}
void BaseChannel::ClearHandledPayloadTypes() {
demuxer_criteria_.payload_types.clear();
payload_types_.clear();
}
void BaseChannel::FlushRtcpMessages_n() {

6
pc/channel.h
View File

@ -134,7 +134,7 @@ class BaseChannel : public ChannelInterface,
// This method will also remove any existing streams that were bound to this
// channel on the basis of payload type, since one of these streams might
// actually belong to a new channel. See: crbug.com/webrtc/11477
void SetPayloadTypeDemuxingEnabled(bool enabled) override;
bool SetPayloadTypeDemuxingEnabled(bool enabled) override;
bool Enable(bool enable) override;
@ -224,7 +224,7 @@ class BaseChannel : public ChannelInterface,
bool AddRecvStream_w(const StreamParams& sp);
bool RemoveRecvStream_w(uint32_t ssrc);
void ResetUnsignaledRecvStream_w();
void SetPayloadTypeDemuxingEnabled_w(bool enabled);
bool SetPayloadTypeDemuxingEnabled_w(bool enabled);
bool AddSendStream_w(const StreamParams& sp);
bool RemoveSendStream_w(uint32_t ssrc);
@ -322,6 +322,8 @@ class BaseChannel : public ChannelInterface,
webrtc::RtpTransceiverDirection remote_content_direction_ =
webrtc::RtpTransceiverDirection::kInactive;
// Cached list of payload types, used if payload type demuxing is re-enabled.
std::set<uint8_t> payload_types_ RTC_GUARDED_BY(worker_thread());
webrtc::RtpDemuxerCriteria demuxer_criteria_;
// This generator is used to generate SSRCs for local streams.
// This is needed in cases where SSRCs are not negotiated or set explicitly

2
pc/channel_interface.h
View File

@ -52,7 +52,7 @@ class ChannelInterface {
virtual bool SetRemoteContent(const MediaContentDescription* content,
webrtc::SdpType type,
std::string* error_desc) = 0;
virtual void SetPayloadTypeDemuxingEnabled(bool enabled) = 0;
virtual bool SetPayloadTypeDemuxingEnabled(bool enabled) = 0;
// Access to the local and remote streams that were set on the channel.
virtual const std::vector<StreamParams>& local_streams() const = 0;

101
pc/peer_connection_integrationtest.cc
View File

@ -14,6 +14,7 @@
#include <stdio.h>
#include <algorithm>
#include <functional>
#include <list>
#include <map>
@ -2786,6 +2787,106 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
EXPECT_TRUE(ExpectNewFrames(media_expectations));
}
// Used for the test below.
void RemoveBundleGroupSsrcsAndMidExtension(cricket::SessionDescription* desc) {
RemoveSsrcsAndKeepMsids(desc);
desc->RemoveGroupByName("BUNDLE");
for (ContentInfo& content : desc->contents()) {
cricket::MediaContentDescription* media = content.media_description();
cricket::RtpHeaderExtensions extensions = media->rtp_header_extensions();
extensions.erase(std::remove_if(extensions.begin(), extensions.end(),
[](const RtpExtension& extension) {
return extension.uri ==
RtpExtension::kMidUri;
}),
extensions.end());
media->set_rtp_header_extensions(extensions);
}
}
// Tests that video flows between multiple video tracks when BUNDLE is not used,
// SSRCs are not signaled and the MID RTP header extension is not used. This
// relies on demuxing by payload type, which normally doesn't work if you have
// multiple media sections using the same payload type, but which should work as
// long as the media sections aren't bundled.
// Regression test for: http://crbug.com/webrtc/12023
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
EndToEndCallWithTwoVideoTracksNoBundleNoSignaledSsrcAndNoMid) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddVideoTrack();
caller()->AddVideoTrack();
callee()->AddVideoTrack();
callee()->AddVideoTrack();
caller()->SetReceivedSdpMunger(&RemoveBundleGroupSsrcsAndMidExtension);
callee()->SetReceivedSdpMunger(&RemoveBundleGroupSsrcsAndMidExtension);
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ASSERT_EQ(2u, caller()->pc()->GetReceivers().size());
ASSERT_EQ(2u, callee()->pc()->GetReceivers().size());
// Make sure we are not bundled.
ASSERT_NE(caller()->pc()->GetSenders()[0]->dtls_transport(),
caller()->pc()->GetSenders()[1]->dtls_transport());
// Expect video to be received in both directions on both tracks.
MediaExpectations media_expectations;
media_expectations.ExpectBidirectionalVideo();
EXPECT_TRUE(ExpectNewFrames(media_expectations));
}
// Used for the test below.
void ModifyPayloadTypesAndRemoveMidExtension(
cricket::SessionDescription* desc) {
int pt = 96;
for (ContentInfo& content : desc->contents()) {
cricket::MediaContentDescription* media = content.media_description();
cricket::RtpHeaderExtensions extensions = media->rtp_header_extensions();
extensions.erase(std::remove_if(extensions.begin(), extensions.end(),
[](const RtpExtension& extension) {
return extension.uri ==
RtpExtension::kMidUri;
}),
extensions.end());
media->set_rtp_header_extensions(extensions);
cricket::VideoContentDescription* video = media->as_video();
ASSERT_TRUE(video != nullptr);
std::vector<cricket::VideoCodec> codecs = {{pt++, "VP8"}};
video->set_codecs(codecs);
}
}
// Tests that two video tracks can be demultiplexed by payload type alone, by
// using different payload types for the same codec in different m= sections.
// This practice is discouraged but historically has been supported.
// Regression test for: http://crbug.com/webrtc/12029
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
EndToEndCallWithTwoVideoTracksDemultiplexedByPayloadType) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
caller()->AddVideoTrack();
caller()->AddVideoTrack();
callee()->AddVideoTrack();
callee()->AddVideoTrack();
caller()->SetGeneratedSdpMunger(&ModifyPayloadTypesAndRemoveMidExtension);
callee()->SetGeneratedSdpMunger(&ModifyPayloadTypesAndRemoveMidExtension);
// We can't remove SSRCs from the generated SDP because then no send streams
// would be created.
caller()->SetReceivedSdpMunger(&RemoveSsrcsAndKeepMsids);
callee()->SetReceivedSdpMunger(&RemoveSsrcsAndKeepMsids);
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
ASSERT_EQ(2u, caller()->pc()->GetReceivers().size());
ASSERT_EQ(2u, callee()->pc()->GetReceivers().size());
// Make sure we are bundled.
ASSERT_EQ(caller()->pc()->GetSenders()[0]->dtls_transport(),
caller()->pc()->GetSenders()[1]->dtls_transport());
// Expect video to be received in both directions on both tracks.
MediaExpectations media_expectations;
media_expectations.ExpectBidirectionalVideo();
EXPECT_TRUE(ExpectNewFrames(media_expectations));
}
TEST_F(PeerConnectionIntegrationTestUnifiedPlan, NoStreamsMsidLinePresent) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();

103
pc/sdp_offer_answer.cc
View File

@ -12,6 +12,7 @@
#include <algorithm>
#include <queue>
#include <set>
#include "api/media_stream_proxy.h"
#include "api/uma_metrics.h"
@ -4033,7 +4034,13 @@ RTCError SdpOfferAnswerHandler::PushdownMediaDescription(
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(sdesc);
UpdatePayloadTypeDemuxingState(source);
if (!UpdatePayloadTypeDemuxingState(source)) {
// Note that this is never expected to fail, since RtpDemuxer doesn't return
// an error when changing payload type demux criteria, which is all this
// does.
LOG_AND_RETURN_ERROR(RTCErrorType::INTERNAL_ERROR,
"Failed to update payload type demuxing state.");
}
// Push down the new SDP media section for each audio/video transceiver.
for (const auto& transceiver : transceivers().List()) {
@ -4700,21 +4707,33 @@ const std::string SdpOfferAnswerHandler::GetTransportName(
return "";
}
void SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
bool SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
cricket::ContentSource source) {
RTC_DCHECK_RUN_ON(signaling_thread());
// We may need to delete any created default streams and disable creation of
// new ones on the basis of payload type. This is needed to avoid SSRC
// collisions in Call's RtpDemuxer, in the case that a transceiver has
// created a default stream, and then some other channel gets the SSRC
// signaled in the corresponding Unified Plan "m=" section. For more context
// signaled in the corresponding Unified Plan "m=" section. Specifically, we
// need to disable payload type based demuxing when two bundled "m=" sections
// are using the same payload type(s). For more context
// see https://bugs.chromium.org/p/webrtc/issues/detail?id=11477
const SessionDescriptionInterface* sdesc =
(source == cricket::CS_LOCAL ? local_description()
: remote_description());
size_t num_receiving_video_transceivers = 0;
size_t num_receiving_audio_transceivers = 0;
const cricket::ContentGroup* bundle_group =
sdesc->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
std::set<int> audio_payload_types;
std::set<int> video_payload_types;
bool pt_demuxing_enabled_audio = true;
bool pt_demuxing_enabled_video = true;
for (auto& content_info : sdesc->description()->contents()) {
// If this m= section isn't bundled, it's safe to demux by payload type
// since other m= sections using the same payload type will also be using
// different transports.
if (!bundle_group || !bundle_group->HasContentName(content_info.name)) {
continue;
}
if (content_info.rejected ||
(source == cricket::ContentSource::CS_LOCAL &&
!RtpTransceiverDirectionHasRecv(
@ -4726,19 +4745,37 @@ void SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
continue;
}
switch (content_info.media_description()->type()) {
case cricket::MediaType::MEDIA_TYPE_AUDIO:
++num_receiving_audio_transceivers;
case cricket::MediaType::MEDIA_TYPE_AUDIO: {
const cricket::AudioContentDescription* audio_desc =
content_info.media_description()->as_audio();
for (const cricket::AudioCodec& audio : audio_desc->codecs()) {
if (audio_payload_types.count(audio.id)) {
// Two m= sections are using the same payload type, thus demuxing
// by payload type is not possible.
pt_demuxing_enabled_audio = false;
}
audio_payload_types.insert(audio.id);
}
break;
case cricket::MediaType::MEDIA_TYPE_VIDEO:
++num_receiving_video_transceivers;
}
case cricket::MediaType::MEDIA_TYPE_VIDEO: {
const cricket::VideoContentDescription* video_desc =
content_info.media_description()->as_video();
for (const cricket::VideoCodec& video : video_desc->codecs()) {
if (video_payload_types.count(video.id)) {
// Two m= sections are using the same payload type, thus demuxing
// by payload type is not possible.
pt_demuxing_enabled_video = false;
}
video_payload_types.insert(video.id);
}
break;
}
default:
// Ignore data channels.
continue;
}
}
bool pt_demuxing_enabled_video = num_receiving_video_transceivers <= 1;
bool pt_demuxing_enabled_audio = num_receiving_audio_transceivers <= 1;
// Gather all updates ahead of time so that all channels can be updated in a
// single Invoke; necessary due to thread guards.
@ -4760,26 +4797,34 @@ void SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
transceiver->internal()->channel());
}
if (!channels_to_update.empty()) {
pc_->worker_thread()->Invoke<void>(
RTC_FROM_HERE, [&channels_to_update, pt_demuxing_enabled_audio,
pt_demuxing_enabled_video]() {
for (const auto& it : channels_to_update) {
RtpTransceiverDirection local_direction = it.first;
cricket::ChannelInterface* channel = it.second;
cricket::MediaType media_type = channel->media_type();
if (media_type == cricket::MediaType::MEDIA_TYPE_AUDIO) {
channel->SetPayloadTypeDemuxingEnabled(
pt_demuxing_enabled_audio &&
RtpTransceiverDirectionHasRecv(local_direction));
} else if (media_type == cricket::MediaType::MEDIA_TYPE_VIDEO) {
channel->SetPayloadTypeDemuxingEnabled(
pt_demuxing_enabled_video &&
RtpTransceiverDirectionHasRecv(local_direction));
if (channels_to_update.empty()) {
return true;
}
return pc_->worker_thread()->Invoke<bool>(
RTC_FROM_HERE, [&channels_to_update, bundle_group,
pt_demuxing_enabled_audio, pt_demuxing_enabled_video]() {
for (const auto& it : channels_to_update) {
RtpTransceiverDirection local_direction = it.first;
cricket::ChannelInterface* channel = it.second;
cricket::MediaType media_type = channel->media_type();
bool in_bundle_group = (bundle_group && bundle_group->HasContentName(
channel->content_name()));
if (media_type == cricket::MediaType::MEDIA_TYPE_AUDIO) {
if (!channel->SetPayloadTypeDemuxingEnabled(
(!in_bundle_group || pt_demuxing_enabled_audio) &&
RtpTransceiverDirectionHasRecv(local_direction))) {
return false;
}
} else if (media_type == cricket::MediaType::MEDIA_TYPE_VIDEO) {
if (!channel->SetPayloadTypeDemuxingEnabled(
(!in_bundle_group || pt_demuxing_enabled_video) &&
RtpTransceiverDirectionHasRecv(local_direction))) {
return false;
}
}
});
}
}
return true;
});
}
} // namespace webrtc

2
pc/sdp_offer_answer.h
View File

@ -490,7 +490,7 @@ class SdpOfferAnswerHandler {
const std::string GetTransportName(const std::string& content_name);
// Based on number of transceivers per media type, enabled or disable
// payload type based demuxing in the affected channels.
void UpdatePayloadTypeDemuxingState(cricket::ContentSource source);
bool UpdatePayloadTypeDemuxingState(cricket::ContentSource source);
// ==================================================================
// Access to pc_ variables

2
pc/test/mock_channel_interface.h
View File

@ -46,7 +46,7 @@ class MockChannelInterface : public cricket::ChannelInterface {
webrtc::SdpType,
std::string*),
(override));
MOCK_METHOD(void, SetPayloadTypeDemuxingEnabled, (bool), (override));
MOCK_METHOD(bool, SetPayloadTypeDemuxingEnabled, (bool), (override));
MOCK_METHOD(const std::vector<StreamParams>&,
local_streams,
(),