admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor UpdatePayloadTypeDemuxingState and add documentation.

Browse Source 
This simplifies the work that happens on the worker thread in
preparation of avoiding having to go to the worker at all.

Bug: webrtc:11993
Change-Id: I13f063bdecce8efdb978ef1976c819019f30e020
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244082
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35610}
This commit is contained in:
Tomas Gunnarsson 2022-01-02 20:40:22 +00:00 committed by WebRTC LUCI CQ
parent f643aea8ac
commit 92f9b74df7
2 changed files with 69 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pc/channel.cc
View File

@ -302,6 +302,12 @@ bool BaseChannel::SetRemoteContent(const MediaContentDescription* content,
}
bool BaseChannel::SetPayloadTypeDemuxingEnabled(bool enabled) {
// TODO(bugs.webrtc.org/11993): The demuxer state needs to be managed on the
// network thread. At the moment there's a workaround for inconsistent state
// between the worker and network thread because of this (see
// OnDemuxerCriteriaUpdatePending elsewhere in this file) and
// SetPayloadTypeDemuxingEnabled_w has an Invoke over to the network thread
// to apply state updates.
RTC_DCHECK_RUN_ON(worker_thread());
TRACE_EVENT0("webrtc", "BaseChannel::SetPayloadTypeDemuxingEnabled");
return SetPayloadTypeDemuxingEnabled_w(enabled);

122
pc/sdp_offer_answer.cc
View File

@ -4947,29 +4947,6 @@ bool SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
}
}
// Gather all updates ahead of time so that all channels can be updated in a
// single Invoke; necessary due to thread guards.
std::vector<std::pair<RtpTransceiverDirection, cricket::ChannelInterface*>>
channels_to_update;
for (const auto& transceiver : transceivers()->ListInternal()) {
cricket::ChannelInterface* channel = transceiver->channel();
const ContentInfo* content =
FindMediaSectionForTransceiver(transceiver, sdesc);
if (!channel || !content) {
continue;
}
RtpTransceiverDirection local_direction =
content->media_description()->direction();
if (source == cricket::CS_REMOTE) {
local_direction = RtpTransceiverDirectionReversed(local_direction);
}
channels_to_update.emplace_back(local_direction, transceiver->channel());
}
if (channels_to_update.empty()) {
return true;
}
// In Unified Plan, payload type demuxing is useful for legacy endpoints that
// don't support the MID header extension, but it can also cause incorrrect
// forwarding of packets when going from one m= section to multiple m=
@ -4996,44 +4973,71 @@ bool SdpOfferAnswerHandler::UpdatePayloadTypeDemuxingState(
bundled_pt_demux_allowed_video = true;
}
// Gather all updates ahead of time so that all channels can be updated in a
// single Invoke; necessary due to thread guards.
std::vector<std::pair<bool, cricket::ChannelInterface*>> channels_to_update;
for (const auto& transceiver : transceivers()->ListInternal()) {
cricket::ChannelInterface* channel = transceiver->channel();
const ContentInfo* content =
FindMediaSectionForTransceiver(transceiver, sdesc);
if (!channel || !content) {
continue;
}
const cricket::MediaType media_type = channel->media_type();
if (media_type != cricket::MediaType::MEDIA_TYPE_AUDIO &&
media_type != cricket::MediaType::MEDIA_TYPE_VIDEO) {
continue;
}
RtpTransceiverDirection local_direction =
content->media_description()->direction();
if (source == cricket::CS_REMOTE) {
local_direction = RtpTransceiverDirectionReversed(local_direction);
}
auto bundle_it = bundle_groups_by_mid.find(channel->content_name());
const cricket::ContentGroup* bundle_group =
bundle_it != bundle_groups_by_mid.end() ? bundle_it->second : nullptr;
bool pt_demux_enabled = RtpTransceiverDirectionHasRecv(local_direction);
if (media_type == cricket::MediaType::MEDIA_TYPE_AUDIO) {
pt_demux_enabled &=
!bundle_group ||
(bundled_pt_demux_allowed_audio &&
payload_types_by_bundle[bundle_group].pt_demuxing_possible_audio);
if (pt_demux_enabled) {
pt_demuxing_has_been_used_audio_ = true;
}
} else {
RTC_DCHECK_EQ(media_type, cricket::MediaType::MEDIA_TYPE_VIDEO);
pt_demux_enabled &=
!bundle_group ||
(bundled_pt_demux_allowed_video &&
payload_types_by_bundle[bundle_group].pt_demuxing_possible_video);
if (pt_demux_enabled) {
pt_demuxing_has_been_used_video_ = true;
}
}
channels_to_update.emplace_back(pt_demux_enabled, transceiver->channel());
}
if (channels_to_update.empty()) {
return true;
}
// TODO(bugs.webrtc.org/11993): This Invoke() will also invoke on the network
// thread for every demuxer sink that needs to be updated. The demuxer state
// needs to be fully (and only) managed on the network thread and once that's
// the case, there's no need to stop by on the worker. Ideally we could also
// do this without blocking.
return pc_->worker_thread()->Invoke<bool>(
RTC_FROM_HERE,
[&channels_to_update, &bundle_groups_by_mid, &payload_types_by_bundle,
bundled_pt_demux_allowed_audio, bundled_pt_demux_allowed_video,
pt_demuxing_has_been_used_audio = &pt_demuxing_has_been_used_audio_,
pt_demuxing_has_been_used_video = &pt_demuxing_has_been_used_video_]() {
RTC_FROM_HERE, [&channels_to_update]() {
for (const auto& it : channels_to_update) {
RtpTransceiverDirection local_direction = it.first;
cricket::ChannelInterface* channel = it.second;
cricket::MediaType media_type = channel->media_type();
auto bundle_it = bundle_groups_by_mid.find(channel->content_name());
const cricket::ContentGroup* bundle_group =
bundle_it != bundle_groups_by_mid.end() ? bundle_it->second
: nullptr;
if (media_type == cricket::MediaType::MEDIA_TYPE_AUDIO) {
bool pt_demux_enabled =
RtpTransceiverDirectionHasRecv(local_direction) &&
(!bundle_group || (bundled_pt_demux_allowed_audio &&
payload_types_by_bundle[bundle_group]
.pt_demuxing_possible_audio));
if (pt_demux_enabled) {
*pt_demuxing_has_been_used_audio = true;
}
if (!channel->SetPayloadTypeDemuxingEnabled(pt_demux_enabled)) {
return false;
}
} else if (media_type == cricket::MediaType::MEDIA_TYPE_VIDEO) {
bool pt_demux_enabled =
RtpTransceiverDirectionHasRecv(local_direction) &&
(!bundle_group || (bundled_pt_demux_allowed_video &&
payload_types_by_bundle[bundle_group]
.pt_demuxing_possible_video));
if (pt_demux_enabled) {
*pt_demuxing_has_been_used_video = true;
}
if (!channel->SetPayloadTypeDemuxingEnabled(pt_demux_enabled)) {
return false;
}
if (!it.second->SetPayloadTypeDemuxingEnabled(it.first)) {
// Note that the state has already been irrevocably changed at this
// point. Is it useful to stop the loop?
return false;
}
}
return true;