Open Bug 1463430 Opened 2 years ago Updated 2 years ago

RTCStats instances returned by RTCRtpReceiver- and RTCRtpSender-level getStats should have persistent unique IDs

Categories

(Core :: WebRTC: Signaling, defect, P3)

60 Branch
defect

Tracking

()

People

(Reporter: mroberts, Assigned: ng)

References

()

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 Safari/537.36

Steps to reproduce:

See below for a little test script that

1. Constructs two RTCPeerConnections, pc1 and pc2
1. Acquires two local audio tracks
2. Adds each local audio track to pc1
3. Negotiates between pc1 and pc2
4. Waits 10 s
5. Calls getStats on
5.1. pc2
5.2. each of pc2's RTCRtpReceivers
5.3. pc2 using the MediaStreamTrack selector
6. Filters out the "inbound-rtp" type RTCStats.

    (async () => {
      const stream1 = await navigator.mediaDevices.getUserMedia({ audio: true });
      const [track1] = stream1.getTracks();
      const stream2 = await navigator.mediaDevices.getUserMedia({ audio: true });
      const [track2] = stream2.getTracks();
      const pc1 = new RTCPeerConnection();
      const pc2 = new RTCPeerConnection();
      [[pc1, pc2], [pc2, pc1]].forEach(([pc1, pc2]) => {
        pc1.onicecandidate = ({ candidate }) => {
          if (candidate) {
            pc2.addIceCandidate(candidate);
          }
        };
      });
      pc1.addTrack(track1, stream1);
      pc1.addTrack(track2, stream2);
      const offer = await pc1.createOffer();
      await pc1.setLocalDescription(offer);
      await pc2.setRemoteDescription(offer);
      const answer = await pc2.createAnswer();
      await pc2.setLocalDescription(answer);
      await pc1.setRemoteDescription(answer);
      await new Promise(resolve => setTimeout(resolve, 10000));
      const pcReport = await pc2.getStats();
      const receiverReports = await Promise.all(pc2.getReceivers().map(receiver => receiver.getStats()));
      const trackReports = await Promise.all([
        pc2.getStats(pc2.getReceivers()[0].track),
        pc2.getStats(pc2.getReceivers()[1].track)
      ]);
      const pcInboundRtpStats = [];
      for (const pcStats of pcReport.values()) {
        if (pcStats.type === 'inbound-rtp' && !pcStats.isRemote) {
          pcInboundRtpStats.push(pcStats);
        }
      }
      const receiverInboundRtpStats = [];
      receiverReports.forEach(receiverReport => {
        for (const receiverStats of receiverReport.values()) {
          if (receiverStats.type === 'inbound-rtp' && !receiverStats.isRemote) {
            receiverInboundRtpStats.push(receiverStats);
          }
        }
      });
      const trackInboundRtpStats = [];
      trackReports.forEach(trackReport => {
        for (const trackStats of trackReport.values()) {
          if (trackStats.type === 'inbound-rtp' && !trackStats.isRemote) {
            trackInboundRtpStats.push(trackStats);
          }
        }
      });
      console.log(pcInboundRtpStats);
      console.log(receiverInboundRtpStats);
      console.log(trackInboundRtpStats);
      track1.stop();
      track2.stop();
      pc1.close();
      pc2.close();
    })().then(() => console.log('Success!'), console.error)


Actual results:

The two "inbound-rtp" type RTCStats that come from the unfiltered RTCPeerConnection-level stats have unique IDs "inbound_rtp_audio_0" and "inbound_rtp_audio_1"; however, the two "inbound-rtp" type RTCStats that come from the RTCRtpReceivers—either via the RTCRtpReceiver-level getStats or the RTCPeerConnection-level getStats using the MediaStreamTrack selector—reuse the same ID, "inbound_rtp_audio_0".


Expected results:

I don't think this agrees with https://w3c.github.io/webrtc-pc/#rtcstats-dictionary, which states

> A unique id that is associated with the object that was inspected to produce this RTCStats object. Two RTCStats objects, extracted from two different RTCStatsReport objects, MUST have the same id if they were produced by inspecting the same underlying object.

Since the underlying objects that were inspected were the same, I expect to always get the two distinct IDs "inbound_rtp_audio_0" and "inbound_rtp_audio_1" in my test, regardless of where I call getStats.

I see similar behavior with RTCRtpSender. I have not checked RTCRtpTransceiver.
Component: Untriaged → WebRTC
Product: Firefox → Core
Component: WebRTC → WebRTC: Signaling
Sounds legit. Making it a P3 for now. Nico, could you triage this further, and perhaps take ownership? Thanks!

I also put the sample code in a jsfiddle and exposed the logging: https://jsfiddle.net/pehrsons/twe3sL27/
Rank: 25
Flags: needinfo?(na-g)
Priority: -- → P3
Assignee: nobody → na-g
Flags: needinfo?(na-g)
Summary: RTCStats instances returned by RTCRtpReceiver- and RTCRtpSender-level getStats should have unique IDs → RTCStats instances returned by RTCRtpReceiver- and RTCRtpSender-level getStats should have persistent unique IDs
Attachment #8986892 - Flags: review?(mfroman)
Comment on attachment 8986892 [details]
Bug 1463430 - persist webrtc stats ids

https://reviewboard.mozilla.org/r/252128/#review258852

Just a couple questions first.

::: dom/media/tests/mochitest/test_peerConnection_stats_persist_ids.html:19
(Diff revision 1)
> +const NUMBER_OF_TRACKS_PER_TYPE = 5;
> +
> +const checkSenderReceiverStatsIdPersistence = async test => {
> +  const pcStats = await test.pcLocal.getStats();
> +  pcStats.forEach((v, k) => {
> +    dump(`@@NG received key ${k}\n`);

Is this actually checking anything?
Also, is 'NG' debugging leftover?

::: media/webrtc/signaling/src/peerconnection/TransceiverImpl.h:51
(Diff revision 1)
>   * Audio/VideoConduit for feeding RTP/RTCP into webrtc.org for decoding, and
>   * feeding audio/video frames into webrtc.org for encoding into RTP/RTCP.
>  */
>  class TransceiverImpl : public nsISupports {
>  public:
> +  typedef uint64_t StatisticsId;

Is this used anywhere?
Attachment #8986892 - Flags: review?(mfroman) → review-
You need to log in before you can comment on or make changes to this bug.