From bd933ee29a2fe35871ba0094847fe282f4a02900 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Wed, 2 Jun 2021 16:17:35 +0200 Subject: [PATCH] SdpOfferAnswerHandler: Significantly reduce audio impairment. It was found from Chrome tracing that worker packet progression in https://webrtc.github.io/samples/src/content/peerconnection/negotiate-timing/ during renegotiation of 100 transceivers is hindered by a multi-hundred millisecond Invoke from the signaling to the worker thread. This causes audio impairment. Fix this by splitting the single Invoke into a series of Invokes, allowing packets received during the renegotiation to be processed between the worker invocations. Experimental data of negotiation from 1 to 100 video transceivers WebRtcDistinctWorkerThread OFF, before change: 4415.60 milliseconds, audio impairment 29760 4216.00 milliseconds, audio impairment 25560 4298.40 milliseconds, audio impairment 25440 WebRtcDistinctWorkerThread OFF, after change: 4258.70 milliseconds, audio impairment 26280 4255.50 milliseconds, audio impairment 25920 4363.10 milliseconds, audio impairment 25200 WebRtcDistinctWorkerThread ON, before change: 4407.80 milliseconds, audio impairment 24840 4541.00 milliseconds, audio impairment 26160 4377.80 milliseconds, audio impairment 17040 WebRtcDistinctWorkerThread ON, after change: 4364.80 milliseconds, audio impairment 0 4174.30 milliseconds, audio impairment 0 4309.00 milliseconds, audio impairment 0 We should reconsider this split after lazy decoders and decoder stream projects have shipped, see - bugs.webrtc.org/12462 - crbug.com/1157227 - crbug.com/1187289 Bug: webrtc:12840, webrtc:12462, chromium:1157227, chromium:1187289 Change-Id: I8e3b3943bd76f09da74b457690799415335b51f5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221103 Commit-Queue: Markus Handell Reviewed-by: Tommi Cr-Commit-Position: refs/heads/master@{#34202} --- pc/sdp_offer_answer.cc | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 30d07dbcb4..761ae7cfae 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -4213,18 +4213,25 @@ RTCError SdpOfferAnswerHandler::PushdownMediaDescription( channels.push_back(std::make_pair(channel, content_desc)); } - if (!channels.empty()) { + // This for-loop of invokes helps audio impairment during re-negotiations. + // One of the causes is that downstairs decoder creation is synchronous at the + // moment, and that a decoder is created for each codec listed in the SDP. + // + // TODO(bugs.webrtc.org/12840): consider merging the invokes again after + // these projects have shipped: + // - bugs.webrtc.org/12462 + // - crbug.com/1157227 + // - crbug.com/1187289 + for (const auto& entry : channels) { RTCError error = pc_->worker_thread()->Invoke(RTC_FROM_HERE, [&]() { std::string error; - for (const auto& entry : channels) { - bool success = - (source == cricket::CS_LOCAL) - ? entry.first->SetLocalContent(entry.second, type, &error) - : entry.first->SetRemoteContent(entry.second, type, &error); - if (!success) { - LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, error); - } + bool success = + (source == cricket::CS_LOCAL) + ? entry.first->SetLocalContent(entry.second, type, &error) + : entry.first->SetRemoteContent(entry.second, type, &error); + if (!success) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, error); } return RTCError::OK(); });