The isRemote from has been removed RTCInboundRTPStreamStats, RTCOutboundRTPStreamStats. Instead new dictionaries exist for the local and remote, inbound and outbound stats. See the following spec PR: https://github.com/w3c/webrtc-stats/pull/191

Setting P1 since you're working on it.

Comment on attachment 8892161 [details] Bug 1380555 - refactor out isRemote; https://reviewboard.mozilla.org/r/163130/#review168534 Looks good! Some problems with ReadParam() and a general increase in conditional code-paths, which I wonder if we need, + enough minor issues that I'd like to see it again. ::: dom/media/tests/mochitest/pc.js:1665 (Diff revision 2) > if (res.isRemote) { > continue; > } We can remove this now. ::: dom/media/tests/mochitest/pc.js:1689 (Diff revision 2) > ok(res.packetsReceived !== undefined, "Rtp packetsReceived"); > ok(res.bytesReceived >= res.packetsReceived, "Rtp bytesReceived"); > } > if (res.remoteId) { > var rem = stats.get(res.remoteId); > - ok(rem.isRemote, "Remote is rtcp"); > + ok(rem.type, "Remote is rtcp"); seems redundant. ::: dom/media/tests/mochitest/pc.js:1696 (Diff revision 2) > - if (false) { // Bug 1325430 if (!this.disableRtpCountChecking) { > + if (false) { // Bug 1325430 if (!this.disableRtpCountChecking) { > - // no guarantee which one is newer! > + // no guarantee which one is newer! > - // Note: this must change when we add a timestamp field to remote RTCP reports > + // Note: this must change when we add a timestamp field to remote RTCP reports > - // and make rem.timestamp be the reception time > + // and make rem.timestamp be the reception time > - if (res.timestamp >= rem.timestamp) { > + if (res.timestamp >= rem.timestamp) { > ok(rem.packetsReceived <= res.packetsSent, "No more than sent packets"); > - } else { > + } else { > info("REVERSED timestamps: rec:" + > - rem.packetsReceived + " time:" + rem.timestamp + " sent:" + res.packetsSent + " time:" + res.timestamp); > + rem.packetsReceived + " time:" + rem.timestamp + " sent:" + res.packetsSent + " time:" + res.timestamp); > - } > + } > - // Else we may have received more than outdated Rtcp packetsSent > + // Else we may have received more than outdated Rtcp packetsSent Odd indent ::: dom/media/tests/mochitest/test_peerConnection_stats.html:26 (Diff revision 2) > - "burstPacketsLost", "burstLossCount", "burstDiscardCount", > + return this.types.includes(type); > - "gapDiscardRate", "gapLossRate",], > - deprecated: ["mozRtt"], > - }, > + }, > - "outbound-rtp": { > - expected: ["id", "timestamp", "type", "ssrc", "isRemote", "mediaType", > + }; > + if (superTypeDefinition.types) { When is this ever falsy? ::: dom/media/tests/mochitest/test_peerConnection_stats.html:29 (Diff revision 2) > - localVideoOnly: ["droppedFrames", "bitrateMean", "bitrateStdDev", > - "framerateMean", "framerateStdDev", "framesEncoded", "firCount", > + for (let i of ["expected", "optional", "unimplemented", "localVideoOnly", > + "deprecated"]) { odd indent ::: dom/media/tests/mochitest/test_peerConnection_stats.html:54 (Diff revision 2) > + "mediaType",], > + optional: ["nackCount",], > + unimplemented: ["mediaTrackId", > + "transportId", > + "codecId", > + "sliCount", > + "qpSum"], inconsistent tail-commas ::: dom/media/tests/mochitest/test_peerConnection_stats.html:95 (Diff revision 2) > +}); > + > +var statsExpectedByType = { > + "inbound-rtp": extendTypeWith(rtcReceivedRtpStreamStats, { > + name: "inbound-rtp", > + optional: ["remoteId",], expected ::: dom/media/tests/mochitest/test_peerConnection_stats.html:114 (Diff revision 2) > + }), > + "outbound-rtp": extendTypeWith(rtcSentRtpStreamStats, { > + name: "outbound-rtp", > + expected: ["packetsSent", > + "bytesSent",], > + optional: ["remoteId", expected ::: dom/media/tests/mochitest/test_peerConnection_stats.html:263 (Diff revision 2) > ok(["audio", "video"].includes(stat.mediaType), > stat.type + ".mediaType is 'audio' or 'video'"); > > // remote id > - if (stat.remoteId) { > - ok(report.has(stat.remoteId), "remoteId exists in report."); > + if (stat.inner.remoteId) { > + ok(report.has(stat.remoteId), "localId exists in report."); s/localId/remoteId/ ::: dom/media/tests/mochitest/test_peerConnection_stats.html:276 (Diff revision 2) > + if (stat.inner.localId) { > + ok(report.has(stat.localId), "localId exists in report."); > + is(report.get(stat.localId).ssrc, stat.ssrc, > + "remote ssrc and local ssrc match."); > + is(report.get(stat.localId).remoteId, stat.id, > "remote object has local object as it's own remote object."); "local object has remote object as it's own remote object." - I think. ::: dom/media/tests/mochitest/test_peerConnection_stats.html:574 (Diff revision 2) > var waitForSyncedRtcp = async pc => { > // Ensures that RTCP is present > let ensureSyncedRtcp = async () => { > let stats = await pc.getStats(); > for (let [k, v] of stats) { > - if (v.type.endsWith("bound-rtp") && !v.remoteId) { > + if (v.type.endsWith("bound-rtp") && (!v.remoteId && !v.localId)) { Redundant parens. ::: dom/media/tests/mochitest/test_peerConnection_stats.html:574 (Diff revision 2) > - if (v.type.endsWith("bound-rtp") && !v.remoteId) { > + if (v.type.endsWith("bound-rtp") && (!v.remoteId && !v.localId)) { > throw new Error(v.id + " is missing remoteId: " "is missing remoteId or localId" - actually, maybe break this test up with specific log messages? ::: dom/media/webrtc/WebrtcGlobal.h:35 (Diff revision 2) > } > > WriteParam(aMsg, false); > } > > - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, paramType* aReslt) s/aReslt/aResult/ ::: dom/media/webrtc/WebrtcGlobal.h:65 (Diff revision 2) > static void Write(Message* aMsg, const paramType& aParam) > { > WriteParam(aMsg, static_cast<const FallibleTArray<T>&>(aParam)); > } > > - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, paramType* aReslt) s/aReslt/aResult/ ::: dom/media/webrtc/WebrtcGlobal.h:165 (Diff revision 2) > -static bool ReadRTCStats(const Message* aMsg, PickleIterator* aIter, RTCStats* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, RTCStats* aReslt) > -{ > + { > - // RTCStats base class > + // RTCStats base class > - if (!ReadParam(aMsg, aIter, &(aResult->mId)) || > + if (!aMsg || !aItr || !aReslt || There seem to be a lot more conditionals here now. How come, and can they be avoided? ::: dom/media/webrtc/WebrtcGlobal.h:272 (Diff revision 2) > WriteParam(aMsg, aParam.mPortNumber); > WriteParam(aMsg, aParam.mTransport); > - WriteRTCStats(aMsg, aParam); > + ParamTraits<mozilla::dom::RTCStats>::Write(aMsg, aParam); > } > > - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, paramType* aReslt) s/aReslt/aResult/ ::: dom/media/webrtc/WebrtcGlobal.h:306 (Diff revision 2) > - WriteRTCStats(aMsg, aParam); > + ParamTraits<mozilla::dom::RTCStats>::Write(aMsg, aParam); > } > > - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, paramType* aReslt) > { > - if (!ReadParam(aMsg, aIter, &(aResult->mActiveConnection)) || > + if (!ReadParam(aMsg, aItr, &(aReslt->mActiveConnection)) || Here you don't check for !aMsg || !aItr, how come you had to elsewhere? Fewer if's == better. ::: dom/media/webrtc/WebrtcGlobal.h:323 (Diff revision 2) > -static void WriteRTCRTPStreamStats( > - Message* aMsg, > +template<> > +struct ParamTraits<mozilla::dom::RTCRTPStreamStats> > - const mozilla::dom::RTCRTPStreamStats& aParam) > { > - WriteParam(aMsg, aParam.mBitrateMean); > - WriteParam(aMsg, aParam.mBitrateStdDev); > + typedef mozilla::dom::RTCRTPStreamStats paramType; > + typedef mozilla::dom::RTCStats superType; Did you try regular inheritance here? Just curious. ::: dom/media/webrtc/WebrtcGlobal.h:342 (Diff revision 2) > - mozilla::dom::RTCRTPStreamStats* aResult) > -{ > - if (!ReadParam(aMsg, aIter, &(aResult->mBitrateMean)) || > - !ReadParam(aMsg, aIter, &(aResult->mBitrateStdDev)) || > - !ReadParam(aMsg, aIter, &(aResult->mCodecId)) || > - !ReadParam(aMsg, aIter, &(aResult->mFramerateMean)) || > - !ReadParam(aMsg, aIter, &(aResult->mFramerateStdDev)) || > - !ReadParam(aMsg, aIter, &(aResult->mIsRemote)) || > - !ReadParam(aMsg, aIter, &(aResult->mMediaTrackId)) || > - !ReadParam(aMsg, aIter, &(aResult->mMediaType)) || > + if (!aMsg || !aItr || !aReslt || > + !ReadParam(aMsg, aItr, &(aReslt->mSsrc)) || > + !ReadParam(aMsg, aItr, &(aReslt->mMediaType)) || > + !ReadParam(aMsg, aItr, &(aReslt->mMediaTrackId)) || > + !ReadParam(aMsg, aItr, &(aReslt->mTransportId)) || > + !ReadParam(aMsg, aItr, &(aReslt->mCodecId)) || > + !ReadParam(aMsg, aItr, &(aReslt->mFirCount)) || > + !ReadParam(aMsg, aItr, &(aReslt->mNackCount)) || > + !ReadParam(aMsg, aItr, &(aReslt->mPliCount))) { > + return ParamTraits<superType>::Read(aMsg, aItr, aReslt); This only calls superType::Read if reading fails. That seems wrong. Applies throughout. Have we considered reversing this pattern to: return ReadParam(aMsg, aItr, &(aReslt->mSsrc)) && ReadParam(aMsg, aItr, &(aReslt->mMediaType)) && ..., ParamTraits<superType>::Read(aMsg, aItr, aReslt)); ? ::: dom/media/webrtc/WebrtcGlobal.h:442 (Diff revision 2) > - static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) > + static bool Read(const Message* aMsg, PickleIterator* aItr, paramType* aReslt) > { > - if (!ReadParam(aMsg, aIter, &(aResult->mBytesReceived)) || > - !ReadParam(aMsg, aIter, &(aResult->mDiscardedPackets)) || > - !ReadParam(aMsg, aIter, &(aResult->mFramesDecoded)) || > - !ReadParam(aMsg, aIter, &(aResult->mJitter)) || > + if (!aMsg || !aItr || !aReslt || > + !ReadParam(aMsg, aItr, &(aReslt->mLocalId)) || > + !ReadParam(aMsg, aItr, &(aReslt->mRoundTripTime))) { > + return ParamTraits<superType>::Read(aMsg, aItr, aReslt); s/aReslt/aResult/ ::: dom/webidl/RTCStatsReport.webidl (Diff revision 2) > - // Local only measurements, RTCP related but not communicated via RTCP. Not > - // present in RTCP case. > unsigned long firCount; > unsigned long pliCount; > unsigned long nackCount; Hmm, reading this comment makes me wonder: Should these be moved in the spec instead? Maybe retain the comment if it is still valid, or add a bug # if this is outstanding work? ::: dom/webidl/RTCStatsReport.webidl:42 (Diff revision 2) > -dictionary RTCInboundRTPStreamStats : RTCRTPStreamStats { > - unsigned long packetsReceived; > +dictionary RTCReceivedRTPStreamStats : RTCRTPStreamStats { > + unsigned long packetsReceived; > - unsigned long long bytesReceived; > + unsigned long long bytesReceived; > - double jitter; > - unsigned long packetsLost; > - long mozAvSyncDelay; > + unsigned long packetsLost; > + double jitter; > + double fractionLost; We seem to be using an indent of 2 in this file. Let's keep it consistent whatever it is. ::: dom/webidl/RTCStatsReport.webidl:48 (Diff revision 2) > - unsigned long packetsReceived; > + unsigned long packetsReceived; > - unsigned long long bytesReceived; > + unsigned long long bytesReceived; > - double jitter; > - unsigned long packetsLost; > - long mozAvSyncDelay; > - long mozJitterBufferDelay; > + unsigned long packetsLost; > + double jitter; > + double fractionLost; > + // unsigned long discardedPackets; TODO @@NG handle backwards compat in JS You have this implemented in this patch AFAICT, so maybe just a comment rather than a TODO? ::: dom/webidl/RTCStatsReport.webidl:49 (Diff revision 2) > - unsigned long long bytesReceived; > + unsigned long long bytesReceived; > - double jitter; > - unsigned long packetsLost; > - long mozAvSyncDelay; > - long mozJitterBufferDelay; > - long roundTripTime; > + unsigned long packetsLost; > + double jitter; > + double fractionLost; > + // unsigned long discardedPackets; TODO @@NG handle backwards compat in JS > + // packetsDiscarded not yet supported for audio bug #? ::: dom/webidl/RTCStatsReport.webidl:55 (Diff revision 2) > + long mozAvSyncDelay; // TODO @@NG remove - create bug > + long mozJitterBufferDelay; // TODO @@NG remove - create bug bug # ::: dom/webidl/RTCStatsReport.webidl:70 (Diff revision 2) > - unsigned long packetsSent; > + unsigned long packetsSent; > - unsigned long long bytesSent; > + unsigned long long bytesSent; 2 indent ::: dom/webidl/RTCStatsReport.webidl:76 (Diff revision 2) > - unsigned long long bytesSent; > + unsigned long long bytesSent; > - double targetBitrate; // config encoder bitrate target of this SSRC in bits/s > +}; > + > +dictionary RTCOutboundRTPStreamStats : RTCSentRTPStreamStats { > + DOMString remoteId; > + double targetBitrate; // config encoder bitrate target of this SSRC in bits/s lose the comment ::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp:248 (Diff revision 2) > - if (s.mIsRemote) { > - id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_PACKETLOSS_RATE : > - WEBRTC_VIDEO_QUALITY_OUTBOUND_PACKETLOSS_RATE; > - } else { > + // TODO @@NG > + // if (s.mIsRemote) { > + // id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_PACKETLOSS_RATE : > + // WEBRTC_VIDEO_QUALITY_OUTBOUND_PACKETLOSS_RATE; > + // } else { You have some work left moving some telemetry code. I understand it's coming in a separate patch, but I'm opening an issue still so we don't forget. Note that it's generally frowned on to check in commented out code. ::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:3817 (Diff revision 2) > - RTCOutboundRTPStreamStats s; > + // TODO @@NG make sure that all fields are being populated > + // TODO @@NG change "remoteIds" Todo ::: media/webrtc/signaling/src/peerconnection/WebrtcGlobalInformation.cpp:1119 (Diff revision 2) > for (decltype(array.Length()) i = 0; i < array.Length(); i++) { > auto& s = array[i]; I wonder: can we do a for (auto& s : query->report->mOutboundRTPStreamStats.Value()) here now? I may be wrong. Worth a try.

Comment on attachment 8892161 [details] Bug 1380555 - refactor out isRemote; https://reviewboard.mozilla.org/r/163130/#review168534 > When is this ever falsy? Yes "var rtcStats = extendTypeWith({}, ..." This may be more concise though newType.types = [...newType.types, ...superTypeDefinition.types || []]; > expected localId is alway expected, remoteId is only expected if RTCP has arrived > expected see above > There seem to be a lot more conditionals here now. How come, and can they be avoided? Are you referring to checking the naked pointers before using them? > Did you try regular inheritance here? Just curious. There is no way around explicitly specifying the super type that I can think of, and this seems pretty readable. > This only calls superType::Read if reading fails. That seems wrong. Applies throughout. > > Have we considered reversing this pattern to: > > return ReadParam(aMsg, aItr, &(aReslt->mSsrc)) && > ReadParam(aMsg, aItr, &(aReslt->mMediaType)) && > ..., > ParamTraits<superType>::Read(aMsg, aItr, aReslt)); > > ? I like it. I will fix the superType::read, turning the whole file upside down seems like a job for another patch though, which is fine because I also want to check the raw pointers in the methods I didn't need to touch. > You have some work left moving some telemetry code. I understand it's coming in a separate patch, but I'm opening an issue still so we don't forget. > > Note that it's generally frowned on to check in commented out code. I am leaving the issue open for the reason you mentioned, but I have removed the commented code. > I wonder: can we do a > > for (auto& s : query->report->mOutboundRTPStreamStats.Value()) > > here now? I may be wrong. Worth a try. Yes, it does!

Comment on attachment 8892161 [details] Bug 1380555 - refactor out isRemote; https://reviewboard.mozilla.org/r/163130/#review171036 Nice! Missed flipping a few ReadParam's that you're touching anyway due to renaming. Also, don't forget DOM review. ::: dom/media/webrtc/WebrtcGlobal.h:223 (Diff revisions 2 - 3) > - if (!ReadParam(aMsg, aItr, &(aReslt->mTransportId)) || > - !ReadParam(aMsg, aItr, &(aReslt->mLocalCandidateId)) || > - !ReadParam(aMsg, aItr, &(aReslt->mPriority)) || > - !ReadParam(aMsg, aItr, &(aReslt->mNominated)) || > - !ReadParam(aMsg, aItr, &(aReslt->mWritable)) || > - !ReadParam(aMsg, aItr, &(aReslt->mReadable)) || > - !ReadParam(aMsg, aItr, &(aReslt->mRemoteCandidateId)) || > - !ReadParam(aMsg, aItr, &(aReslt->mSelected)) || > - !ReadParam(aMsg, aItr, &(aReslt->mState)) || > - !ReadParam(aMsg, aItr, &(aReslt->mBytesSent)) || > - !ReadParam(aMsg, aItr, &(aReslt->mBytesReceived)) || > - !ReadParam(aMsg, aItr, &(aReslt->mLastPacketSentTimestamp)) || > - !ReadParam(aMsg, aItr, &(aReslt->mLastPacketReceivedTimestamp)) || > - !ParamTraits<mozilla::dom::RTCStats>::Read(aMsg, aItr, aReslt)) { > + if (!ReadParam(aMsg, aItr, &(aResult->mTransportId)) || > + !ReadParam(aMsg, aItr, &(aResult->mLocalCandidateId)) || > + !ReadParam(aMsg, aItr, &(aResult->mPriority)) || > + !ReadParam(aMsg, aItr, &(aResult->mNominated)) || > + !ReadParam(aMsg, aItr, &(aResult->mWritable)) || > + !ReadParam(aMsg, aItr, &(aResult->mReadable)) || > + !ReadParam(aMsg, aItr, &(aResult->mRemoteCandidateId)) || > + !ReadParam(aMsg, aItr, &(aResult->mSelected)) || > + !ReadParam(aMsg, aItr, &(aResult->mState)) || > + !ReadParam(aMsg, aItr, &(aResult->mBytesSent)) || > + !ReadParam(aMsg, aItr, &(aResult->mBytesReceived)) || > + !ReadParam(aMsg, aItr, &(aResult->mLastPacketSentTimestamp)) || > + !ReadParam(aMsg, aItr, &(aResult->mLastPacketReceivedTimestamp)) || > + !ParamTraits<mozilla::dom::RTCStats>::Read(aMsg, aItr, aResult)) { > return false; > } > > return true; Might as well flip this one as well, since you touched it? x4 ::: media/webrtc/signaling/src/peerconnection/WebrtcGlobalInformation.cpp:1017 (Diff revisions 2 - 3) > // the ICE component id for each candidate pair and candidate) > > std::map<std::string, StreamResult> streamResults; > > // Build list of streams, and whether or not they failed. > - for (size_t i = 0; > + for (const auto& pair: query->report->mIceCandidatePairStats.Value()) { space before : x4

Comment on attachment 8895467 [details] Bug 1380555 - P2 - Refactor isRemote from telemetry; https://reviewboard.mozilla.org/r/166662/#review171838 w/ nits. ::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp:255 (Diff revision 1) > - bool isAudio = (s.mId.Value().Find("audio") != -1); > + if (s.mPacketsLost.WasPassed() && s.mPacketsReceived.WasPassed() && > + (s.mPacketsLost.Value() + s.mPacketsReceived.Value()) != 0) { > + id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_PACKETLOSS_RATE : > + WEBRTC_VIDEO_QUALITY_OUTBOUND_PACKETLOSS_RATE; > + Accumulate(id, > + (s.mPacketsLost.Value() * 1000) / > + (s.mPacketsLost.Value() + s.mPacketsReceived.Value())); > + > + } Is there any way to abstract this since it is mostly duplicated for the Inbound stats below? ::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp:269 (Diff revision 1) > + if (lastRemoteInboundStats && s.mBytesReceived.WasPassed()) { > + auto& laststats = *lastRemoteInboundStats; > + auto i = FindId(laststats, s.mId.Value()); > + if (i != laststats.NoIndex) { > + auto& lasts = laststats[i]; > + if (lasts.mBytesReceived.WasPassed()) { > + auto delta_ms = int32_t(s.mTimestamp.Value() - > + lasts.mTimestamp.Value()); > + // In theory we're called every second, so delta *should* be in that range. > + // Small deltas could cause errors due to division > + if (delta_ms > 500 && delta_ms < 60000) { > + id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_BANDWIDTH_KBITS : > + WEBRTC_VIDEO_QUALITY_OUTBOUND_BANDWIDTH_KBITS; > + Accumulate(id, ((s.mBytesReceived.Value() - > + lasts.mBytesReceived.Value()) * 8) / delta_ms); > + } > + // We could accumulate values until enough time has passed > + // and then Accumulate() but this isn't that important. > + } > + } > + } Also, mostly duplicated below in the inbound stats. ::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp:319 (Diff revision 1) > auto delta_ms = int32_t(s.mTimestamp.Value() - > lasts.mTimestamp.Value()); > // In theory we're called every second, so delta *should* be in that range. > // Small deltas could cause errors due to division > if (delta_ms > 500 && delta_ms < 60000) { > HistogramID id; I think you missed removing this occurance of HistogramId.

Comment on attachment 8895497 [details] Bug 1380555 - refactor out isRemote; https://reviewboard.mozilla.org/r/166698/#review171922

Comment on attachment 8897182 [details] Bug 1380555 - P2 - Refactor isRemote from telemetry; https://reviewboard.mozilla.org/r/168468/#review174086 Looks good to me after adding the comment below! ::: media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp:279 (Diff revision 1) > - auto& s = array[i]; > - bool isAudio = (s.mId.Value().Find("audio") != -1); > + bool isAudio = mediaIsAudio(s); > + if (s.mPacketsLost.WasPassed() && s.mPacketsReceived.WasPassed() && > + (s.mPacketsLost.Value() + s.mPacketsReceived.Value()) != 0) { > + id = isAudio ? WEBRTC_AUDIO_QUALITY_OUTBOUND_PACKETLOSS_RATE : > + WEBRTC_VIDEO_QUALITY_OUTBOUND_PACKETLOSS_RATE; > + Accumulate(id, I would add the same comment from the packets lost section for recording inbound stats here (or abstract this code like you did for recordAudioBandwidth).

Spec Document: http://rawgit.com/w3c/webrtc-stats/master/webrtc-stats.html Webidl review notes: discaredPackets is cloned into packetsDiscarded for now, a warning will be coming, and eventually discaredPackets be removed When called with the legacy getStats call back API: * the old type names are still used * isRemote is inserted * if localId is present it is duplicated into remoteId

