Bug 1616875: Make RtpSourceObserver update stats on main thread, and simplify. r=ng

This essentially implements the "queue a task" step required by the spec, which
ensures that if JS checks stats multiple times without relinquishing the event
loop, it will see the same thing each time.

Differential Revision: https://phabricator.services.mozilla.com/D64468

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Byron Campen [:bwc] 2020-03-19 18:43:12 +00:00
parent 78e76f65fb
commit 4a860bed41
5 changed files with 56 additions and 56 deletions

View File

@ -248,13 +248,12 @@ class RtpSourcesTest : public ::testing::Test {
webrtc::RTPHeader header;
constexpr unsigned int ssrc = 857265;
constexpr unsigned int csrc = 3268365;
constexpr int64_t timestamp = 10000;
constexpr int64_t jitter = 0;
header.ssrc = ssrc;
header.numCSRCs = 1;
header.arrOfCSRCs[0] = csrc;
observer.OnRtpPacket(header, timestamp, jitter);
observer.OnRtpPacket(header, jitter);
// One for the SSRC, one for the CSRC
EXPECT_EQ(observer.mRtpSources.size(), static_cast<size_t>(2));
@ -286,14 +285,13 @@ class RtpSourcesTest : public ::testing::Test {
constexpr unsigned int ssrc = 239485;
constexpr unsigned int csrc0 = 3425;
constexpr unsigned int csrc1 = 36457;
constexpr int64_t timestamp = 10000;
constexpr int64_t jitter = 0;
header.ssrc = ssrc;
header.numCSRCs = 2;
header.arrOfCSRCs[0] = csrc0;
header.arrOfCSRCs[1] = csrc1;
observer.OnRtpPacket(header, timestamp, jitter);
observer.OnRtpPacket(header, jitter);
// One for the SSRC, two for the CSRCs
EXPECT_EQ(observer.mRtpSources.size(), static_cast<size_t>(3));

View File

@ -279,7 +279,7 @@ void WebrtcAudioConduit::OnRtpPacket(const webrtc::RTPHeader& aHeader,
const int64_t aTimestamp,
const uint32_t aJitter) {
ASSERT_ON_THREAD(mStsThread);
mRtpSourceObserver.OnRtpPacket(aHeader, aTimestamp, aJitter);
mRtpSourceObserver->OnRtpPacket(aHeader, aJitter);
}
void WebrtcAudioConduit::OnRtcpBye() {
@ -313,7 +313,7 @@ void WebrtcAudioConduit::SetRtcpEventObserver(
void WebrtcAudioConduit::GetRtpSources(
nsTArray<dom::RTCRtpSourceEntry>& outSources) {
MOZ_ASSERT(NS_IsMainThread());
return mRtpSourceObserver.GetRtpSources(outSources);
return mRtpSourceObserver->GetRtpSources(outSources);
}
// test-only: inserts a CSRC entry in a RtpSourceObserver's history for
@ -337,7 +337,7 @@ void WebrtcAudioConduit::InsertAudioLevelForContributingSource(
const uint8_t aAudioLevel) {
MOZ_ASSERT(NS_IsMainThread());
mozilla::InsertAudioLevelForContributingSource(
mRtpSourceObserver, aCsrcSource, aTimestamp, aRtpTimestamp,
*mRtpSourceObserver, aCsrcSource, aTimestamp, aRtpTimestamp,
aHasAudioLevel, aAudioLevel);
}

View File

@ -17,6 +17,7 @@
// Audio Engine Includes
#include "webrtc/common_types.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_packet_observer.h"
#include "webrtc/modules/audio_device/include/fake_audio_device.h"
#include "webrtc/voice_engine/include/voe_base.h"
#include "webrtc/voice_engine/channel_proxy.h"
@ -197,7 +198,7 @@ class WebrtcAudioConduit : public AudioSessionConduit,
mSendChannel(-1),
mDtmfEnabled(false),
mMutex("WebrtcAudioConduit::mMutex"),
mRtpSourceObserver(mCall->GetTimestampMaker()),
mRtpSourceObserver(new RtpSourceObserver(mCall->GetTimestampMaker())),
mStsThread(aStsThread) {}
virtual ~WebrtcAudioConduit();
@ -358,7 +359,7 @@ class WebrtcAudioConduit : public AudioSessionConduit,
webrtc::AudioFrame mAudioFrame; // for output pulls
// Accessed from both main and mStsThread. Uses locks internally.
RtpSourceObserver mRtpSourceObserver;
RefPtr<RtpSourceObserver> mRtpSourceObserver;
// Socket transport service thread. Any thread.
const nsCOMPtr<nsISerialEventTarget> mStsThread;

View File

@ -24,52 +24,52 @@ double RtpSourceObserver::RtpSourceEntry::ToLinearAudioLevel() const {
RtpSourceObserver::RtpSourceObserver(
const dom::RTCStatsTimestampMaker& aTimestampMaker)
: mMaxJitterWindow(0),
mLevelGuard("RtpSourceObserver::mLevelGuard"),
mTimestampMaker(aTimestampMaker) {}
: mMaxJitterWindow(0), mTimestampMaker(aTimestampMaker) {}
void RtpSourceObserver::OnRtpPacket(const webrtc::RTPHeader& aHeader,
const int64_t aTimestamp,
const uint32_t aJitter) {
MutexAutoLock lock(mLevelGuard);
{
// We assume that aTimestamp is not significantly in the past, and just get
// a JS timestamp for the current time instead of converting aTimestamp.
DOMHighResTimeStamp jsNow = mTimestampMaker.GetNow();
mMaxJitterWindow =
std::max(mMaxJitterWindow, static_cast<int64_t>(aJitter) * 2);
// We are supposed to report the time at which this packet was played out,
// but we have only just received the packet. We try to guess when it will
// be played out.
// TODO: We need to move where we update these stats to MediaPipeline, where
// we send frames to the media track graph. In order to do that, we will
// need to have the ssrc (and csrc) for decoded frames, but we don't have
// that right now. Once we move this to the correct place, we will no longer
// need to keep anything but the most recent data.
const auto predictedPlayoutTime = jsNow + aJitter;
auto& hist = mRtpSources[GetKey(aHeader.ssrc, EntryType::Synchronization)];
hist.Prune(jsNow);
// ssrc-audio-level handling
hist.Insert(jsNow, predictedPlayoutTime, aHeader.timestamp,
aHeader.extension.hasAudioLevel, aHeader.extension.audioLevel);
DOMHighResTimeStamp jsNow = mTimestampMaker.GetNow();
// csrc-audio-level handling
const auto& list = aHeader.extension.csrcAudioLevels;
for (uint8_t i = 0; i < aHeader.numCSRCs; i++) {
const uint32_t& csrc = aHeader.arrOfCSRCs[i];
auto& hist = mRtpSources[GetKey(csrc, EntryType::Contributing)];
hist.Prune(jsNow);
bool hasLevel = i < list.numAudioLevels;
uint8_t level = hasLevel ? list.arrOfAudioLevels[i] : 0;
hist.Insert(jsNow, predictedPlayoutTime, aHeader.timestamp, hasLevel,
level);
}
}
NS_DispatchToMainThread(NS_NewRunnableFunction(
"RtpSourceObserver::OnRtpPacket",
[this, self = RefPtr<RtpSourceObserver>(this), aHeader, aJitter,
jsNow]() {
mMaxJitterWindow =
std::max(mMaxJitterWindow, static_cast<int64_t>(aJitter) * 2);
// We are supposed to report the time at which this packet was played
// out, but we have only just received the packet. We try to guess when
// it will be played out.
// TODO: We need to move where we update these stats to MediaPipeline,
// where we send frames to the media track graph. In order to do that,
// we will need to have the ssrc (and csrc) for decoded frames, but we
// don't have that right now. Once we move this to the correct place, we
// will no longer need to keep anything but the most recent data.
const auto predictedPlayoutTime = jsNow + aJitter;
auto& hist =
mRtpSources[GetKey(aHeader.ssrc, EntryType::Synchronization)];
hist.Prune(jsNow);
// ssrc-audio-level handling
hist.Insert(jsNow, predictedPlayoutTime, aHeader.timestamp,
aHeader.extension.hasAudioLevel,
aHeader.extension.audioLevel);
// csrc-audio-level handling
const auto& list = aHeader.extension.csrcAudioLevels;
for (uint8_t i = 0; i < aHeader.numCSRCs; i++) {
const uint32_t& csrc = aHeader.arrOfCSRCs[i];
auto& hist = mRtpSources[GetKey(csrc, EntryType::Contributing)];
hist.Prune(jsNow);
bool hasLevel = i < list.numAudioLevels;
uint8_t level = hasLevel ? list.arrOfAudioLevels[i] : 0;
hist.Insert(jsNow, predictedPlayoutTime, aHeader.timestamp, hasLevel,
level);
}
}));
}
void RtpSourceObserver::GetRtpSources(
nsTArray<dom::RTCRtpSourceEntry>& outSources) const {
MutexAutoLock lock(mLevelGuard);
MOZ_ASSERT(NS_IsMainThread());
outSources.Clear();
for (const auto& it : mRtpSources) {
const RtpSourceEntry* entry =
@ -90,6 +90,7 @@ void RtpSourceObserver::GetRtpSources(
const RtpSourceObserver::RtpSourceEntry*
RtpSourceObserver::RtpSourceHistory::FindClosestNotAfter(int64_t aTime) const {
MOZ_ASSERT(NS_IsMainThread());
// This method scans the history for the entry whose timestamp is closest to a
// given timestamp but no greater. Because it is scanning forward, it keeps
// track of the closest entry it has found so far in case it overshoots.
@ -117,6 +118,7 @@ RtpSourceObserver::RtpSourceHistory::FindClosestNotAfter(int64_t aTime) const {
}
void RtpSourceObserver::RtpSourceHistory::Prune(const int64_t aTimeNow) {
MOZ_ASSERT(NS_IsMainThread());
const auto aTimeT = aTimeNow - mMaxJitterWindow;
const auto aTimePrehistory = aTimeNow - kHistoryWindow;
bool found = false;
@ -151,12 +153,14 @@ void RtpSourceObserver::RtpSourceHistory::Insert(const int64_t aTimeNow,
const uint32_t aRtpTimestamp,
const bool aHasAudioLevel,
const uint8_t aAudioLevel) {
MOZ_ASSERT(NS_IsMainThread());
Insert(aTimeNow, aTimestamp)
.Update(aTimestamp, aRtpTimestamp, aHasAudioLevel, aAudioLevel);
}
RtpSourceObserver::RtpSourceEntry& RtpSourceObserver::RtpSourceHistory::Insert(
const int64_t aTimeNow, const int64_t aTimestamp) {
MOZ_ASSERT(NS_IsMainThread());
// Time T is the oldest time inside the jitter window (now - jitter)
// Time J is the newest time inside the jitter window (now + jitter)
// Time x is the jitter adjusted entry time

View File

@ -10,10 +10,9 @@
#include <vector>
#include <map>
#include "mozilla/Mutex.h"
#include "nsISupportsImpl.h"
#include "mozilla/dom/RTCRtpSourcesBinding.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_packet_observer.h"
#include "webrtc/common_types.h"
#include "RTCStatsReport.h"
// Unit Test class
@ -28,15 +27,14 @@ namespace mozilla {
* * csrc-audio-level RTP header extension
* * ssrc-audio-level RTP header extension
*/
class RtpSourceObserver : public webrtc::RtpPacketObserver {
class RtpSourceObserver {
public:
explicit RtpSourceObserver(
const dom::RTCStatsTimestampMaker& aTimestampMaker);
virtual ~RtpSourceObserver(){};
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(RtpSourceObserver)
void OnRtpPacket(const webrtc::RTPHeader& aRtpHeader,
const int64_t aTimestamp, const uint32_t aJitter) override;
void OnRtpPacket(const webrtc::RTPHeader& aHeader, const uint32_t aJitter);
/*
* Get the most recent 10 second window of CSRC and SSRC sources.
@ -47,7 +45,8 @@ class RtpSourceObserver : public webrtc::RtpPacketObserver {
void GetRtpSources(nsTArray<dom::RTCRtpSourceEntry>& outSources) const;
private:
// Note: these are pool allocated
virtual ~RtpSourceObserver() = default;
struct RtpSourceEntry {
RtpSourceEntry() = default;
void Update(const int64_t aTimestamp, const uint32_t aRtpTimestamp,
@ -164,8 +163,6 @@ class RtpSourceObserver : public webrtc::RtpPacketObserver {
std::map<uint64_t, RtpSourceHistory> mRtpSources;
// 2 x the largest observed
int64_t mMaxJitterWindow;
// Guards statistics
mutable Mutex mLevelGuard;
dom::RTCStatsTimestampMaker mTimestampMaker;
// Unit test