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: 1724cfbdba
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}
This commit is contained in:
@ -117,6 +117,8 @@ class FakeWebRtcVoiceEngine
|
||||
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;
|
||||
|
||||
@ -132,6 +132,13 @@ std::string ToString(const AudioCodec& codec) {
|
||||
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 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
|
||||
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 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
|
||||
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 @@ bool WebRtcVoiceMediaChannel::SetRecvCodecs(
|
||||
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 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
|
||||
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;
|
||||
|
||||
@ -251,12 +251,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
|
||||
|
||||
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_;
|
||||
|
||||
@ -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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecs) {
|
||||
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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsWithOpusNoStereo) {
|
||||
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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsWithOpus0Stereo) {
|
||||
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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsWithOpus1Stereo) {
|
||||
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 @@ TEST_F(WebRtcVoiceEngineTestFake, SetRecvCodecsWithMultipleStreams) {
|
||||
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 @@ TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamUnsupportedCodec) {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user