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

shimAddIceCandidateNullOrEmpty fails on Safari 17.0 #1140

Open
3 tasks done
thewildtree opened this issue Sep 29, 2023 · 3 comments
Open
3 tasks done

shimAddIceCandidateNullOrEmpty fails on Safari 17.0 #1140

thewildtree opened this issue Sep 29, 2023 · 3 comments

Comments

@thewildtree
Copy link

thewildtree commented Sep 29, 2023

  • I have provided steps to reproduce (e.g. a link to a jsfiddle)
  • I have provided browser name, version and adapter.js version
  • This issue only happens when adapter.js is used

Versions affected

Browser name including version (e.g. Chrome 64.0.3282.119)

Safari 17.0 (19616.1.27.111.16)

adapter.js (e.g. 6.1.0)

All versions >=8.0.0

Description

The shimAddIceCandidateNullOrEmpty step fails on Safari. This particular check breaks (len==0):

adapter/release/adapter.js

Lines 1480 to 1482 in 28c9f94

var nativeAddIceCandidate = window.RTCPeerConnection.prototype.addIceCandidate;
if (!nativeAddIceCandidate || nativeAddIceCandidate.length === 0) {
return;

Causing this API to not be replaced correctly. This breaks the empty-candidate scenario, causing an error to be thrown.

Steps to reproduce

  1. Insert https://webrtc.github.io/adapter/adapter-latest.js into your site (via <script>)
  2. Insert debugger breakpoint at line mentioned above
  3. See shim failing to be correctly applied.

We encountered this bug when testing https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/tree/main/net/webrtc/gstwebrtc-api on Safari, where we depend on the empty-candidate scenario to work correctly.

Note: when pausing with the debugger and refreshing right after, it sometimes actually passes that check and the shim works correctly, once in 20 tries I'd say. Might be a Safari bug for all it's worth.

Expected results

The addIceCandidate is correctly modified to have Safari handle the empty candidate scenario fine.

Actual results

addIceCandidate is not modified, causing Safari to throw an error when encountering an empty candidate string.

@thewildtree
Copy link
Author

Also tested on Safari on iOS 17 - same thing happens.

@fippo
Copy link
Member

fippo commented Sep 29, 2023

@youennf that seems like a regression?

@thewildtree
Copy link
Author

To maybe speed things up - this is almost certainly a Safari bug.

As it is right now, window.RTCPeerConnection.prototype.addIceCandidate has length: 0, name: "addIceCandidate".

When I add a 3s delay before calling shimAddIceCandidateNullOrEmpty, about 80% of the time a different function appears with length: 3, name: "withCallback" and the shim is correctly applied. It's inconsistent though, no matter how long the delay is, sometimes the signature is just incorrect anyway.

Any ideas on how to proceed with this?

gstreamer-github pushed a commit to sdroege/gst-plugin-rs that referenced this issue Oct 30, 2023
There's currently a Safari-side bug causing webrtc-adapter to be unable to correctly shim the empty-candidate scenario
which we're using. This patch is very much a workaround and should be removed as soon as Safari and/or webrtc-adapter
fixes this on their side.

https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/issues/439
webrtcHacks/adapter#1140

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1377>
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

No branches or pull requests

2 participants