Comment on attachment 8898951 [details] Bug 1380555 - refactor out isRemote; https://reviewboard.mozilla.org/r/170304/#review175522

Mass change P1->P2 to align with new Mozilla triage process

Target for this is 65 (bug 1393306 added warnings that say so).

Removes RTP stat field isRemote and adds the new types remote-inbound-rtp, and remote-outbound-rtp

Spec Review Notes: This changeset removes the isRemote field of the WebRTC RTP Stats [0], and instead uses the new method of indicating remoteness with the type[1][2] field. This also necessitates the addition of the localId [3][4] field. Bug 1515716 has been filed as a follow up to more fully refactor the types and move remoteId and localId into their respective new dictionaries. [0] https://w3c.github.io/webrtc-stats/#streamstats-dict* [1] https://www.w3.org/TR/webrtc/#dom-rtcstats-type [2] https://w3c.github.io/webrtc-stats/#rtcstatstype-str* [3] https://w3c.github.io/webrtc-stats/#remoteoutboundrtpstats-dict* [4] https://w3c.github.io/webrtc-stats/#remoteinboundrtpstats-dict*

Pushed by na-g@nostrum.com: https://hg.mozilla.org/integration/autoland/rev/ccb218cd2d87 remove deprecated WebRTC RTP stat isRemote in favor of new stat types r=jib,smaug

Backed out for mozlint failure on PeerConnection.js Backout link: https://hg.mozilla.org/integration/autoland/rev/3d48e9c1530eb7446e01c1b68121e733fa848173 Push link: https://hg.mozilla.org/integration/autoland/rev/ccb218cd2d8781509264669b290fbdee586ff513 Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=218267860&repo=autoland&lineNumber=272

Pushed by na-g@nostrum.com: https://hg.mozilla.org/integration/autoland/rev/ccca038b9272 remove deprecated WebRTC RTP stat isRemote in favor of new stat types r=jib,smaug

Fixed the lint issue, re-opened the phabricator revision, and re-auto-landing.

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/legacy-peerconnection-getstats-support-has-been-removed/