From 37b8b11661cce2691adff93ccc6bb6d1ada1bc3a Mon Sep 17 00:00:00 2001 From: kwiberg Date: Thu, 3 Nov 2016 02:46:53 -0700 Subject: [PATCH] Revert of Removed the legacy behavior of stopping playout when setting new receive codecs. (patchset #1 id:1 of https://codereview.webrtc.org/2409483003/ ) Reason for revert: Reverting because of the reasons given in comment #16: "This change breaks a scenario that is unfortunately not covered by unit tests, but can easily happen in a real call. The scenario that is broken by the change is this: 1. A sends an offer to B, with a set of codecs C_a (which is a subset of C_b, the codecs supported by B) 2. B responds with an answer, and sets the receive codecs to C_a. 3. At a later time, B generates a new offer which by default includes all codecs in C_b. 4. B calls SetLocalDescription() with this offer, that adds new receive codecs. 5. Adding the new codecs fails, because of the check at https://cs.chromium.org/chromium/src/third_party/webrtc/voice_engine/channel..... This causes SetLocalDescription() itself to fail. The net effect is that B cannot set a local description it just generated. Before the CL mentioned above, we'd stop playout before changing the codecs, and the operation would succeed." Original issue's description: > Removed the legacy behavior of stopping playout when setting new receive codecs. > > BUG=webrtc:4690 > > Committed: https://crrev.com/917d4e1e7131f35764cff932a8793151585e8179 > Cr-Commit-Position: refs/heads/master@{#14610} TBR=solenberg@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:4690 Review-Url: https://codereview.webrtc.org/2478433003 Cr-Commit-Position: refs/heads/master@{#14905} --- webrtc/media/engine/webrtcvoiceengine.cc | 16 +++++++++++++++- webrtc/media/engine/webrtcvoiceengine.h | 2 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc index fc901ae1e6..99369a2547 100644 --- a/webrtc/media/engine/webrtcvoiceengine.cc +++ b/webrtc/media/engine/webrtcvoiceengine.cc @@ -1801,6 +1801,12 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs( return true; } + if (playout_) { + // Receive codecs can not be changed while playing. So we temporarily + // pause playout. + ChangePlayout(false); + } + bool result = true; for (const AudioCodec& codec : new_codecs) { webrtc::CodecInst voe_codec = {0}; @@ -1825,6 +1831,9 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs( recv_codecs_ = codecs; } + if (desired_playout_ && !playout_) { + ChangePlayout(desired_playout_); + } return result; } @@ -1953,7 +1962,12 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs( } void WebRtcVoiceMediaChannel::SetPlayout(bool playout) { - TRACE_EVENT0("webrtc", "WebRtcVoiceMediaChannel::SetPlayout"); + desired_playout_ = playout; + return ChangePlayout(desired_playout_); +} + +void WebRtcVoiceMediaChannel::ChangePlayout(bool playout) { + TRACE_EVENT0("webrtc", "WebRtcVoiceMediaChannel::ChangePlayout"); RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); if (playout_ == playout) { return; diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h index cd832a1c08..df2f64d53e 100644 --- a/webrtc/media/engine/webrtcvoiceengine.h +++ b/webrtc/media/engine/webrtcvoiceengine.h @@ -226,6 +226,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, WebRtcVoiceEngine* engine() { return engine_; } int GetLastEngineError() { return engine()->GetLastEngineError(); } int GetOutputLevel(int channel); + void ChangePlayout(bool playout); int CreateVoEChannel(); bool DeleteVoEChannel(int channel); bool IsDefaultRecvStream(uint32_t ssrc) { @@ -243,6 +244,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel, int max_send_bitrate_bps_ = 0; AudioOptions options_; rtc::Optional dtmf_payload_type_; + bool desired_playout_ = false; bool recv_transport_cc_enabled_ = false; bool recv_nack_enabled_ = false; bool playout_ = false;