Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RTCRtpStreamStats dictionary #2802

Merged
merged 2 commits into from Sep 14, 2018

Conversation

a2sheppy
Copy link
Contributor

Only Firefox info is in there; everything else is null except IE,
which is always false.

@Elchi3 Elchi3 added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Sep 12, 2018
@Elchi3
Copy link
Member

Elchi3 commented Sep 13, 2018

Thanks for your PR!

The structure looks alright to me and IE being false is reasonable, but without links to bugs or anything I can't review the Firefox information. The branch name includes https://bugzilla.mozilla.org/show_bug.cgi?id=1480498, which is about some renaming, but that renaming is not at all reflected with "alternative_name" in this PR. Please elaborate and provide more information for the reviewer.

"mdn_url": "https://developer.mozilla.org/docs/Web/API/RTCRtpStreamStats",
"support": {
"chrome": {
"version_added": null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be false for all Chrome, Android webview, and Opera.

@a2sheppy
Copy link
Contributor Author

@Elchi3 - So... the data I have here comes from actually going through the code on https://hg.mozilla.org/mozilla-unified/tags, manually doing a binary search through the tags for each release version of Firefox for each item until I found where the property was added. So, you know, good fun. :)

Adds alternative name clause for Firefox. Sets version to
false for all Chrome and Opera versions.
@@ -0,0 +1,730 @@
{
"api": {
"RTCRtpStreamStats": {
Copy link
Contributor Author

@a2sheppy a2sheppy Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RTCRTPStreamStats was added in bug 902003 for Firefox 27: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

},
"firefox": [
{
"version_added": "63"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"deprecated": false
}
},
"codecId": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 for Firefox 27, as part of the original implementation of the interface: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

}
}
},
"mediaTrackId": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 for Firefox 27, as part of the original implementation of the interface: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

}
}
},
"remoteId": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 for Firefox 27, as part of the original implementation of the interface: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

}
}
},
"ssrc": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 for Firefox 27, as part of the original implementation of the interface: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

}
}
},
"transportId": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 for Firefox 27, as part of the original implementation of the interface: https://hg.mozilla.org/mozilla-central/rev/5372aea57cdb

}
}
},
"firCount": {
Copy link
Contributor Author

@a2sheppy a2sheppy Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a more recent change to this line but it appears to be whitespace only.

}
}
},
"isRemote": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in bug 902003 after the initial addition of the interface: https://bugzilla.mozilla.org/attachment.cgi?id=821837

}
}
},
"kind": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
},
"mediaType": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
},
"nackCount": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
},
"pliCount": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"version_added": null
},
"firefox": {
"version_added": false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not present per current WebIDL.

}
}
},
"sliCount": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not present in latest WebIDL.

@a2sheppy
Copy link
Contributor Author

@Elchi3 & @jpmedley - I have finished adding comments to the PR throughout the code to provide bug and changeset links for every Firefox version number specified. I did not duplicate info for both desktop and mobile as they are the same here.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me. 👍

@Elchi3 Elchi3 merged commit 01a67de into mdn:master Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants