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

RFC 169: WebRTC Echo server #169

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

fippo
Copy link

@fippo fippo commented Oct 18, 2023

@fippo
Copy link
Author

fippo commented Oct 18, 2023

(obviously the RFC is borrowed from the h3 server as is the implementation ;-))

@fippo fippo changed the title RFC xx: WebRTC Echo server RFC 169: WebRTC Echo server Nov 7, 2023

### Dependencies

As of writing this RFC, the only dependency is `aiortc`.
Copy link
Member

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.
Copy link
Member

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?

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.

Copy link
Member

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).
Copy link
Contributor

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)

Copy link
Author

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.

Copy link
Member

@gsnedders gsnedders Dec 1, 2023

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?

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.

Copy link
Author

@fippo fippo Dec 11, 2023

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.

@fippo
Copy link
Author

fippo commented Feb 25, 2024

What is the next step here?

@jgraham
Copy link
Contributor

jgraham commented Mar 5, 2024

I think the next step is for @gsnedders to give some feedback on what can work from WebKit's point of view.

@alvestrand
Copy link

Ping - @gsnedders was asked to comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants