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

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

Closed
lets-swapcode opened this issue Jul 6, 2023 · 6 comments
Labels
enhancement New feature or request fixed Fixed and needs validation

Comments

@lets-swapcode
Copy link

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
Copy link
Author

[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
Copy link
Author

[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
Copy link
Collaborator

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
Copy link
Collaborator

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

lets-swapcode added a commit to lets-swapcode/mediasoup-client-swift-pro that referenced this issue Aug 5, 2023
…support customisation for peerConnection options to take Ice servers and relay transport policy
@fedulvtubudul
Copy link
Collaborator

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.

@fedulvtubudul fedulvtubudul added enhancement New feature or request fixed Fixed and needs validation labels Aug 16, 2023
@lets-swapcode
Copy link
Author

lets-swapcode commented Sep 11, 2023

Thanks. Tested and working perfectly fine.

Please add support for iOS 12 also so that we don't have to maintain a local clone.
#18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed Fixed and needs validation
Projects
None yet
Development

No branches or pull requests

2 participants