Callback for send bitrate estimates - new roll

Issue https://webrtc-codereview.appspot.com/4459004/ was commited as
r5259, after which flakiness was detected and a rollback was performed
at r5261.

Patch Set 1 of this issue is the code submitted in r5259. Subsequent
patch sets fixes a race condition which caused the seen problems.

The root cause was a dead lock between a thread sending rtp packets and
and a timed module processing thread:

webrtc::RTPSender::BitrateUpdated()  // Get RTPSender stats lock
webrtc::Bitrate::Process()  // Get Bitrate lock
webrtc::RTPSender::ProcessBitrate()
webrtc::ModuleRtpRtcpImpl::Process()
...

webrtc::Bitrate::Update()  // Get Bitrate lock
webrtc::RTPSender::UpdateRtpStats()  // Get RTPSender stats lock
webrtc::RTPSender::SendToNetwork()
...

This is fixed in Bitrate::Process() by releasing the lock before
calling the callback.

BUG=2235
R=mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/5619004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5281 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
sprang@webrtc.org
2013-12-13 09:46:59 +00:00
parent f3973e81d5
commit 6811b6e308
15 changed files with 322 additions and 97 deletions

View File

@ -38,6 +38,7 @@ const uint32_t kAbsoluteSendTime = 0x00aabbcc;
const uint8_t kAudioLevel = 0x5a;
const uint8_t kAudioLevelExtensionId = 9;
const int kAudioPayload = 103;
const uint64_t kStartTime = 123456789;
} // namespace
using testing::_;
@ -81,12 +82,12 @@ class LoopbackTransportTest : public webrtc::Transport {
class RtpSenderTest : public ::testing::Test {
protected:
RtpSenderTest()
: fake_clock_(123456789),
mock_paced_sender_(),
rtp_sender_(),
payload_(kPayload),
transport_(),
kMarkerBit(true) {
: fake_clock_(kStartTime),
mock_paced_sender_(),
rtp_sender_(),
payload_(kPayload),
transport_(),
kMarkerBit(true) {
EXPECT_CALL(mock_paced_sender_,
SendPacket(_, _, _, _, _, _)).WillRepeatedly(testing::Return(true));
}
@ -779,6 +780,73 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) {
rtp_sender_->RegisterFrameCountObserver(NULL);
}
TEST_F(RtpSenderTest, BitrateCallbacks) {
class TestCallback : public BitrateStatisticsObserver {
public:
TestCallback()
: BitrateStatisticsObserver(), num_calls_(0), ssrc_(0), bitrate_() {}
virtual ~TestCallback() {}
virtual void Notify(const BitrateStatistics& stats, uint32_t ssrc) {
++num_calls_;
ssrc_ = ssrc;
bitrate_ = stats;
}
uint32_t num_calls_;
uint32_t ssrc_;
BitrateStatistics bitrate_;
} callback;
// Simulate kNumPackets sent with kPacketInterval ms intervals.
const uint32_t kNumPackets = 15;
const uint32_t kPacketInterval = 20;
// Overhead = 12 bytes RTP header + 1 byte generic header.
const uint32_t kPacketOverhead = 13;
char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC";
const uint8_t payload_type = 127;
ASSERT_EQ(
0,
rtp_sender_->RegisterPayload(payload_name, payload_type, 90000, 0, 1500));
uint8_t payload[] = {47, 11, 32, 93, 89};
rtp_sender_->SetStorePacketsStatus(true, 1);
uint32_t ssrc = rtp_sender_->SSRC();
rtp_sender_->RegisterBitrateObserver(&callback);
// Initial process call so we get a new time window.
rtp_sender_->ProcessBitrate();
uint64_t start_time = fake_clock_.CurrentNtpInMilliseconds();
// Send a few frames.
for (uint32_t i = 0; i < kNumPackets; ++i) {
ASSERT_EQ(0,
rtp_sender_->SendOutgoingData(kVideoFrameKey,
payload_type,
1234,
4321,
payload,
sizeof(payload),
0));
fake_clock_.AdvanceTimeMilliseconds(kPacketInterval);
}
rtp_sender_->ProcessBitrate();
const uint32_t expected_packet_rate = 1000 / kPacketInterval;
EXPECT_EQ(1U, callback.num_calls_);
EXPECT_EQ(ssrc, callback.ssrc_);
EXPECT_EQ(start_time + (kNumPackets * kPacketInterval),
callback.bitrate_.timestamp_ms);
EXPECT_EQ(expected_packet_rate, callback.bitrate_.packet_rate);
EXPECT_EQ((kPacketOverhead + sizeof(payload)) * 8 * expected_packet_rate,
callback.bitrate_.bitrate_bps);
rtp_sender_->RegisterBitrateObserver(NULL);
}
class RtpSenderAudioTest : public RtpSenderTest {
protected:
RtpSenderAudioTest() {}