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

MediaStream.addTrack does not accept CanvasCaptureMediaStreamTrack #506

Open
abardik opened this issue May 12, 2020 · 25 comments
Open

MediaStream.addTrack does not accept CanvasCaptureMediaStreamTrack #506

abardik opened this issue May 12, 2020 · 25 comments

Comments

@abardik
Copy link

abardik commented May 12, 2020

Expected behavior

MediaStream.addTrack should allow to use as a first argument an instance of CanvasCaptureMediaStreamTrack captured from <canvas> by captureStream method.

Observerd behavior

MediaStream.addTrack throws an exception "argument must be an instance of MediaStreamTrack" if the first argument is an instance of CanvasCaptureMediaStreamTrack.

In Chrome/Firefox/Safari an instance of CanvasCaptureMediaStreamTrack inherits from MediaStreamTrack, so it can be used in MediaStream.addTrack method. In cordova app using iosrtc plugin CanvasCaptureMediaStreamTrack is not an instance of MediaStreamTrack, so we have an aforementioned exception in MediaStream.addTrack:

MediaStream.prototype.addTrack = function (track) {
debug('addTrack() [track:%o]', track);
if (!(track instanceof MediaStreamTrack)) {
throw new Error('argument must be an instance of MediaStreamTrack');
}

Steps to reproduce the problem

// capture local camera and microphone
navigator.mediaDevices.getUserMedia({ audio: true, video: true }).then(function(stream) {
  // get canvas stream
  var canvas = document.createElement('canvas');
  var canvasStream = canvas.captureStream();
  var track = canvasStream.getVideoTracks()[0];

  // add canvas video track to the stream
  try {
    stream.addTrack(track);
  }
  catch(err) {
    console.error(err); // output: Error: argument must be an instance of MediaStreamTrack
    console.log(track instanceof MediaStreamTrack); // output: false
  }
});

Platform information

  • Cordova version: cli-9.0.0 (iOS 5.0.1)
  • Plugin version: 6.0.10
  • iOS version: 13.3.1, 13.4.1
  • Xcode version: 10.2
@hthetiot
Copy link
Contributor

@abardik

In Chrome/Firefox/Safari an instance of CanvasCaptureMediaStreamTrack inherits from MediaStreamTrack, so it can be used in MediaStream.addTrack method. In cordova app using iosrtc plugin CanvasCaptureMediaStreamTrack is not an instance of MediaStreamTrack, so we have an aforementioned exception in MediaStream.addTrack:

First, This plugin is not suposed to be used ourside of WKWebview and will not support any other enviroment.

Second, You have captureStream on Canvas working on iOS WKWebview ?
That would surpise me.

@hthetiot
Copy link
Contributor

In any case, even if WKWebview was allowing Canvas.captureStream that I'm pretty sure it does not, It will require enormous work to enable converting a "Native" MediaStream to an iosRTC WebRTC MediaStream. But again I already think this is irrelevant since CaptureStream is not even working in Safari therefor it will not work in WKWebview.

This is a free software, I will not implement such features for your blue eyes ;)

@abardik
Copy link
Author

abardik commented May 13, 2020

@hthetiot

First, This plugin is not suposed to be used ourside of WKWebview and will not support any other enviroment.

I mentioned other browsers only to show that CanvasCaptureMediaStreamTrack is an instance of MediaStreamTrack in those browsers. Of course, I'm not using iosrtc outside WKWebView, because it's impossible and doesn't make sence.

Second, You have captureStream on Canvas working on iOS WKWebview ?
That would surpise me.

It works in Safari and WKWebView at least on iOS 13.4.1 and 13.3.1 (iPhone XS), I don't know about earlier versions. I can send CanvasCaptureMediaStreamTrack to MediaStream.addTrack in Safari, but not in WKWebView.

@abardik
Copy link
Author

abardik commented May 13, 2020

@hthetiot

This is a free software, I will not implement such features for your blue eyes ;)

I'm not asking you to implement any features for me. I've already managed this without intermediate local stream. I'm just reporting an incompatibility issue in MediaStream.addTrack method.

You did a lot of work to make this plugin compatible with W3C WebRTC standard. I thought reporting this issue would help in this. Also, CanvasCaptureMediaStreamTrack is a good way to implement collaboration features like whiteboard, bringing new user experience in cordova apps, which is also good.

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

Thank you @abardik

