VLprojects / mediasoup-client-swift

Swift wrapper for libmediasoupclient
MIT License
42 stars 17 forks source link

load device to set ice server option missing (available in android/ web) #14

Closed lets-swapcode closed 1 year ago

lets-swapcode commented 1 year ago

Our repo - device.load(with: rtpCapabilities) Android, web repo has extra parameters to set ICE server configuration - check last load function

` code

        for (i in 0 until mIceServers!!.length()) {
            val server = mIceServers!!.getJSONObject(i)
            var iceServer = IceServer(
                server.getString("urls"),
                server.getString("username"),
                server.getString("credential")
            )
            serverList.add(iceServer)
        }
        val rtcConfiguration = RTCConfiguration(serverList)
        if (icePolicy == "relay") rtcConfiguration.iceTransportsType =
            org.webrtc.PeerConnection.IceTransportsType.RELAY
        mOptions!!.setRTCConfig(rtcConfiguration)
        mJMMediaDevice?.load(rtpCapabilities.toString(), mOptions)

`

so how to set ice server in iOS? I can see send transport has few functions. ` code

public func restartICE(with iceParameters: String) throws
public func updateICEServers(_ iceServers: String) throws
public func updateICETransportPolicy(_ transportPolicy: Mediasoup.ICETransportPolicy) throws

`

Question

  1. What is the right way to set ice server like android?
  2. Will updateICEServer work? do we need to call restartICE as well? Note - I tried this too. server is not receiving any ice server info. please help.
lets-swapcode commented 1 year ago

[update]

We checked the source code for swift wrapper. class device wrapper.mm.

The function load device does not have the peer connection parameter and it is taking default one which you have initialised in init.

`device wrapper.mm

(void) loadWithRouterRTPCapabilities:(NSString ) routerRTPCapabilities error:(out NSError __autoreleasing _Nullable *_Nullable)error {

mediasoupTry(^{
    auto routerRTPCapabilitiesString = std::string(routerRTPCapabilities.UTF8String);
    auto routerRTPCapabilitiesJSON = nlohmann::json::parse(routerRTPCapabilitiesString);
    self->_device->Load(routerRTPCapabilitiesJSON, self->_pcOptions);
}, error);

} `

please expose/ add this parameter in the function itself so that I can pass the server list.

lets-swapcode commented 1 year ago

[update]

We cloned the project and added the missing implementation for peer connection options.

We are testing it for few days now, it is working fine.

fedulvtubudul commented 1 year ago

You should remember that mediasoup-client-swift is just a wrapper on top of libmediasoupclient. Each time we change something in the API it makes harder to keep it in sync when libmediasoupclient is updated. Thats why I don't want to add something that is not available in the C++ version of libemediasoupclient.

If we look at the TypeScript implementation of the mediasoup client, they have the iceServers and iceTransportPolicy parameters when creating a transport. C++ version of the libmediasoupclient does not have these parameters. Thats why Swift wrapper does not have them as well.

So what's really need to be done in the first place is to implement these parameters in C++ library. I've created an issue on the original library repo and will try to make what you suggest, but it will take time.

Closing this issue as it's not a problem of wrapper itself and issue in libmediasoupclient will be more relevant.

fedulvtubudul commented 1 year ago

@lets-swapcode next day I finally realised what you've actually meant 😅. And yep, that parameter should be added on mediasoup-client-swift side.

fedulvtubudul commented 1 year ago

Please check version 0.4.2. I've added parameters you need into Device methods createSendTransport and createReceiveTransport. Please tell if it solves your task with ICE servers configuration.

I've seen your PR and really appreciate your help. But I see couple of issues with that PR.

First of all it may not work with some ICE server configurations. As docs says, username and credentials parameters are optional, and urls can be "either a single string or an array of strings". So beware of that if you use the code you've submitted in this PR.

Second thing that is not an issue by itself, but can become such in future. You've added a new parameter to the Device load method, witch does not exist in TS version of the mediasoup client. But developers of libmediasoupclient and mediasoup-client use TS implementation as a reference, while C++ implementation is a "clone" which is supposed to be as close as possible to the TS version. All the other mediasoup clients are just wrappers on top of that two. So I suggest copying TS implementation API, not the Android or any other "secondary" implementation.

lets-swapcode commented 1 year ago

Thanks. Tested and working perfectly fine.

Please add support for iOS 12 also so that we don't have to maintain a local clone. https://github.com/VLprojects/mediasoup-client-swift/issues/18