OpenTelecom / WKWebViewRTC

MIT License
94 stars 30 forks source link

Video support #9

Closed JustDoIt9 closed 4 years ago

hthetiot commented 4 years ago

@noahmehl Note: fixed issue on iosRTC today that you may benefit from https://github.com/cordova-rtc/cordova-plugin-iosrtc/commit/f9588292caea8f04d3b79f4f5a92bb3a06288cf4

See implementation that need to be fixed here https://github.com/OpenTelecom/WKWebViewRTC/blob/video-support/WKWebViewRTC/Classes/iRTCPeerConnection.swift#L640

noahmehl commented 4 years ago

@hthetiot WOW! thank you so much! @JustDoIt9 could you please integrate?

Else wise, @hthetiot thank you for all of the work on this!

hthetiot commented 4 years ago

deleteMediaStreamTrack is not the one, you want removeMediaStreamTrack

noahmehl commented 4 years ago

@JustDoIt9 I have tested this on an iPhone 11 Pro Max running iOS 14.2. The Cordova sample is working for me. I will test with an iPad in the office on Monday. I also bumped the pod spec version number.

noahmehl commented 4 years ago

@hthetiot I think we're completely open to anything at this point. We tried to do our best to attribute your work where appropriate. I do think that there's a slight separation of concerns about having a WebRTC framework, and having a bridge between that and WkWebView specifically. The Cordova plugin is also specific to the Cordova project, so I'm not sure it should live there either?

That being said, I'm wondering if we should use the https://github.com/cordova-rtc/cordova-plugin-iosrtc as a git submodule so that it's tracked 1:1 with your work?

In the short term, we are, and were, just trying to get something that works, and there does seem to be a need for it (a vanilla WkWebView shim).

Again, your work (as well as the rest of the Cordova community, etc) is very much appreciated, and we want to do our best to meet your expectations :)

hthetiot commented 4 years ago

I think we can use the same JS and swift and create abstract layer for using Cordova or your bridge, we also looking add using websocket on iosRTC for performance and Stream.

We can put that here https://github.com/cordova-rtc/iosrtc.WebRTC.library

And you can let user use pod from google or iosRTC build for webRTC framework via https://github.com/cordova-rtc/iosrtc.webRTC.framework

Notice that we have M87, M79 and a patched M69 for iOS 14. We also moving to Typescript (PR https://github.com/cordova-rtc/cordova-plugin-iosrtc/pull/599) and working on addTransceiver (https://github.com/cordova-rtc/cordova-plugin-iosrtc/issues/589)

Let think about it. First step is make an issue in https://github.com/cordova-rtc/iosrtc.WebRTC.library to define scope then PR based on your fork but with adapter layer.

hthetiot commented 4 years ago

Cordova usage can be in plugin only and plugin to use library not need adapter, doh one using websocket you may want it.

xhamrak commented 4 years ago

Hi, I don't know, if this is only my problem, but if I testing it on this web: https://webrtc.github.io/test-pages/ , the links doesn't work. Also, it is problem on master branch too, so maybe problem with new ios(in september it works, and it is problem on more pages)? I testing it on xs pro max 14.2 and xcode 12.3 beta. (On simulator doesn't work too) Can someone check it? Thanks

JustDoIt9 commented 4 years ago

Hi, I don't know, if this is only my problem, but if I testing it on this web: https://webrtc.github.io/test-pages/ , the links doesn't work. Also, it is problem on master branch too, so maybe problem with new ios(in september it works, and it is problem on more pages)? I testing it on xs pro max 14.2 and xcode 12.3 beta. (On simulator doesn't work too) Can someone check it? Thanks

Yea, some functionalities of iosrtc-plugin don't work for official webrtc tests. So, video tests are working well but not for audio. And, this is not a problem of ios version, I guess.

On the other hand, audio support of iosrtc-plugin works for sip.js tests. (A little weird. )

xhamrak commented 4 years ago

Hi, I don't know, if this is only my problem, but if I testing it on this web: https://webrtc.github.io/test-pages/ , the links doesn't work. Also, it is problem on master branch too, so maybe problem with new ios(in september it works, and it is problem on more pages)? I testing it on xs pro max 14.2 and xcode 12.3 beta. (On simulator doesn't work too) Can someone check it? Thanks

Yea, some functionalities of iosrtc-plugin don't work for official webrtc tests. So, video tests are working well but not for audio. And, this is not a problem of ios version, I guess.

On the other hand, audio support of iosrtc-plugin works for sip.js tests. (A little weird. )

I don't know, but it doesn't work on my page. If I want turn on camera or mic, it freeze. Two months ago mic works, so I think, it have something to do with updates.

noahmehl commented 4 years ago

@xhamrak could you please be more specific? Which test was working on master (as well as iOS version)? The same test is not working now?

xhamrak commented 4 years ago

@xhamrak could you please be more specific? Which test was working on master (as well as iOS version)? The same test is not working now?

On iOS 13.7 I tested master on two pages. On my, mic was working(I tested it now on iOS 14.2 and works as well). I also tested on this page https://webrtc.github.io/test-pages/ and I remember, that those links were working. Now, on both branch master and this, it doesn't work. So, I think, that iOS have some bug, that those links not working.

But, on my page, microphone works on master branch, but when I run this branch with video, even if I don't turn on camera, only mic, the page freeze. With camera same situation. I don't know where is a problem.

JustDoIt9 commented 4 years ago

https://github.com/cordova-rtc/cordova-plugin-iosrtc-sample

This is the official test for iosrtc-plugin, and you should check out test project.

noahmehl commented 4 years ago

@hthetiot Can you please give us some guidance (or just info) on why the iosrtc.webRTC.framework (M87 for instance) is preferable over the GoogleWebRTC Pod v1.1.29229?

noahmehl commented 4 years ago

@hthetiot we can create a separate PR for switching the WebRTC framework if needed.

hthetiot commented 3 years ago

@xhamrak could you please be more specific? Which test was working on master (as well as iOS version)? The same test is not working now?

On iOS 13.7 I tested master on two pages. On my, mic was working(I tested it now on iOS 14.2 and works as well). I also tested on this page https://webrtc.github.io/test-pages/ and I remember, that those links were working. Now, on both branch master and this, it doesn't work. So, I think, that iOS have some bug, that those links not working.

But, on my page, microphone works on master branch, but when I run this branch with video, even if I don't turn on camera, only mic, the page freeze. With camera same situation. I don't know where is a problem.

If you had issue on 14.3 but not 13+ and 14.2 it's most likely due the last 6.0.17 fix that is missing in this library: https://github.com/cordova-rtc/cordova-plugin-iosrtc/issues/618#issuecomment-763527698

xhamrak commented 3 years ago

@xhamrak could you please be more specific? Which test was working on master (as well as iOS version)? The same test is not working now?

On iOS 13.7 I tested master on two pages. On my, mic was working(I tested it now on iOS 14.2 and works as well). I also tested on this page https://webrtc.github.io/test-pages/ and I remember, that those links were working. Now, on both branch master and this, it doesn't work. So, I think, that iOS have some bug, that those links not working. But, on my page, microphone works on master branch, but when I run this branch with video, even if I don't turn on camera, only mic, the page freeze. With camera same situation. I don't know where is a problem.

If you had issue on 14.3 but not 13+ and 14.2 it's most likely due the last 6.0.17 fix that is missing in this library: cordova-rtc/cordova-plugin-iosrtc#618 (comment)

Okay, thanks, lately I look at it, maybe this weekend.