RTCMediaStreamTrackStats is four dictionaries in one

[jan-ivar]

The description of nearly every member in RTCMediaStreamTrackStats starts with "Only valid for..."!

Like #191 we can describe this more effectively in WebIDL than with prose:

remoteSource	video	audio
false	RTCLocalVideoStreamTrackStats	RTCLocalAudioStreamTrackStats
true	RTCRemoteVideoStreamTrackStats	RTCRemoteAudioStreamTrackStats

Base:

dictionary RTCMediaStreamTrackStats : RTCStats {
    DOMString           trackIdentifier;
    boolean             ended;
    boolean             detached;
    DOMString           kind;
    RTCPriorityType     priority;
};

Video:

dictionary RTCVideoStreamTrackStats : RTCMediaStreamTrackStats {
    unsigned long       frameWidth;
    unsigned long       frameHeight;
    double              framesPerSecond;
};

dictionary RTCLocalVideoStreamTrackStats : RTCVideoStreamTrackStats {
    unsigned long       framesSent;
    unsigned long       framesCaptured;
};

dictionary RTCRemoteVideoStreamTrackStats : RTCVideoStreamTrackStats {
    DOMHighResTimeStamp estimatedPlayoutTimestamp;
    unsigned long       framesReceived;
    unsigned long       framesDecoded;
    unsigned long       framesDropped;
    unsigned long       framesCorrupted;
    unsigned long       partialFramesLost;
    unsigned long       fullFramesLost;
};

Audio:

dictionary RTCAudioStreamTrackStats : RTCMediaStreamTrackStats {
    double              audioLevel;
    double              totalAudioEnergy;
    boolean             voiceActivityFlag;
    double              totalSamplesDuration;
};

dictionary RTCLocalAudioStreamTrackStats : RTCAudioStreamTrackStats {
    double              echoReturnLoss;
    double              echoReturnLossEnhancement;
    unsigned long long  totalSamplesSent;
};

dictionary RTCRemoteAudioStreamTrackStats : RTCAudioStreamTrackStats {
    DOMHighResTimeStamp estimatedPlayoutTimestamp;
    unsigned long long  totalSamplesReceived;
    unsigned long long  concealedSamples;
    unsigned long long  concealmentEvents;
    double              jitterBufferDelay;
};

If I got some wrong, I blame the prose for being vague.

[henbos]

I think this is nicer and the way it should have been designed from the start. I'm a bit hesitant though. With #191 we could add new dictionary "types" for "isRemote = true", a case which had not been implemented by Chrome/Firefox, and keep the old dictionaries.

This change on the other hand would require 4 new dictionaries "types", deprecating the existing "track", which is not backwards compatible with what is already shipped in Chrome since several versions ago.

If we do this change it would be good to do it in coordination with other disruptive changes, like if we do sender/receiver stats instead of track stats (#231).

[jan-ivar]

@henbos I'm not proposing replacing kind with a unique type. Dictionaries don't have JS-observable types, so refactoring into 4 dictionaries alone is purely editorial, for a more readable spec.

If we don't go for #231 then I'm not proposing removing remoteSource either.

If we do go for #231 then remoteSource disappears naturally (remoteSource == "receiver").

I'm hoping we do the latter, since I find it intuitive. That's why I left remoteSource out in the examples.

[henbos]

Oh okay I assumed the new dictionaries would imply unqiue RTCStats.type:s but if not this is purely editorial. You still have to do the "ifs" in JavaScript but the spec would indeed be more readable.

[jan-ivar]

Right, the stats spec doesn't typically spell out those steps anywhere, but if it did there'd still be if's (just fewer of them at the top instead of per line-item probably).

[alvestrand]

I do worry about the multiplication of types. In OO, there's a design principle like "don't do with inheritance what you can do with containment".
The code smell associated with that is that when you add new elements, you have to add them to multiple subclasses. So far, there's no sign of that, so this seems to make refactoring sense.
And having it only in the spec and not observable is a Good Thing; if we got it wrong, we can refactor without impacting implementations.

[alvestrand]

TPAC feedback: Ready to move forward on this.

[henbos]

jan-ivar since you're doing #231 do you want to take this on too?

[henbos]

@jan-ivar

[henbos]

Fixed by #273