-
Notifications
You must be signed in to change notification settings - Fork 67
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
RFC 169: WebRTC Echo server #169
base: master
Are you sure you want to change the base?
Conversation
described in https://lists.w3.org/Archives/Public/public-webrtc/2021Feb/0043.html https://www.youtube.com/watch?v=nPkyvxsKed8 (meeting) Draft implementation: web-platform-tests/wpt#36487
(obviously the RFC is borrowed from the h3 server as is the implementation ;-)) |
Co-authored-by: Sam Sneddon <[email protected]>
|
||
### Dependencies | ||
|
||
As of writing this RFC, the only dependency is `aiortc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Risks are similar to [RFC #42](https://github.com/web-platform-tests/rfcs/blob/master/rfcs/quic.md#risks). | ||
|
||
The WebRTC test server itself is standalone and the maintenance cost of the code will be low from the `wptserve`'s perspective. | ||
The WebRTC team at Google and Microsoft will be responsible for maintaining the integration point and the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alvestrand can you confirm Google's WebRTC can commit to maintain this together with Microsoft?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We plan to submit and maintain WPT tests that use this facility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming!
|
||
## Risks | ||
|
||
Risks are similar to [RFC #42](https://github.com/web-platform-tests/rfcs/blob/master/rfcs/quic.md#risks). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this involve new binary dependencies? I assume it does, but it would help to be specific here since those require special handling, especially in vedor repos where we can't depend on pypi. They also add some maintenance risk (as we've seen with aioquic not updating its releases for newer Python versions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python bindings for libsrtp and google-crc32c should be the only new dependencies I think. @jlaine might know more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/aiortc/aiortc/blob/0ecc5379762883fbf6454d4633b948c2ad50dec8/pyproject.toml#L28-L38 shows quite a lot of (direct) Python dependencies:
dependencies = [
"aioice>=0.9.0,<1.0.0",
"av>=9.0.0,<12.0.0",
"cffi>=1.0.0",
"cryptography>=2.2",
'dataclasses; python_version < "3.7"',
"google-crc32c>=1.1",
"pyee>=9.0.0",
"pylibsrtp>=0.5.6",
"pyopenssl>=23.1.0",
]
I think av
is the dependency that worries me most here—especially insofar as it has a pretty major dependency (ffmpeg
—which is a dependency likely to make legal departments nervous given the patent status of many codecs it supports—and is also just a pretty sizeable project which is more likely to cause issue on new platforms).
(There's a few other notable new dependencies: aiortc
itself depends on opus and libvpx, and pylibstrp
depends on libsrtp, but these are probably less problematic)
Are we actually planning on doing any testing that actually requires media codecs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we actually planning on doing any testing that actually requires media codecs?
Yes.
The initial proposals (passing the RTP packets back to the test for analysis) would not need media codecs on the server, but once we have experience with those, running media codecs on the server is a definite option.
In order to do the tests, we have to negotiate use of media codecs; I have no idea whether it's possible to build aiortc in such a way that it can claim to have media codecs but actually not have them.
The argument for doing these tests in wpt is that we would get wpt's infrastructure for tracking and sharing tests with a relatively small investment in engineering on the webrtc support side (by using aiortc). If we start asking to reengineer aiortc in order to integrate it, the value of this approach starts dropping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now forking to remove "av" as a dependency is possible: https://github.com/aiortc/aiortc/compare/main...fippo:aiortc:no-av?expand=1
but that gets to a point where the maintenance cost will add up.
Just running this in Chromium's CI would be ok as the tests will be testing low-level libWebRTC behavior which is going to be largely identical in consuming browsers like Safari.
What is the next step here? |
I think the next step is for @gsnedders to give some feedback on what can work from WebKit's point of view. |
Ping - @gsnedders was asked to comment. |
described in
https://lists.w3.org/Archives/Public/public-webrtc/2021Feb/0043.html
https://www.youtube.com/watch?v=nPkyvxsKed8 (meeting)
Draft implementation:
web-platform-tests/wpt#36487