Ignore FEC packet in stats, if it is first packet on ssrc.

BUG=chrome:410456
R=mflodman@webrtc.org, tommi@webrtc.org

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7096 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
sprang@webrtc.org
2014-09-08 08:20:18 +00:00
parent 6d08ca6379
commit c30e9e2300
3 changed files with 70 additions and 57 deletions

View File

@ -408,36 +408,31 @@ ReceiveStatisticsImpl::~ReceiveStatisticsImpl() {
void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header,
size_t bytes, size_t bytes,
bool retransmitted) { bool retransmitted) {
StatisticianImplMap::iterator it; StreamStatisticianImpl* impl;
{ {
CriticalSectionScoped cs(receive_statistics_lock_.get()); CriticalSectionScoped cs(receive_statistics_lock_.get());
it = statisticians_.find(header.ssrc); StatisticianImplMap::iterator it = statisticians_.find(header.ssrc);
if (it == statisticians_.end()) { if (it != statisticians_.end()) {
std::pair<StatisticianImplMap::iterator, uint32_t> insert_result = impl = it->second;
statisticians_.insert(std::make_pair( } else {
header.ssrc, new StreamStatisticianImpl(clock_, this, this))); impl = new StreamStatisticianImpl(clock_, this, this);
it = insert_result.first; statisticians_[header.ssrc] = impl;
} }
} }
it->second->IncomingPacket(header, bytes, retransmitted); // StreamStatisticianImpl instance is created once and only destroyed when
// this whole ReceiveStatisticsImpl is destroyed. StreamStatisticianImpl has
// it's own locking so don't hold receive_statistics_lock_ (potential
// deadlock).
impl->IncomingPacket(header, bytes, retransmitted);
} }
void ReceiveStatisticsImpl::FecPacketReceived(uint32_t ssrc) { void ReceiveStatisticsImpl::FecPacketReceived(uint32_t ssrc) {
CriticalSectionScoped cs(receive_statistics_lock_.get()); CriticalSectionScoped cs(receive_statistics_lock_.get());
StatisticianImplMap::iterator it = statisticians_.find(ssrc); StatisticianImplMap::iterator it = statisticians_.find(ssrc);
assert(it != statisticians_.end()); // Ignore FEC if it is the first packet.
if (it != statisticians_.end()) {
it->second->FecPacketReceived(); it->second->FecPacketReceived();
} }
void ReceiveStatisticsImpl::ChangeSsrc(uint32_t from_ssrc, uint32_t to_ssrc) {
CriticalSectionScoped cs(receive_statistics_lock_.get());
StatisticianImplMap::iterator from_it = statisticians_.find(from_ssrc);
if (from_it == statisticians_.end())
return;
if (statisticians_.find(to_ssrc) != statisticians_.end())
return;
statisticians_[to_ssrc] = from_it->second;
statisticians_.erase(from_it);
} }
StatisticianMap ReceiveStatisticsImpl::GetActiveStatisticians() const { StatisticianMap ReceiveStatisticsImpl::GetActiveStatisticians() const {

View File

@ -114,8 +114,6 @@ class ReceiveStatisticsImpl : public ReceiveStatistics,
virtual int32_t Process() OVERRIDE; virtual int32_t Process() OVERRIDE;
virtual int32_t TimeUntilNextProcess() OVERRIDE; virtual int32_t TimeUntilNextProcess() OVERRIDE;
void ChangeSsrc(uint32_t from_ssrc, uint32_t to_ssrc);
virtual void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback) virtual void RegisterRtcpStatisticsCallback(RtcpStatisticsCallback* callback)
OVERRIDE; OVERRIDE;

View File

@ -219,12 +219,11 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) {
EXPECT_EQ(1u, callback.num_calls_); EXPECT_EQ(1u, callback.num_calls_);
} }
TEST_F(ReceiveStatisticsTest, RtpCallbacks) { class RtpTestCallback : public StreamDataCountersCallback {
class TestCallback : public StreamDataCountersCallback {
public: public:
TestCallback() RtpTestCallback()
: StreamDataCountersCallback(), num_calls_(0), ssrc_(0), stats_() {} : StreamDataCountersCallback(), num_calls_(0), ssrc_(0), stats_() {}
virtual ~TestCallback() {} virtual ~RtpTestCallback() {}
virtual void DataCountersUpdated(const StreamDataCounters& counters, virtual void DataCountersUpdated(const StreamDataCounters& counters,
uint32_t ssrc) { uint32_t ssrc) {
@ -252,8 +251,10 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) {
uint32_t num_calls_; uint32_t num_calls_;
uint32_t ssrc_; uint32_t ssrc_;
StreamDataCounters stats_; StreamDataCounters stats_;
} callback; };
TEST_F(ReceiveStatisticsTest, RtpCallbacks) {
RtpTestCallback callback;
receive_statistics_->RegisterRtpStatisticsCallback(&callback); receive_statistics_->RegisterRtpStatisticsCallback(&callback);
const uint32_t kHeaderLength = 20; const uint32_t kHeaderLength = 20;
@ -300,4 +301,23 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) {
callback.ExpectMatches( callback.ExpectMatches(
5, kSsrc1, 4 * kPacketSize1, kPaddingLength * 2, 4, 1, 1); 5, kSsrc1, 4 * kPacketSize1, kPaddingLength * 2, 4, 1, 1);
} }
TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) {
RtpTestCallback callback;
receive_statistics_->RegisterRtpStatisticsCallback(&callback);
const uint32_t kHeaderLength = 20;
// If first packet is FEC, ignore it.
receive_statistics_->FecPacketReceived(kSsrc1);
EXPECT_EQ(0u, callback.num_calls_);
header1_.headerLength = kHeaderLength;
receive_statistics_->IncomingPacket(
header1_, kPacketSize1 + kHeaderLength, false);
callback.ExpectMatches(1, kSsrc1, kPacketSize1, 0, 1, 0, 0);
receive_statistics_->FecPacketReceived(kSsrc1);
callback.ExpectMatches(2, kSsrc1, kPacketSize1, 0, 1, 0, 1);
}
} // namespace webrtc } // namespace webrtc