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

Revert "Always offer transport sequence number header extension for audio"

Browse Source 
This reverts commit fd965c008c7bc395bb276f260262ac11ccd25406.

Reason for revert: Cause test failure.

Original change's description:
> Always offer transport sequence number header extension for audio
> 
> If the extension is negotiated, it will only be used if
> the field trial WebRTC-Audio-SendSideBwe is enabled.
> This allows simpler experimentation if it should be used or not.
> 
> Bug: webrtc:10309 webrtc:10286
> Change-Id: I797e6f14c06d46189e40f6d09805c2e09afc015b
> Reviewed-on: https://webrtc-review.googlesource.com/c/122542
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#26689}

TBR=ossu@webrtc.org,sprang@webrtc.org,srte@webrtc.org,perkj@webrtc.org

Change-Id: I1b7d3fa5c282a5bf049ca54695ad16c8278a2698
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10309 webrtc:10286
Reviewed-on: https://webrtc-review.googlesource.com/c/123182
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Commit-Queue: Ying Wang <yinwa@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26703}
This commit is contained in:
Ying Wang
2019-02-15 08:53:09 +00:00
committed by Commit Bot
parent 7e0e44f8b4
commit 397c06fe9d
8 changed files with 48 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
audio/BUILD.gn
View File

@ -149,7 +149,6 @@ if (rtc_include_tests) {
"../logging:rtc_event_log_api",
"../modules/audio_device:mock_audio_device",
"../rtc_base:rtc_base_tests_utils",
"../test:field_trial",
# For TestAudioDeviceModule
"../modules/audio_device:audio_device_impl",

4
audio/audio_send_stream.cc
View File

@ -264,8 +264,8 @@ void AudioSendStream::ConfigureStream(
RtcpBandwidthObserver* bandwidth_observer = nullptr;
if (stream->allocation_settings_.ShouldSendTransportSequenceNumber(
new_ids.transport_sequence_number)) {
if (stream->allocation_settings_.IncludeAudioInFeedback(
new_ids.transport_sequence_number != 0)) {
channel_send->EnableSendTransportSequenceNumber(
new_ids.transport_sequence_number);
// Probing in application limited region is only used in combination with

72
audio/audio_send_stream_tests.cc
View File

@ -9,8 +9,6 @@
*/
#include "test/call_test.h"
#include "test/constants.h"
#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/rtcp_packet_parser.h"
@ -139,54 +137,42 @@ TEST_F(AudioSendStreamCallTest, SupportsAudioLevel) {
RunBaseTest(&test);
}
class TransportWideSequenceNumberObserver : public AudioSendTest {
public:
explicit TransportWideSequenceNumberObserver(bool expect_sequence_number)
: AudioSendTest(), expect_sequence_number_(expect_sequence_number) {
EXPECT_TRUE(parser_->RegisterRtpHeaderExtension(
kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId));
}
TEST_F(AudioSendStreamCallTest, SupportsTransportWideSequenceNumbers) {
static const uint8_t kExtensionId = test::kTransportSequenceNumberExtensionId;
class TransportWideSequenceNumberObserver : public AudioSendTest {
public:
TransportWideSequenceNumberObserver() : AudioSendTest() {
EXPECT_TRUE(parser_->RegisterRtpHeaderExtension(
kRtpExtensionTransportSequenceNumber, kExtensionId));
}
private:
Action OnSendRtp(const uint8_t* packet, size_t length) override {
RTPHeader header;
EXPECT_TRUE(parser_->Parse(packet, length, &header));
private:
Action OnSendRtp(const uint8_t* packet, size_t length) override {
RTPHeader header;
EXPECT_TRUE(parser_->Parse(packet, length, &header));
EXPECT_EQ(header.extension.hasTransportSequenceNumber,
expect_sequence_number_);
EXPECT_FALSE(header.extension.hasTransmissionTimeOffset);
EXPECT_FALSE(header.extension.hasAbsoluteSendTime);
EXPECT_TRUE(header.extension.hasTransportSequenceNumber);
EXPECT_FALSE(header.extension.hasTransmissionTimeOffset);
EXPECT_FALSE(header.extension.hasAbsoluteSendTime);
observation_complete_.Set();
observation_complete_.Set();
return SEND_PACKET;
}
return SEND_PACKET;
}
void ModifyAudioConfigs(
AudioSendStream::Config* send_config,
std::vector<AudioReceiveStream::Config>* receive_configs) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
kTransportSequenceNumberExtensionId));
}
void ModifyAudioConfigs(
AudioSendStream::Config* send_config,
std::vector<AudioReceiveStream::Config>* receive_configs) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
}
void PerformTest() override {
EXPECT_TRUE(Wait()) << "Timed out while waiting for a single RTP packet.";
}
const bool expect_sequence_number_;
};
void PerformTest() override {
EXPECT_TRUE(Wait()) << "Timed out while waiting for a single RTP packet.";
}
} test;
TEST_F(AudioSendStreamCallTest, SendsTransportWideSequenceNumbersInFieldTrial) {
ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
TransportWideSequenceNumberObserver test(/*expect_sequence_number=*/true);
RunBaseTest(&test);
}
TEST_F(AudioSendStreamCallTest,
DoesNotSendTransportWideSequenceNumbersPerDefault) {
TransportWideSequenceNumberObserver test(/*expect_sequence_number=*/false);
RunBaseTest(&test);
}

3
audio/audio_send_stream_unittest.cc
View File

@ -28,7 +28,6 @@
#include "modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h"
#include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"
#include "rtc_base/task_queue.h"
#include "test/field_trial.h"
#include "test/gtest.h"
#include "test/mock_audio_encoder.h"
#include "test/mock_audio_encoder_factory.h"
@ -354,7 +353,6 @@ TEST(AudioSendStreamTest, SetMuted) {
}
TEST(AudioSendStreamTest, AudioBweCorrectObjectsOnChannelProxy) {
ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
ConfigHelper helper(true, true);
auto send_stream = helper.CreateAudioSendStream();
}
@ -506,7 +504,6 @@ TEST(AudioSendStreamTest, DontRecreateEncoder) {
}
TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) {
ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
auto new_config = helper.config();

7
media/engine/webrtc_voice_engine.cc
View File

@ -559,8 +559,11 @@ RtpCapabilities WebRtcVoiceEngine::GetCapabilities() const {
int id = 1;
capabilities.header_extensions.push_back(
webrtc::RtpExtension(webrtc::RtpExtension::kAudioLevelUri, id++));
capabilities.header_extensions.push_back(webrtc::RtpExtension(
webrtc::RtpExtension::kTransportSequenceNumberUri, id++));
if (allocation_settings_.EnableTransportSequenceNumberExtension()) {
capabilities.header_extensions.push_back(webrtc::RtpExtension(
webrtc::RtpExtension::kTransportSequenceNumberUri, id++));
}
return capabilities;
}

10
rtc_base/experiments/audio_allocation_settings.cc
View File

@ -65,12 +65,16 @@ bool AudioAllocationSettings::ConfigureRateAllocationRange() const {
return audio_send_side_bwe_;
}
bool AudioAllocationSettings::ShouldSendTransportSequenceNumber(
bool AudioAllocationSettings::EnableTransportSequenceNumberExtension() const {
// TODO(srte): Update this to be more accurate.
return audio_send_side_bwe_ && !allocate_audio_without_feedback_;
}
bool AudioAllocationSettings::IncludeAudioInFeedback(
int transport_seq_num_extension_header_id) const {
if (force_no_audio_feedback_)
return false;
return audio_send_side_bwe_ && !allocate_audio_without_feedback_ &&
transport_seq_num_extension_header_id != 0;
return transport_seq_num_extension_header_id != 0;
}
bool AudioAllocationSettings::UpdateAudioTargetBitrate(

9
rtc_base/experiments/audio_allocation_settings.h
View File

@ -27,13 +27,14 @@ class AudioAllocationSettings {
bool IgnoreSeqNumIdChange() const;
// Returns true if the bitrate allocation range should be configured.
bool ConfigureRateAllocationRange() const;
// Returns true if sent audio packets should have transport wide sequence
// numbers.
// Returns true if the transport sequence number extension should be enabled.
bool EnableTransportSequenceNumberExtension() const;
// Returns true if audio traffic should be included in transport wide feedback
// packets.
// |transport_seq_num_extension_header_id| the extension header id for
// transport sequence numbers. Set to 0 if not the extension is not
// configured.
bool ShouldSendTransportSequenceNumber(
int transport_seq_num_extension_header_id) const;
bool IncludeAudioInFeedback(int transport_seq_num_extension_header_id) const;
// Returns true if target bitrate for audio streams should be updated.
// |transport_seq_num_extension_header_id| the extension header id for
// transport sequence numbers. Set to 0 if not the extension is not

2
video/end_to_end_tests/transport_feedback_tests.cc
View File

@ -316,7 +316,6 @@ TEST_F(TransportFeedbackEndToEndTest, VideoTransportFeedbackNotConfigured) {
}
TEST_F(TransportFeedbackEndToEndTest, AudioReceivesTransportFeedback) {
test::ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
TransportFeedbackTester test(true, 0, 1);
RunBaseTest(&test);
}
@ -425,7 +424,6 @@ TEST_F(TransportFeedbackEndToEndTest,
}
TEST_F(TransportFeedbackEndToEndTest, TransportSeqNumOnAudioAndVideo) {
test::ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
static constexpr int kExtensionId = 8;
static constexpr size_t kMinPacketsToWaitFor = 50;
class TransportSequenceNumberTest : public test::EndToEndTest {