1341421 - Stats field remoteId is not handled consistently when empty

Status: NEW

(Core :: WebRTC, defect, P3)

Description

[Nico Grunbaum [:ng, @chew:mozilla.org]]

remoteId (soon to be associateStatsId) is missing from inbound rtp audio when it should be present but contain an empty string.

Comment 1

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Example capture comparing inbound RTP audio and outbound RTP audio:
  "inbound_rtp_audio_0": {
    "id": "inbound_rtp_audio_0",
    "timestamp": 1487707763946.38,
    "type": "inboundrtp",
    "isRemote": false,
    "mediaType": "audio",
    "ssrc": "2206160262",
    "bytesReceived": 3829,
    "jitter": 0,
    "packetsLost": 0,
    "packetsReceived": 33
  },
...
  "outbound_rtp_audio_0": {
    "id": "outbound_rtp_audio_0",
    "timestamp": 1487707763946.38,
    "type": "outboundrtp",
    "isRemote": false,
    "mediaType": "audio",
    "remoteId": "",
    "ssrc": "948754621",
    "bytesSent": 4159,
    "packetsSent": 33
  },

Comment 2

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Do you have steps to reproduce? I can't reproduce it with https://jsfiddle.net/jib1/ngo77y5e/

FWIW I've seen this happen for video when my camera has hosed itself in Firefox (no self-view), but I haven't been able to reproduce it.

Comment 3

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Jan-Ivar try: https://jsfiddle.net/vj2rkbns/1/ , and you are right I am seeing it in video.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

1 second is not enough time for RTCP packets to arrive. RTCP stats are not present in that case. Not a bug.

When I increase the wait to 10 seconds, the remoteId and RTCP data are there.

Comment 5

[Nico Grunbaum [:ng, @chew:mozilla.org]]

What is the difference between it not having the field and the field being set to ""?  The way I was reading the spec, the field is required to be present[1]. See the second to last bullet under "Section 2. Conformance".

[1] https://www.w3.org/TR/webrtc-stats/#conformance

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

The snippet below shows a clearly inconsistent behavior between inbound, outbound. Inbound lacks a remoteId field and outbound has an empty remoteId field. This looks like a bug to me.

(In reply to Nico Grunbaum [:ng] from comment #1)
> Example capture comparing inbound RTP audio and outbound RTP audio:
>   "inbound_rtp_audio_0": {
>     "id": "inbound_rtp_audio_0",
>     "timestamp": 1487707763946.38,
>     "type": "inboundrtp",
>     "isRemote": false,
>     "mediaType": "audio",
>     "ssrc": "2206160262",
>     "bytesReceived": 3829,
>     "jitter": 0,
>     "packetsLost": 0,
>     "packetsReceived": 33
>   },
> ...
>   "outbound_rtp_audio_0": {
>     "id": "outbound_rtp_audio_0",
>     "timestamp": 1487707763946.38,
>     "type": "outboundrtp",
>     "isRemote": false,
>     "mediaType": "audio",
>     "remoteId": "",
>     "ssrc": "948754621",
>     "bytesSent": 4159,
>     "packetsSent": 33
>   },

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

What version is this? I see "inboundrtp" without a hyphen, suggesting not Nightly. Please set the version field. Also, you're only showing part of the capture. Are any isRemote == true instances present in the full capture?

If there are no rtcp instances, then there are no broken links, which would be good news. Then the only bug here would be the inconsistency ("" vs missing).

The bullets you reference are only an example. The conformance criteria are actually here http://w3c.github.io/webrtc-pc/#mandatory-to-implement-stats and say:

  "An implementation MUST support generating statistics of the following types when the corresponding objects exist on a PeerConnection, with the attributes that are listed when they are valid for that object".

Note the two "when"s.

Before RTCP data have arrived, it seems fair to say there's no monitored object for RTCP stats data. remoteId members have no way to be be valid in that case, and would need to be absent, is my best reading. Of course, it wouldn't hurt for the spec to be explicit about this, but I think we're conformant.

I think we should investigate the source of the inconsistency, and remove the { remoteId: "" } appendage.

Comment 8

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Jan-Ivar, thanks for the link. I agree that this is a matter of consistency, and am going to drop the priority. This was a build off of a dated revision in bound.

Comment 9

[Bulk Bug Changes for mreavy's org]

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