I can address the fact that it support an instance of MediaStreamTrack for pure MediaStream the problem is that it will not work when you add that stream to the iOSRTC PeerConnection do you understand that ?

Note: until an issue is close any status like wonfix can change. I update label to reflect the last status, not the final.

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

@abardik

It works in Safari and WKWebView at least on iOS 13.4.1 and 13.3.1 (iPhone XS), I don't know about earlier versions. I can send CanvasCaptureMediaStreamTrack to MediaStream.addTrack in Safari, but not in WKWebView.

"It works in Safari and WKWebView" "but not in WKWebView." Sorry I'm confused by your answer, it does work in WKWebView or it does not, OR it does not because you use iosRTC ?

@hthetiot
Copy link
Contributor

Let say it does in WKWebView, let say I allow it to work on native MediaStream (should actually already works with new MediaStream BTW).

what should be the behavior when you add this stream to the RTCPeerConnection in term of failure?

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

Ok I confirm captureStream landed on iOS WKWebView.

/ capture local camera and microphone
navigator.mediaDevices.getUserMedia({ audio: true, video: true }).then(function(stream) {
  // get canvas stream
  var canvas = document.createElement('canvas');
  var canvasStream = canvas.captureStream();
  var track = canvasStream.getVideoTracks()[0];

  // add canvas video track to the stream
  try {
    stream.addTrack(track);
  }
  catch(err) {
    console.error(err); // output: Error: argument must be an instance of MediaStreamTrack
    console.log(track instanceof MediaStreamTrack); // output: false
  }
});

This example is not even logic, you ask for video and audio from iosRTC and add a new video Native MediaStreamTrack on the same iosRTC MediaStream (so you have 2 video tracks on MediaStream).

Let say that you want to capture the VideoStream to draw on the canvas, If so you will need iOSRTC Renderer.save https://github.com/cordova-rtc/cordova-plugin-iosrtc/blob/master/extra/renderer-and-libwebrtc-tests.js#L385, but even with that you will only be able to display the MediaStream on a video tag, not sending it to RTCPeerConnection anyway.

Why use navigator.mediaDevices.getUserMedia in your example that will provide an iosRTC MediaStream that will not support your CanvasCaptureMediaStreamTrack as explained because this is not a WebRTC bridged MediaStreamTrack ?

You expect to be able to send a MediaStream from iosRTC getUserMedia to the Canvas then create an iosRTC MediaStream from it ? That is a features that will require lot of development, mainly converting the video of CanvasCaptureMediaStreamTrack to a WebRTC MediaStreamTrack inside the plugin.

Finally, if what you want is to add CanvasCaptureMediaStreamTrack, to a MediaStream, you can already, you need to use new MediaStream([canvasTrack]) or new MediaStream(canvasStream) instead that handle NativeMediaStreamTrack and result into a NativeMediaStream.

https://github.com/cordova-rtc/cordova-plugin-iosrtc/blob/master/js/MediaStream.js#L47

Screen Shot 2020-05-14 at 1 26 15 PM

This work today on iosrtc and result in a "NativeMediaStreamTrack"

var canvas = document.createElement('canvas');
var canvasStream = canvas.captureStream();

var stream = new MediaStream(canvasStream);

// OR
var canvasTrack = canvasStream.getVideoTracks()[0];
var stream = new MediaStream([canvasTrack]);

But the stream or the track will not be compatible with WebRTC, that would require to convert NativeMediaStreamTrack or NativeMediaStream to iosRTC MediaStreamTrack or MediaStream and I will not implement that features for the moment due that this is a free software I maintain on my spare time.

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

"I'm not asking you to implement any features for me. "
Yes you do, you just dont understand that If I let people use NativeMediaStreamTrack on iosRTC generated MediaStream without making distinction they will expect the track to be streamed via WebRTC when it's not possible that why it does result in argument must be an instance of MediaStreamTrack to prevent this.

You create a iosRTC MediaStream with getUserMedia you can only add iosRTC MediaStreamTrack so you know it can be added to iosRTC PeerConnection.

If you want a "native" Stream with "native" MediaTrack use new MediaStream([canvasTrack]); or new MediaStream(canvasStream); it works today. But I alrady suspect that if you draw on canvas then add on Stream that for sending the stream back via WebRTC, otherwise you can simply display the canvas....

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

