adapter
adapter copied to clipboard
webrtcHacks
Shim RTCRtpScriptTransform.
Description
Add a chrome shim for RTCRtpScriptTransform.
Purpose
Help web developers write WebRTC encoded-transform code to spec that works across browsers.
Working example: https://jan-ivar.github.io/samples/src/content/insertable-streams/video-analyzer/ (uses temp shim)
Usage
Making the shim work requires adding the following line in workers:
// accept shimming
self.onmessage = ({data}) => data.rtctransform && self.onrtctransform({transformer: data.rtctransform});
I don't think this should be done by adapter. adapter's job was that of a small polyfill during the early life of WebRTC. This phase is over (and adoption numbers for new versions of adapter are less than great already)
I do not consider the unwillingness of two parties in the W3C Working Group to ignore that "web developers" have adopted what Chromium has been doing for years (and I do not recall any contributions to the underlying native C++ code for insertable streams). The spec's lack of adoption by "web developers" is pretty clear from the comments here, the refusal to merge https://github.com/w3c/webrtc-encoded-transform/issues/105 is rather harmful for "web developers". Not that "web developers" have a strong voice in the working group because the WG is not tackling problems relevant to them...
Using "adapter provides a polyfill" as an excuse to not implement "legacy" as done by Safari in the case of the rather widely used offerToReceiveAudio is a precedent that I would not like to see repeated. The suggestion of a polyfilll here seems to go in the same direction.
Web developers should not suffer while vendors reach agreement. The role of adapter.js has always been to assist these developers write the same WebRTC code on all browsers without blocking on vendors, using the latest spec.
Web developers need an adapter here to write the same WebRTC code across browsers, so this being a "phase" that is "over" seems demonstrably not so.
This 34-line polyfill makes the same application work in Chrome, Safari and Firefox. This is the right place.
No-one's forced to use adapter, so blocking tools for web developers on the basis of no-one's using them seems flawed.
I'm happy to debate the marginal merits of main-thread parsing vs the perceived difficulty of writing worker code over in https://github.com/w3c/webrtc-encoded-transform/issues/89#issuecomment-1858693240, but let's not have that discussion bleed in here.
Web developers should not suffer while vendors reach agreement
Web developers do suffer from the lack of documentation caused by https://github.com/w3c/webrtc-encoded-transform/issues/105. MDN decided to only document "encoded transform" and claims (technically correct) that it is not supported in Chromium-based browsers.
The article does not mention that Chromium has supported this way longer than others browsers, even if using a different API. Including support in Workers (which took a bit longer to actually get performant).
The role of adapter.js has always been to assist these developers write the same WebRTC code on all browsers without blocking on vendors, using the latest spec.
In cases where the spec was showing consensus. There are clear signs for lack of consensus in this case. The datachannel binaryType is a recent example where adapter did not get into a decade-long pointless argument.
This 34-line polyfill makes the same application work in Chrome, Safari and Firefox. This is the right place.
No, this makes a tiny sample application work, by chance. Real applications are a bit more complicated (and even if using adapter which is increasingly less common they tend to pull a version and then stick to it).
The polyfill unconditionally calls createEncodedStreams upon setting .transform. This works because the sample sets the nonstandard encodedInsertableStreams flag to true. A developer only reading MDN will wonder why stuff does not work and throws an error. (note: .getConfiguration returns that flag, .setConfiguration does not allow changing it).
This flag can not be set unconditionally as, being built for E2EE, Chromium refuses to send un-transformed data. Which may also surprise developers that do not initially set a transform. While that is being worked on here it has not landed in enough browser versions to help with a polyfill. A developer trying to unset .transform ... will not get the expected result?
Also the flag has performance implications noted here so unconditionally poylfilling it with a null-transform is a bad idea.
All of these suggest this is not the right place.
My understanding is that historically, the idea with adapter.js was to shim APIs that had consensus, allowing developers to write modern code without worrying about implementation status, but they could be rest assures that any code they wrote was modern - they shouldn't have to migrate again in the future. The more feature got implemented, the more adapter.js became a NO-OP. And honestly I thought we had already reached the finish line here... I've heard so little about adapter.js in the past 5 years that I had assumed this library was dead (both in terms of web developer usage and in terms of being ~NO-OP on up-to-date browser versions). Am I mistaken here?
Bridging the gap assumes the shim represents what is about to be implemented. Which assumes agreement. Which I don't think we have?
If this is about removing developer pain points, then instead of shimming A on top of B, could we shim B on top of A? I'm not seriously suggesting this, I'm just trying to highlight the importance of knowing where we're going with this, and what the plans for forming consensus are. Especially if we're implicitly encouraging the use of adapter.js in... 2024!
I applaud wanting to make web developers suffer less, and I admit I don't fully understand the state of things, I just think we need a plan so that people don't migrate from option A to option B and then finally consensus lands on some other option C.
My understanding is that historically, the idea with adapter.js was to shim APIs that had consensus,
RTCRtpScriptTransform appears to have consensus from the WebRTC WG's September 2. 2021 successful CfC to publish FPWD with RTCRtpScriptTransform in it and no createEncodedStreams. This shim addresses that gap.
Bridging the gap assumes the shim represents what is about to be implemented. Which assumes agreement. Which I don't think we have?
This applies a different standard. What do you mean by "about to be implemented"? Safari and Firefox already implement https://www.w3.org/TR/webrtc-encoded-transform.
If you mean "about to be implemented" in Chrome, then I cannot answer what Google intends to implement. But we also haven't applied that standard in the past, e.g. with Plan-B.
Thanks @fippo for the detailed feedback!
The polyfill unconditionally calls createEncodedStreams upon setting .transform. This works because the sample sets the nonstandard encodedInsertableStreams flag to true.
That seems no different from requiring nonstandard {sdpSemantics: "unified-plan"} for standard stuff to work in old Chromium (other browsers silently ignore it because WebIDL). But I see there's more to it below.
A developer only reading MDN will wonder why stuff does not work and throws an error.
This is not the MDN repo. But if you're suggesting we shouldn't shim unless it can fool users who are unaware they are using a shim, then I disagree. I also don't recall that being the bar in the past (e.g. getParameters in Firefox).
I think if we draw a Venn diagram of features in different browsers and a shim covers the shared center of it then it's useful.
(note: .getConfiguration returns that flag, .setConfiguration does not allow changing it).
This flag can not be set unconditionally as, being built for E2EE, Chromium refuses to send un-transformed data. Which may also surprise developers that do not initially set a transform. While that is being worked on here it has not landed in enough browser versions to help with a polyfill.
These seem like inherent limitations in Chrome's implementation regardless of API. Glad to hear crbug 1502781 (comment) is "improving conformance incrementally"! — From looking at it:
Allow createEncodedStreams on PCs without encodedInsertableStreams param
Allow creating Encoded Transforms for any Receivers and Senders, so long as createEncodedStreams() is called by JS synchronously after sender/receiver creation. This is achieved by setting up a WebRTC transform on all transceivers, but "short circuiting" it if JS hasn't set up its own transform within an event loop spin of creation. This will make the transform just pass frames to be transformed directly back without invoking the cost of a thread hop or any JS work.
The existing behaviour (dropping all frames until a JS transform is installed) is preserved for PCs created with {encodedInsertableStreams: true}.
This implements the algorithm defined in https://www.w3.org/TR/2023/WD-webrtc-encoded-transform-20231012/#stream-creation, but for the Chromium createEncodedStreams() method, improving conformance incrementally.
So it sounds like encodedInsertableStreams will soon no longer be required, which sounds great! It should simplify things a great deal.
I'm happy to wait for that to merge to not have to address it in the shim. At the same time it seems orthogonal to someone aware of these limitations.
A developer trying to unset .transform ... will not get the expected result?
Again, if the Chrome implementation cannot support that then that is not the fault of the shim. It is also not a reason not to shim IMHO.
That said, I'm happy to improve on the transform = null case in the shim. It should probably close previous streams.
That seems no different from requiring nonstandard {sdpSemantics: "unified-plan"} for standard stuff to work in old Chromium
It is no different in the sense that unified-plan/plan-b was not a thing that was meaningfully poylfillable either (or we would not have a pointless and painful migration for years)
This is not the MDN repo.
I'll ask folks on the MDN advisory board then.
But if you're suggesting we shouldn't shim unless it can fool users who are unaware they are using a shim, then I disagree. I also don't recall that being the bar in the past (e.g. getParameters in Firefox).
getParameters in Firefox was notably written by someone familiar with Firefox (i.e. you).
The insertable streams API is used for things like end-to-end encryption which are highly sensitive and require full transparency. "Fooling" with a polyfill is a bad idea in such a case. And it might be harmful for sites that gate such a feature on the existence of the spec API.
Again, if the Chrome implementation cannot support that then that is not the fault of the shim. It is also not a reason not to shim IMHO.
So you are proposing to do a half-broken shim. Which will cause what beyond developers getting frustrated by adapter.js and Chromium? While the native implementation in Chromium is doing just what it is supposed to be and already in production (which includes Facetime).
adapter has been helping where it made sense, reducing the friction. This change will not do that.
I'd suggest doing your own polyfill for "web developers" to include. It can be done ontop of adapter even. I do not think you will get much adoption though.
RTCRtpScriptTransform appears to have consensus from the WebRTC WG's September 2
I see, I was reacting quite strongly to this being a motivation for the PR:
Web developers should not suffer while vendors reach agreement.
But if there is agreement here, LGTM from me. Still sad to see adapter.js being revived - @alvestrand Can you sign off on this PR and does Chromium have a plan to implement this as per shim?