zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Control combined_audio_video_bwe with config bool.

Browse Source 
Permits setting RTP extensions for AudioReceiveStream without enabling
combined A/V BWE. This prevents spamming the log with "Failed to find
extension id:".

BUG=webrtc:4870
R=mflodman@webrtc.org, stefan@webrtc.org

Review URL: https://codereview.webrtc.org/1256803004

Cr-Commit-Position: refs/heads/master@{#9633}
This commit is contained in:
pbos
2015-07-24 07:10:18 -07:00
committed by Commit bot
parent cfd5f96307
commit 6bb1b6e7fe
5 changed files with 37 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
talk/media/webrtc/webrtcvoiceengine.cc
View File

@ -3637,9 +3637,9 @@ void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) {
webrtc::AudioReceiveStream::Config config;
config.rtp.remote_ssrc = ssrc;
// Only add RTP extensions if we support combined A/V BWE.
if (options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false)) {
config.rtp.extensions = recv_rtp_extensions_;
}
config.rtp.extensions = recv_rtp_extensions_;
config.combined_audio_video_bwe =
options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false);
config.voe_channel_id = channel->channel();
config.sync_group = receive_stream_params_[ssrc].sync_label;
webrtc::AudioReceiveStream* s = call_->CreateAudioReceiveStream(config);

55
talk/media/webrtc/webrtcvoiceengine_unittest.cc
View File

@ -3381,7 +3381,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) {
media_channel->SetCall(nullptr);
}
TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) {
TEST_F(WebRtcVoiceEngineTestFake, CanChangeCombinedBweOption) {
// Test that changing the combined_audio_video_bwe option results in the
// expected state changes on an associated Call.
cricket::FakeCall call(webrtc::Call::Config(nullptr));
@ -3399,42 +3399,42 @@ TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) {
EXPECT_TRUE(media_channel->AddRecvStream(
cricket::StreamParams::CreateLegacy(kAudioSsrc2)));
// Combined BWE should not be set up yet (no RTP extensions).
// Combined BWE should not be set up yet.
EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.rtp.extensions.empty());
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.rtp.extensions.empty());
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.combined_audio_video_bwe);
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.combined_audio_video_bwe);
// Enable combined BWE option - now it should be set up.
cricket::AudioOptions options;
options.combined_audio_video_bwe.Set(true);
EXPECT_TRUE(media_channel->SetOptions(options));
EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.rtp.extensions.empty());
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.rtp.extensions.empty());
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.combined_audio_video_bwe);
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.combined_audio_video_bwe);
// Disable combined BWE option - should be disabled again.
options.combined_audio_video_bwe.Set(false);
EXPECT_TRUE(media_channel->SetOptions(options));
EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.rtp.extensions.empty());
EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.rtp.extensions.empty());
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1)
->GetConfig()
.combined_audio_video_bwe);
EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2)
->GetConfig()
.combined_audio_video_bwe);
media_channel->SetCall(nullptr);
}
TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBwe_Call) {
TEST_F(WebRtcVoiceEngineTestFake, SetCallConfiguresAudioReceiveChannels) {
// Test that calling SetCall() on the voice media channel results in the
// expected state changes in Call.
cricket::FakeCall call(webrtc::Call::Config(nullptr));
@ -3445,9 +3445,6 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBwe_Call) {
EXPECT_TRUE(SetupEngine());
cricket::WebRtcVoiceMediaChannel* media_channel =
static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
cricket::AudioOptions options;
options.combined_audio_video_bwe.Set(true);
EXPECT_TRUE(media_channel->SetOptions(options));
EXPECT_TRUE(media_channel->AddRecvStream(
cricket::StreamParams::CreateLegacy(kAudioSsrc1)));
EXPECT_TRUE(media_channel->AddRecvStream(
@ -3474,7 +3471,7 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBwe_Call) {
EXPECT_EQ(0, call.GetAudioReceiveStreams().size());
}
TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams_Call) {
TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams) {
// Test that adding receive streams after enabling combined bandwidth
// estimation will correctly configure each channel.
cricket::FakeCall call(webrtc::Call::Config(nullptr));
@ -3492,6 +3489,9 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams_Call) {
EXPECT_TRUE(media_channel->AddRecvStream(
cricket::StreamParams::CreateLegacy(kSsrcs[i])));
EXPECT_NE(nullptr, call.GetAudioReceiveStream(kSsrcs[i]));
EXPECT_TRUE(call.GetAudioReceiveStream(kSsrcs[i])
->GetConfig()
.combined_audio_video_bwe);
}
EXPECT_EQ(ARRAY_SIZE(kSsrcs), call.GetAudioReceiveStreams().size());
@ -3499,7 +3499,7 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams_Call) {
EXPECT_EQ(0, call.GetAudioReceiveStreams().size());
}
TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweExtensions_Call) {
TEST_F(WebRtcVoiceEngineTestFake, ConfiguresAudioReceiveStreamRtpExtensions) {
// Test that setting the header extensions results in the expected state
// changes on an associated Call.
cricket::FakeCall call(webrtc::Call::Config(nullptr));
@ -3511,9 +3511,6 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweExtensions_Call) {
cricket::WebRtcVoiceMediaChannel* media_channel =
static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
media_channel->SetCall(&call);
cricket::AudioOptions options;
options.combined_audio_video_bwe.Set(true);
EXPECT_TRUE(media_channel->SetOptions(options));
for (uint32 ssrc : ssrcs) {
EXPECT_TRUE(media_channel->AddRecvStream(
cricket::StreamParams::CreateLegacy(ssrc)));

3
webrtc/audio_receive_stream.h
View File

@ -59,6 +59,9 @@ class AudioReceiveStream : public ReceiveStream {
// Call::CreateReceiveStream().
// TODO(solenberg): Use unique_ptr<> once our std lib fully supports C++11.
std::map<uint8_t, AudioDecoder*> decoder_map;
// TODO(pbos): Remove config option once combined A/V BWE is always on.
bool combined_audio_video_bwe = false;
};
virtual Stats GetStats() const = 0;

6
webrtc/video/audio_receive_stream.cc
View File

@ -86,13 +86,15 @@ bool AudioReceiveStream::DeliverRtcp(const uint8_t* packet, size_t length) {
bool AudioReceiveStream::DeliverRtp(const uint8_t* packet, size_t length) {
RTPHeader header;
if (!rtp_header_parser_->Parse(packet, length, &header)) {
return false;
}
// Only forward if the parsed header has absolute sender time. RTP time stamps
// Only forward if the parsed header has absolute sender time. RTP timestamps
// may have different rates for audio and video and shouldn't be mixed.
if (header.extension.hasAbsoluteSendTime) {
if (config_.combined_audio_video_bwe &&
header.extension.hasAbsoluteSendTime) {
int64_t arrival_time_ms = TickTime::MillisecondTimestamp();
size_t payload_size = length - header.headerLength;
remote_bitrate_estimator_->IncomingPacket(arrival_time_ms, payload_size,

1
webrtc/video/bitrate_estimator_tests.cc
View File

@ -210,6 +210,7 @@ class BitrateEstimatorTest : public test::CallTest {
receive_config.voe_channel_id = 0;
receive_config.rtp.extensions.push_back(
RtpExtension(RtpExtension::kAbsSendTime, kASTExtensionId));
receive_config.combined_audio_video_bwe = true;
audio_receive_stream_ = test_->receiver_call_->CreateAudioReceiveStream(
receive_config);
} else {