-
Notifications
You must be signed in to change notification settings - Fork 6
Initial effort to implement SSE #3
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
Conversation
Note that this PR is not based on |
} | ||
// Advertise support for SSE | ||
let link = '<' + config.rest + '/sse/' + uuid + '>; ' + | ||
'rel="urn:ietf:params:whep:ext:core:server-sent-events"; ' + |
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.
Heads-up: I noticed there's some inconsistency about these rel values in the spec.
Opened wish-wg/webrtc-http-egress-protocol#13 to discuss that and come up with a canonical answer, but this line and its parallel in web/watch.js may need to change.
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.
The inconsistency has been resolved. No action is needed in this PR.
Sergio's updated the spec and reference lib to consistently use urn:ietf:params:whep:ext:core:server-sent-events
(the value used in this PR).
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 the heads up! (and sorry for the late feedback, I was busy at the IETF meeting).
Update: this patch is now completely broken, since we refactored the whole project to become a library instead, and so the whole code changed considerably. I'll keep the PR open as a memo, but I'll open a new one to add the same features to the new codebase instead. |
This is a rough implementation of Server-Side Events (SSE) as used in WHEP in the latest draft. It currently only covers
viewercount
,active
andinactive
: no support forlayers
or active layer selection yet, as the specification is currently lacking in that regard IMO.The implementation is a bit flaky, right now, so it's not meant to be a final realization of the feature, but then this was just added to the draft and may change considerably in the future anyway. That said, it seems to be working as expected in its current status, which means it's fine for prototyping purposes.