Also you have to realize that not all WKWebView are equals for example on iOS 10 "Native" MediaStream does not even exist and iOS 11 and 12 and less recent 13 will not have CaptureStream implemented.

@hthetiot
Copy link
Contributor

hthetiot commented May 14, 2020

Also, CanvasCaptureMediaStreamTrack is a good way to implement collaboration features like whiteboard

No it's not a "good way" to implement whiteboard, you want to have people draw over compressed video stream?

The best way is to implement SVG to canvas (via fabric.js for example) then serialize the vector data result and transfer via JSON.patch to only send what has changed and send it via DataChannel, like that you dont stream video and instead synchronize serialize vector canvas data.

@abardik
Copy link
Author

abardik commented May 14, 2020

@hthetiot

Thank you for the detailed explanation about iosRTC MediaStream.

"It works in Safari and WKWebView" "but not in WKWebView." Sorry I'm confused by your answer, it does work in WKWebView or it does not, OR it does not because you use iosRTC ?

The first part was about captureStream, which you asked for. The second one - about addTrack.

the problem is that it will not work when you add that stream to the iOSRTC PeerConnection do you understand that ?

Yes, I do now. But it wasn't clear at the first time, that I can't send canvas stream to peer connection. It was looking like I can't send it to addTrack because it's not an instanceof MediaStreamTrack.

what should be the behavior when you add this stream to the RTCPeerConnection in term of failure?

Sorry, I don't understand, what do you mean "in term of failure"? I can explain the expected behavior when it's succeeded, because I already have this behavior in other browsers. You just send CanvasCaptureMediaStreamTrack to pc.addTrack or pc.replaceTrack, and then it just appears (even without renegotiation in case of replaceTrack) on the other side of the call as a separate video track which you can attach to any <video>.

This example is not even logic, you ask for video and audio from iosRTC and add a new video Native MediaStreamTrack on the same iosRTC MediaStream (so you have 2 video tracks on MediaStream).

Yes. What exactly is not logic - having more than one video tracks in one media stream which then will be sent over one peer connection? For example, if I want to send camera track (in small resolution, to show in the upper left corner of the recepient's screen, with the ability for him to hide the camera separately from other video tracks) + screen capture track (in maximum resolution, to show in full screen) + canvas capture track (which can be shown separately from camera and screen) - it does not make any sence?

Why use navigator.mediaDevices.getUserMedia in your example that will provide an iosRTC MediaStream that will not support your CanvasCaptureMediaStreamTrack as explained because this is not a WebRTC bridged MediaStreamTrack ?

I proposed a working WebRTC scenario from other browsers, before you explained that it's not possible in iosRTC because MediaStreamTrack provided by iosRTC is not even close to native MediaStreamTrack. I didn't develop iosRTC, so I couldn't know this.

You expect to be able to send a MediaStream from iosRTC getUserMedia to the Canvas then create an iosRTC MediaStream from it ?

No. I expected to combine two or more video tracks from different sources (camera, screen, canvas) in one media stream and then send it via one peer connection.

Finally, if what you want is to add CanvasCaptureMediaStreamTrack, to a MediaStream, you can already, you need to use new MediaStream([canvasTrack]) or new MediaStream(canvasStream) instead that handle NativeMediaStreamTrack and result into a NativeMediaStream.

Yes, thank you for the example.

But the stream or the track will not be compatible with WebRTC, that would require to convert NativeMediaStreamTrack or NativeMediaStream to iosRTC MediaStreamTrack or MediaStream and I will not implement that features for the moment due that this is a free software I maintain on my spare time.

Now, it's totally clear, after you explained that it's a way deeply than just a type incompatibility issue.

If I let people use NativeMediaStreamTrack on iosRTC generated MediaStream without making distinction they will expect the track to be streamed via WebRTC when it's not possible that why it does result in argument must be an instance of MediaStreamTrack to prevent this.

This is exactly what I expected and why I opened this issue.

But I alrady suspect that if you draw on canvas then add on Stream that for sending the stream back via WebRTC, otherwise you can simply display the canvas....

Yes, of course.

Also you have to realize that not all WKWebView are equals for example on iOS 10 "Native" MediaStream does not even exist and iOS 11 and 12 and less recent 13 will not have CaptureStream implemented.

Yes, but it is not the reason to not deliver new features to up-to-dated users, especially if the market share of iOS 13+ is 93% (98% for iOS 11+, where captureStream was appeared the first time in Sep 2017). And you can always show something like "This feature is available only on iOS 11+".

No it's not a "good way" to implement whiteboard, you want to have people draw over compressed video stream?

No, I don't, why would I? I want two separate and compressed video tracks - one for the screen and another one for the witeboard, which will eat a minimum amount of traffic because it will be static almost all the time. I'm not saying this is the best way to implement a whiteboard. I'm just saying this way is good enough to live, at least from the first look.

The best way is to implement SVG to canvas (via fabric.js for example) then serialize the vector data result and transfer via JSON.patch to only send what has changed and send it via DataChannel

Yes, this is probably the best way in terms of traffic and performance, thank you. But it's a way more complicated than just adding additional video (canvas) track into the already established peer connection and then just attach it to <video> on the other peer. But, of course, if you need a two-way whiteboard with the ability to undo the other peer's actions, for example, then yes, the data channel is probably the best way.

Thank you.

@hthetiot
Copy link
Contributor

hthetiot commented May 15, 2020

Yes. What exactly is not logic - having more than one video tracks in one media stream which then will be sent over one peer connection?

The issue here is 2 video track on one MediaStream, it may cause issue depending how you use peerConnection.addTrack or addStream, I recomand you group per stream with addTrack(track, stream1), addTrack(canvasTrack, canvasStream) on browser that support canvasCapture.

@abardik
Copy link
Author

abardik commented May 15, 2020

@hthetiot

Thank you for the suggestion. But could you please briefly explain why? I already moved from addStream/removeStream to addTrack/removeTrack. And replaceTrack was added recently for some cases.

@hthetiot
Copy link
Contributor

hthetiot commented May 17, 2020

Notice that replaceTrack is not supported yet, you may features detect and use addTrack and renegotiate if missing.

Using addTrack with Stream allow to group tack by stream it play nice with many other WebRTC implementtion and unified-plan to my understanding.

@abardik
Copy link
Author

abardik commented May 18, 2020

@hthetiot

Using addTrack with Stream allow to group tack by stream it play nice with many other WebRTC implementtion and unified-plan to my understanding.

Maybe I have to try some examples, because it's still not clear for me what are the benefits of using one-track streams instead of multi-track streams or stream-less tracks.

Anyway, thank you for the suggestions.

@hthetiot hthetiot reopened this Jul 17, 2020
@hthetiot hthetiot removed the wontfix label Jul 17, 2020
@hthetiot
Copy link
Contributor

I may have found a solution.
PeterXu@0f154cf

@hthetiot
Copy link
Contributor

hthetiot commented Jul 17, 2020

@PeterXu I think it's may be time for me to bring your websocket+canvas video renderer fork's changes (https://github.com/PeterXu/cordova-plugin-iosrtc/blob/master/src/PluginMediaStreamRenderer.swift#L335) to the official repository combined with audioContext this time on top of it based on https://github.com/edimuj/cordova-plugin-audioinput.

@PeterXu
Copy link

PeterXu commented Jul 30, 2020

@hthetiot , Please ping me if need my help.

@hthetiot hthetiot added this to the 7.0.x milestone Jul 31, 2020
@hthetiot
Copy link
Contributor

@hthetiot , Please ping me if need my help.

Thank you @PeterXu, i dont know where to start, may be we can create a branch with a PR to start, add websocket then your canvas implementation, see how it goes.

@hthetiot hthetiot modified the milestones: 7.0.x, 7.0.0 Oct 17, 2020
@PeterXu
Copy link

PeterXu commented Oct 19, 2020

This is good way and I will prepare a PR for it.

@hthetiot
Copy link
Contributor

You may merge master if you can, I landed a better track management in Peer injection @PeterXu

@hthetiot
Copy link
Contributor

#587

@hthetiot hthetiot modified the milestones: 7.0.0, 8.0.x Nov 25, 2020
@hthetiot
Copy link
Contributor

@hthetiot hthetiot modified the milestones: 8.0.x, 6.1.0 Jan 20, 2021
@hthetiot
Copy link
Contributor

PR is here #623

@hthetiot hthetiot modified the milestones: 6.1.0, 6.1.x, 7.0.0, 6.2.0 Jan 20, 2021
@hthetiot hthetiot modified the milestones: 6.2.0, triage Mar 1, 2021
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

3 participants