Revert of WebRtcVoiceMediaChannel::AddRecvStream: Don't call SetRecPayloadType (patchset #13 id:260001 of https://codereview.webrtc.org/2686043006/ )
Reason for revert:
Makes perf and Chromium FYI bots unhappy.
Original issue's description:
> WebRtcVoiceMediaChannel::AddRecvStream: Don't call SetRecPayloadType
>
> This removes one more place where we were unable to handle codecs not
> in the built-in set.
>
> BUG=webrtc:5805
>
> Review-Url: https://codereview.webrtc.org/2686043006
> Cr-Commit-Position: refs/heads/master@{#17370}
> Committed: https://chromium.googlesource.com/external/webrtc/+/1724cfbdbafef4736fedda37312b0286f1eb03d0
TBR=ossu@webrtc.org,solenberg@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2772043002
Cr-Commit-Position: refs/heads/master@{#17374}
diff --git a/webrtc/media/engine/fakewebrtcvoiceengine.h b/webrtc/media/engine/fakewebrtcvoiceengine.h
index 180d06b..e7d95d0 100644
--- a/webrtc/media/engine/fakewebrtcvoiceengine.h
+++ b/webrtc/media/engine/fakewebrtcvoiceengine.h
@@ -117,6 +117,8 @@
return -1;
}
Channel* ch = new Channel();
+ auto db = webrtc::acm2::RentACodec::Database();
+ ch->recv_codecs.assign(db.begin(), db.end());
ch->neteq_capacity = config.acm_config.neteq_config.max_packets_in_buffer;
ch->neteq_fast_accelerate =
config.acm_config.neteq_config.enable_fast_accelerate;
diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc
index 31a9d27..7c6d794 100644
--- a/webrtc/media/engine/webrtcvoiceengine.cc
+++ b/webrtc/media/engine/webrtcvoiceengine.cc
@@ -132,6 +132,13 @@
return ss.str();
}
+std::string ToString(const webrtc::CodecInst& codec) {
+ std::stringstream ss;
+ ss << codec.plname << "/" << codec.plfreq << "/" << codec.channels
+ << " (" << codec.pltype << ")";
+ return ss.str();
+}
+
bool IsCodec(const AudioCodec& codec, const char* ref_name) {
return (_stricmp(codec.name.c_str(), ref_name) == 0);
}
@@ -1459,8 +1466,7 @@
const std::vector<webrtc::RtpExtension>& extensions,
webrtc::Call* call,
webrtc::Transport* rtcp_send_transport,
- const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory,
- const std::map<int, webrtc::SdpAudioFormat>& decoder_map)
+ const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory)
: call_(call), config_() {
RTC_DCHECK_GE(ch, 0);
RTC_DCHECK(call);
@@ -1473,7 +1479,6 @@
config_.voe_channel_id = ch;
config_.sync_group = sync_group;
config_.decoder_factory = decoder_factory;
- config_.decoder_map = decoder_map;
RecreateAudioReceiveStream();
}
@@ -1862,9 +1867,8 @@
ChangePlayout(false);
}
- decoder_map_ = std::move(decoder_map);
for (auto& kv : recv_streams_) {
- kv.second->RecreateAudioReceiveStream(decoder_map_);
+ kv.second->RecreateAudioReceiveStream(decoder_map);
}
recv_codecs_ = codecs;
@@ -2221,12 +2225,38 @@
return false;
}
+ // Turn off all supported codecs.
+ // TODO(solenberg): Remove once "no codecs" is the default state of a stream.
+ for (webrtc::CodecInst voe_codec : webrtc::acm2::RentACodec::Database()) {
+ voe_codec.pltype = -1;
+ if (engine()->voe()->codec()->SetRecPayloadType(channel, voe_codec) == -1) {
+ LOG_RTCERR2(SetRecPayloadType, channel, ToString(voe_codec));
+ DeleteVoEChannel(channel);
+ return false;
+ }
+ }
+
+ // Only enable those configured for this channel.
+ for (const auto& codec : recv_codecs_) {
+ webrtc::CodecInst voe_codec = {0};
+ if (WebRtcVoiceEngine::ToCodecInst(codec, &voe_codec)) {
+ voe_codec.pltype = codec.id;
+ if (engine()->voe()->codec()->SetRecPayloadType(
+ channel, voe_codec) == -1) {
+ LOG_RTCERR2(SetRecPayloadType, channel, ToString(voe_codec));
+ DeleteVoEChannel(channel);
+ return false;
+ }
+ }
+ }
+
recv_streams_.insert(std::make_pair(
- ssrc,
- new WebRtcAudioReceiveStream(
- channel, ssrc, receiver_reports_ssrc_, recv_transport_cc_enabled_,
- recv_nack_enabled_, sp.sync_label, recv_rtp_extensions_, call_, this,
- engine()->decoder_factory_, decoder_map_)));
+ ssrc, new WebRtcAudioReceiveStream(channel, ssrc, receiver_reports_ssrc_,
+ recv_transport_cc_enabled_,
+ recv_nack_enabled_,
+ sp.sync_label, recv_rtp_extensions_,
+ call_, this,
+ engine()->decoder_factory_)));
recv_streams_[ssrc]->SetPlayout(playout_);
return true;
diff --git a/webrtc/media/engine/webrtcvoiceengine.h b/webrtc/media/engine/webrtcvoiceengine.h
index 64f7afa..c777c6c 100644
--- a/webrtc/media/engine/webrtcvoiceengine.h
+++ b/webrtc/media/engine/webrtcvoiceengine.h
@@ -251,12 +251,7 @@
WebRtcVoiceEngine* const engine_ = nullptr;
std::vector<AudioCodec> send_codecs_;
-
- // TODO(kwiberg): decoder_map_ and recv_codecs_ store the exact same
- // information, in slightly different formats. Eliminate recv_codecs_.
- std::map<int, webrtc::SdpAudioFormat> decoder_map_;
std::vector<AudioCodec> recv_codecs_;
-
int max_send_bitrate_bps_ = 0;
AudioOptions options_;
rtc::Optional<int> dtmf_payload_type_;
diff --git a/webrtc/media/engine/webrtcvoiceengine_unittest.cc b/webrtc/media/engine/webrtcvoiceengine_unittest.cc
index 3777b3d..2d41ecd 100644
--- a/webrtc/media/engine/webrtcvoiceengine_unittest.cc
+++ b/webrtc/media/engine/webrtcvoiceengine_unittest.cc
@@ -31,7 +31,6 @@
#include "webrtc/test/gtest.h"
#include "webrtc/voice_engine/transmit_mixer.h"
-using testing::ContainerEq;
using testing::Return;
using testing::StrictMock;
@@ -796,12 +795,26 @@
parameters.codecs[2].id = 126;
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
EXPECT_TRUE(AddRecvStream(kSsrcX));
- EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}},
- {106, {"ISAC", 16000, 1}},
- {126, {"telephone-event", 8000, 1}},
- {107, {"telephone-event", 32000, 1}}})));
+ int channel_num = voe_.GetLastChannel();
+
+ webrtc::CodecInst gcodec;
+ rtc::strcpyn(gcodec.plname, arraysize(gcodec.plname), "ISAC");
+ gcodec.plfreq = 16000;
+ gcodec.channels = 1;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num, gcodec));
+ EXPECT_EQ(106, gcodec.pltype);
+ EXPECT_STREQ("ISAC", gcodec.plname);
+
+ rtc::strcpyn(gcodec.plname, arraysize(gcodec.plname), "telephone-event");
+ gcodec.plfreq = 8000;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num, gcodec));
+ EXPECT_EQ(126, gcodec.pltype);
+ EXPECT_STREQ("telephone-event", gcodec.plname);
+
+ gcodec.plfreq = 32000;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num, gcodec));
+ EXPECT_EQ(107, gcodec.pltype);
+ EXPECT_STREQ("telephone-event", gcodec.plname);
}
// Test that we fail to set an unknown inbound codec.
@@ -832,11 +845,16 @@
parameters.codecs.push_back(kOpusCodec);
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
EXPECT_TRUE(AddRecvStream(kSsrcX));
- EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}},
- {103, {"ISAC", 16000, 1}},
- {111, {"opus", 48000, 2}}})));
+ int channel_num = voe_.GetLastChannel();
+ webrtc::CodecInst opus;
+ cricket::WebRtcVoiceEngine::ToCodecInst(kOpusCodec, &opus);
+ // Even without stereo parameters, recv codecs still specify channels = 2.
+ EXPECT_EQ(2, opus.channels);
+ EXPECT_EQ(111, opus.pltype);
+ EXPECT_STREQ("opus", opus.plname);
+ opus.pltype = 0;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num, opus));
+ EXPECT_EQ(111, opus.pltype);
}
// Test that we can decode OPUS with stereo = 0.
@@ -849,11 +867,16 @@
parameters.codecs[2].params["stereo"] = "0";
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
EXPECT_TRUE(AddRecvStream(kSsrcX));
- EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}},
- {103, {"ISAC", 16000, 1}},
- {111, {"opus", 48000, 2, {{"stereo", "0"}}}}})));
+ int channel_num2 = voe_.GetLastChannel();
+ webrtc::CodecInst opus;
+ cricket::WebRtcVoiceEngine::ToCodecInst(kOpusCodec, &opus);
+ // Even when stereo is off, recv codecs still specify channels = 2.
+ EXPECT_EQ(2, opus.channels);
+ EXPECT_EQ(111, opus.pltype);
+ EXPECT_STREQ("opus", opus.plname);
+ opus.pltype = 0;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num2, opus));
+ EXPECT_EQ(111, opus.pltype);
}
// Test that we can decode OPUS with stereo = 1.
@@ -866,11 +889,15 @@
parameters.codecs[2].params["stereo"] = "1";
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
EXPECT_TRUE(AddRecvStream(kSsrcX));
- EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}},
- {103, {"ISAC", 16000, 1}},
- {111, {"opus", 48000, 2, {{"stereo", "1"}}}}})));
+ int channel_num2 = voe_.GetLastChannel();
+ webrtc::CodecInst opus;
+ cricket::WebRtcVoiceEngine::ToCodecInst(kOpusCodec, &opus);
+ EXPECT_EQ(2, opus.channels);
+ EXPECT_EQ(111, opus.pltype);
+ EXPECT_STREQ("opus", opus.plname);
+ opus.pltype = 0;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num2, opus));
+ EXPECT_EQ(111, opus.pltype);
}
// Test that changes to recv codecs are applied to all streams.
@@ -884,15 +911,28 @@
parameters.codecs[0].id = 106; // collide with existing CN 32k
parameters.codecs[2].id = 126;
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
- for (const auto& ssrc : {kSsrcX, kSsrcY}) {
- EXPECT_TRUE(AddRecvStream(ssrc));
- EXPECT_THAT(GetRecvStreamConfig(ssrc).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}},
- {106, {"ISAC", 16000, 1}},
- {126, {"telephone-event", 8000, 1}},
- {107, {"telephone-event", 32000, 1}}})));
- }
+ EXPECT_TRUE(AddRecvStream(kSsrcX));
+ int channel_num2 = voe_.GetLastChannel();
+
+ webrtc::CodecInst gcodec;
+ rtc::strcpyn(gcodec.plname, arraysize(gcodec.plname), "ISAC");
+ gcodec.plfreq = 16000;
+ gcodec.channels = 1;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num2, gcodec));
+ EXPECT_EQ(106, gcodec.pltype);
+ EXPECT_STREQ("ISAC", gcodec.plname);
+
+ rtc::strcpyn(gcodec.plname, arraysize(gcodec.plname), "telephone-event");
+ gcodec.plfreq = 8000;
+ gcodec.channels = 1;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num2, gcodec));
+ EXPECT_EQ(126, gcodec.pltype);
+ EXPECT_STREQ("telephone-event", gcodec.plname);
+
+ gcodec.plfreq = 32000;
+ EXPECT_EQ(0, voe_.GetRecPayloadType(channel_num2, gcodec));
+ EXPECT_EQ(107, gcodec.pltype);
+ EXPECT_STREQ("telephone-event", gcodec.plname);
}
TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsAfterAddingStreams) {
@@ -2921,9 +2961,12 @@
parameters.codecs.push_back(kPcmuCodec);
EXPECT_TRUE(channel_->SetRecvParameters(parameters));
EXPECT_TRUE(AddRecvStream(kSsrcX));
- EXPECT_THAT(GetRecvStreamConfig(kSsrcX).decoder_map,
- (ContainerEq<std::map<int, webrtc::SdpAudioFormat>>(
- {{0, {"PCMU", 8000, 1}}, {103, {"ISAC", 16000, 1}}})));
+ int channel_num2 = voe_.GetLastChannel();
+ webrtc::CodecInst gcodec;
+ rtc::strcpyn(gcodec.plname, arraysize(gcodec.plname), "opus");
+ gcodec.plfreq = 48000;
+ gcodec.channels = 2;
+ EXPECT_EQ(-1, voe_.GetRecPayloadType(channel_num2, gcodec));
}
// Test that we properly clean up any streams that were added, even if