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

SimplePeer support #510

Closed
ghost opened this issue May 18, 2020 · 21 comments
Closed

SimplePeer support #510

ghost opened this issue May 18, 2020 · 21 comments

Comments

@ghost
Copy link

ghost commented May 18, 2020

Expected behavior

I expect to receive and be able to play a remote stream, when the other party sends me one.

Observed behavior

Rather than receiving and playing the stream, this error is logged (as an error):
Callbacks are not supported by "RTCPeerConnection.prototype.getStats" anymore, use Promise instead.

I have zero callback functions in my code, which leads me to believe this is an issue within the iosrtc JS.
As far as I can tell it happens right after PluginRTCDataChannel#setListener() is called. (At least that's how the log goes in xcode)

Steps to reproduce the problem

It always happens for me when I try to finish a connection where both parties send video, regardless of whether the IOS app is the initiator or not. (I'm using SimplePeer 9.7.2 btw.)

Platform information

  • Cordova version: 7.1.0
  • Plugin version: Tried both 6.0.11 and 7.0-RC2
  • iOS version: 13.4.1
  • Xcode version: 11.4.1
@hthetiot
Copy link
Contributor

Hello @SteffenFreeway Can you provide a sample with SimplePeer 9.7.2, it's most likely they call getStats with a callback fonction inside the library.

You can also confirm they use callback by using, to see if that solve the issue.

cordova.plugin.ios.registerGlobals(false)

@hthetiot
Copy link
Contributor

We may want to add getStats to restoreCallbacksSupport that may help.

function restoreCallbacksSupport() {
	debug('restoreCallbacksSupport()');
	getUserMedia = callbackifyMethod(getUserMedia);
	enumerateDevices = callbackifyMethod(enumerateDevices);
	callbackifyPrototype(RTCPeerConnection.prototype, 'createAnswer');
	callbackifyPrototype(RTCPeerConnection.prototype, 'createOffer');
	callbackifyPrototype(RTCPeerConnection.prototype, 'setRemoteDescription');
	callbackifyPrototype(RTCPeerConnection.prototype, 'setLocalDescription');
	callbackifyPrototype(RTCPeerConnection.prototype, 'addIceCandidate');
	callbackifyPrototype(RTCPeerConnection.prototype, 'getStats');
}

@hthetiot
Copy link
Contributor

hthetiot commented May 19, 2020

@hthetiot
Copy link
Contributor

@SteffenFreeway Try this branch with a fix for you:

@ghost
Copy link
Author

ghost commented May 19, 2020

I'm unfortunately not at the office atm, but will try it out as soon as I'm back. Thanks so much for looking into this so quickly :-)

@hthetiot
Copy link
Contributor

Will be pushed to next release unless confirmed in the next 12 hours @SteffenFreeway

@hthetiot hthetiot modified the milestones: 6.0.x, 6.0.12 May 20, 2020
@ghost
Copy link
Author

ghost commented May 20, 2020

Will be pushed to next release unless confirmed in the next 12 hours @SteffenFreeway

I'll be testing it within 2 hours :-)

@ghost
Copy link
Author

ghost commented May 20, 2020

I don't get the getStats error anymore, however I still don't remote video.

Here's the entire log output: https://pastebin.com/8m6uXBTj

And the current error seems to be: Unhandled Promise rejection: res.forEach is not a function

I can't really see whether it's in simplepeer or iosrtc, so I apologize if it's not in your code.

@hthetiot
Copy link
Contributor

hthetiot commented May 20, 2020

Hello @SteffenFreeway
I think the issue is in your getStats Promise callback do you see something like this in your code reports.push(flattenValues(report));

res.forEach(report => {
            reports.push(flattenValues(report))
          })', 'res.forEach' is undefined) ; Zone: <root> ; Task: Promise.then ; Value: TypeError: res.forEach is not a function. (In 'res.forEach(report => {
            reports.push(flattenValues(report))
          })', 'res.forEach' is undefined) http://localhost:8080/private/var/containers/Bundle/Application/F3CC913D-71B5-4426-

Can you try this instead:

res.result.forEach.

I may add iterator interface for RTCStatsResponse later.

@ghost
Copy link
Author

ghost commented May 22, 2020

Thanks once again @hthetiot for looking into this.
Unfortunately the flattenValues code isn't from my source, so it's probably something within SimplePeer :-(

@hthetiot
Copy link
Contributor

Thank you @SteffenFreeway for confirmation, I will update getStats to make it works.
Keep you posted.

@hthetiot
Copy link
Contributor

hthetiot commented May 22, 2020

@SteffenFreeway This should satify any possible use even by SimplePeer now

pc2.getStats().then(function (res) { console.log(res.forEach(function (a) { console.log(a) })) })

or

pc2.getStats(function (res) { console.log(res.forEach(function (a) { console.log(a) })) })

Screen Shot 2020-05-22 at 11 14 11 PM

@hthetiot
Copy link
Contributor

Thank you for your patience @SteffenFreeway, test and let me know if all good I will merge and will be released on 6.0.12

@ghost
Copy link
Author

ghost commented May 24, 2020

I'll give it a test spin tomorrow when I'm back at the office :-)

@hthetiot
Copy link
Contributor

@SteffenFreeway did you test successfully?

@ghost
Copy link
Author

ghost commented May 26, 2020

@hthetiot I'm terribly sorry, but I never got to the office yesterday. I will be going there today for sure though, in about 7 hours. And I'll be testing the RC first thing when I get there.

@ghost
Copy link
Author

ghost commented May 26, 2020

I've tested it now, and unfortunately I still receive no remote video.
The log still contains a few errors / fails, albeit the ones I've previously mentioned seem to be gone :-)
And FWIW the changes don't seem to break anything either, so I think you could probably release 6.0.12 just fine.

As for my issue I've uploaded the latest error log here: https://pastebin.com/0D3qyUJ5

The errors that I've noticed are:
Could not signal service com.apple.WebKit.WebContent: 113: Could not find specified service

And

Failed to make complete framebuffer

Does this mean anything to you?

@hthetiot
Copy link
Contributor

@SteffenFreeway Thank you for your feedback.
I dont see the error "Unhandled Promise rejection: res.forEach is not a function" anymore and the logs you provided look all goes as expected.

Can you provide the way you set video.srcObject ?
Do you see your own camera but not the remote ?

@ghost
Copy link
Author

ghost commented May 27, 2020

@hthetiot
I see my own camera, but not the remote.

Here's the part of the code that handles the remote video (it's TypeScript btw. and the class contains a lot more code than just this, I just stripped it out for brevity):

export class VideoChatModalPage {
	public remoteVideoStream: MediaStream;

	private initializePeer(initiator: boolean, localStream: MediaStream) {
		this.peer = new SimplePeer({
			initiator: initiator,
			trickle: false,
			stream: localStream
		});

		this.peer.on('stream', (stream) => {
			this.log('stream', stream);
			this.remoteVideoStream = stream;
		});
	}
}

I don't see the log entry from the event handler though, so it could be simplepeer never receives a stream. I'll be looking into it some more tomorrow when I'm at the office again, and see if I can find some details about when/where it fails.

@hthetiot hthetiot reopened this May 29, 2020
@hthetiot hthetiot modified the milestones: 6.0.12, 6.0.13 May 29, 2020
@hthetiot hthetiot changed the title Error: Callbacks are not supported by "RTCPeerConnection.prototype.getStats" anymore, use Promise instead. SimplePeer support May 29, 2020
@ghost
Copy link
Author

ghost commented Jun 2, 2020

I finally have had some luck with getting remote video - currently I can only get video on my iOS app if it's the initiator of the connection. However I don't think this is a bug in iosrtc, but rather iOS just being picky, so I'll attempt to work around that myself.

Thanks a bunch for the assistance with Simplepeer support - I'll close the issue as it appears to be working correctly now :-)

@ghost ghost closed this as completed Jun 2, 2020
@hthetiot
Copy link
Contributor

hthetiot commented Jun 2, 2020

Thank you @SteffenFreeway

@hthetiot hthetiot modified the milestones: 6.0.13, 6.0.12 Jun 2, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant