ethand91 / mediasoup-ios-client

Mediasoup 3 iOS Client
ISC License
131 stars 65 forks source link

Create consumers blocked main thread #61

Open Rockjan opened 4 years ago

Rockjan commented 4 years ago

Hi, sorry to trouble you again! The problem is when i create many consumers (about 11 consumers) using this method: [-(Consumer *)consume: id: producerId:rtpParameters:], my app will be frozen for many seconds (the freeze time depend on the device, for iPhone 5s it will be about 10 seconds), even i called this method on a background thread. Then I checked this method's implementation, I found that you indeed dispatch the operation into main thread to avoid the data race and this is the problem. I think if you can create a serial queue to avoid blocking the main thread? Thanks!

ethand91 commented 4 years ago

I think if you can create a serial queue to avoid blocking the main thread?

As long as the method cannot be called twice at the same time it may work. Have a look when i get time.

ethand91 commented 4 years ago

Following your advice the consume call now uses a serial queue, could you try it out? (1.5.2)

Rockjan commented 4 years ago

Following your advice the consume call now uses a serial queue, could you try it out? (1.5.2)

Hi buddy, I think the new method implementation can't work as usual. I found that you used an asynchronous operation to create a consumer obj, as result your method always return nil. So I think you need a block or a synch operation.

Your code : __block Consumer *consumer; dispatch_async(self._workerQueue, ^{ consumer = [TransportWrapper nativeConsume:self._nativeTransport listener:listener id:id producerId:producerId kind:kind rtpParameters:rtpParameters appData:appData]; });

return consumer;

ethand91 commented 4 years ago

Thanks, could you try 1.5.3? Changed the code to use synchronized, this worked for me in a simple app.

Rockjan commented 4 years ago

[!] CocoaPods could not find compatible versions for pod "mediasoup_ios_client": In Podfile: mediasoup_ios_client (= 1.5.3)

Or can you provide me a download link? Thx!

ethand91 commented 4 years ago

You probably need to update the local cocoapod repo.

pod repo update

# Run the below command where your Podfile is
pod install --repo-update
Rockjan commented 4 years ago

It doesn't work. Maybe your latest version is 1.5.2, you told me a wrong version.

And look at the modify record of this file https://github.com/ethand91/mediasoup-ios-client/blob/master/mediasoup-client-ios/src/RecvTransport.mm is still wrong.

ethand91 commented 4 years ago

Cocoapods takes time for the update to take effect, it should work now (1.5.3).

The change was commited to the develop branch

Rockjan commented 4 years ago

Thanks, I will try it out later.

xplatsolutions commented 4 years ago

Could this be related to the problem I am experiencing; or I can open a new issue.

Producer handler methods onConnect and onProduce will hang the application if you try to initiate any async request and block the thread with semaphore wait/signal.

class MediasoupSendTransportHandler : NSObject, SendTransportListener {

  fileprivate weak var delegate: SendTransportListener?
  private var parent: MediasoupRoomManager

  init(parent: MediasoupRoomManager) {
    self.parent = parent
  }

  func onProduce(_ transport: Transport!, kind: String!, rtpParameters: String!, appData: String!, callback: ((String?) -> Void)!) {
    print("SendTransport::onProduce " + kind + " rtpParameters = " + rtpParameters)
    parent.onLocalTransportProduce(transport: transport, kind: kind, rtpParameters: rtpParameters, appData: AppData, callback: callback) // This initiates async request to HTTP REST server and will invoke the callback when finished with producer ID but application freeze if I try to use semaphore to block the thread to sync and signal continuation
  }

  func onConnect(_ transport: Transport!, dtlsParameters: String!) {
    print("SendTransport::onConnect dtlsParameters = " + dtlsParameters)
    parent.onLocalTransportConnect(transport: transport, dtlsParameters: dtlsParameters) // This initiates async request to HTTP REST server but application freeze if I try to use semaphore to block the thread to sync and signal continuation
  }

  func onConnectionStateChange(_ transport: Transport!, connectionState: String!) {
    print("SendTransport::onConnectionStateChange connectionState = " + connectionState)
  }

}

Any ideas on how to make it work with async requests to the server? I can provide more details and code if you want me to open a new issue.

ethand91 commented 4 years ago

@xplatsolutions

Producer handler methods onConnect and onProduce will hang the application if you try to initiate any async request and block the thread with semaphore wait/signal.

Does it work if you remove the semaphore wait/signal? You shouldn't need semaphore anymore.

xplatsolutions commented 4 years ago

Updated to 1.5.3, the problem persists, if you don't invoke the callback: ((String?) -> Void)! before exiting the SendTransportListener.onProduce will freeze the application, I am trying various tricks to accomplish this, the next I am trying is to block the onProduce method until the HTTP REST async call is finished, maybe pass the semaphore to the methods;

Immediately calling the callback with a random producer ID will work but that won't do any good.

I'm all about ideas on this issue.

xplatsolutions commented 4 years ago

I opened a new issue to provide clarity of the problem and detail information to reproduce if necessary. https://github.com/ethand91/mediasoup-ios-client/issues/67

Rockjan commented 4 years ago

Cocoapods takes time for the update to take effect, it should work now (1.5.3).

The change was commited to the develop branch

Thx, it works and we decide to test it when we get time.

xplatsolutions commented 4 years ago

@Rockjan could you please provide if you have to sync threads when onProduce is called, I have trouble to make it work because it freezes the application. https://github.com/ethand91/mediasoup-ios-client/issues/67

I appreciate help on this...pretty stuck

Rockjan commented 4 years ago

@Rockjan could you please provide if you have to sync threads when onProduce is called, I have trouble to make it work because it freezes the application. #67

I appreciate help on this...pretty stuck

Hi, I send an async request to create a producer and within the callback I called the "callback" block to finished the proc.

My code : -(void)onProduce:(Transport )transport kind:(NSString )kind rtpParameters:(NSString )rtpParameters appData:(NSString )appData callback:(void(^)(NSString ))callback { if (transport.getId && kind && rtpParameters && appData) { NSDictionary rtpParam = [NSDictionary dictionaryWithJsonString:rtpParameters];

    [self.wsServer sendRequestWithMethod:@"produce" param:@{@"transportId":transport.getId, @"kind":kind, @"rtpParameters":rtpParam, @"appData":appData} completion:^(NSDictionary * _Nonnull payload, NSError * _Nonnull error) {

        if (error)
        {
            NSLog(@"- Connect transport failed : %@ (%@, %@)-", error.localizedDescription,transport.getId, kind);
        }

        if ([payload isKindOfClass:[NSDictionary class]])
        {
            NSString *producerId = payload[@"id"];
            callback(producerId);
        }
        else
        {
            callback(@"");
        }
    }];
}

}

Hope this helps.

xplatsolutions commented 4 years ago

So self.wsServer sendRequestWithMethod is asynchronous creating a background thread; to complete the request to a Web Socket channel and if onProduce exit you don't receive an error like the below!

I assume you block the thread somehow or things happen in same thread since I tried everything to make it work with async HTTP request so far.

[ERROR] transport_wrapper::+[TransportWrapper nativeProduce:listener:track:encodings:codecOptions:appData:]() | The associated promise has been destructed prior to the associated state becoming ready.
2020-07-15 03:54:34.122181-0400 Phone[2934:1160855] *** Terminating app due to uncaught exception 'RuntimeException', reason: 'The associated promise has been destructed prior to the associated state becoming ready.'

I use Swift latest and iOS 13 but I don't see why this would be different but I see you are using Web Socket and I am using a REST HTTP API.

Rockjan commented 4 years ago

So self.wsServer sendRequestWithMethod is asynchronous creating a background thread; to complete the request to a Web Socket channel and if onProduce exit you don't receive an error like the below!

I assume you block the thread somehow or things happen in same thread since I tried everything to make it work with async HTTP request so far.

[ERROR] transport_wrapper::+[TransportWrapper nativeProduce:listener:track:encodings:codecOptions:appData:]() | The associated promise has been destructed prior to the associated state becoming ready.
2020-07-15 03:54:34.122181-0400 Phone[2934:1160855] *** Terminating app due to uncaught exception 'RuntimeException', reason: 'The associated promise has been destructed prior to the associated state becoming ready.'

I use Swift latest and iOS 13 but I don't see why this would be different but I see you are using Web Socket and I am using a REST HTTP API.

I didn't receive that error ever. The websocket request was triggered on Thread-10, the same thread with method invoked, and the complete block was invoked on Thread-13, I didn't use any method to block Thread-10 to waiting for the completation of Thread-13. The difference is that I use oc + websocket and you use swift + http. But I think this doesn't make sense.

xplatsolutions commented 4 years ago

Yeah, it is super weird getting this kind of error but I am sure it has to do more with the Web Socket <> HTTP frameworks thread creation; As far as I understand Web Socket will establish a channel in a thread and will never change so that might be a significant difference between creating a background thread per request like HTTP frameworks do (AlamoFire - which uses URLSession behind the scene or plain URLSession).

I am not even passing the callback anywhere other than invoking it into a completion handler like you.

All samples so far regarding the iOS mediasoup client are with web socket and it seems I am the first that is using an HTTP API but I'm sure we will figure it out, better now than later since using HTTP REST is very common today.

I am also using SwiftUI but still, I don't see why would that make a difference.

Rockjan commented 4 years ago

Yeah, it is super weird getting this kind of error but I am sure it has to do more with the Web Socket <> HTTP frameworks thread creation; As far as I understand Web Socket will establish a channel in a thread and will never change so that might be a significant difference between creating a background thread per request like HTTP frameworks do (AlamoFire - which uses URLSession behind the scene or plain URLSession).

I am not even passing the callback anywhere other than invoking it into a completion handler like you.

All samples so far regarding the iOS mediasoup client are with web socket and it seems I am the first that is using an HTTP API but I'm sure we will figure it out, better now than later since using HTTP REST is very common today.

I am also using SwiftUI but still, I don't see why would that make a difference.

Yes you are right, I use a serial queue to handle websocket callbacks.

Maybe you can build a request manager above AlamoFire framework with a serial queue just like the websocket callback queue.

xplatsolutions commented 4 years ago

I am changing the networking protocol to try if it works OK with Web Sockets although mediasoup is signal agnostic and we should support HTTP REST, ethan91 will jump to a sample project and reproduce the issue, meanwhile, I will keep two implementations to be able to test moving forward and stick with one at the end.

xplatsolutions commented 4 years ago

@Rockjan let me know after testing the latest 1.5.3 cocoapods if you get this issue, please. https://github.com/ethand91/mediasoup-ios-client/issues/68

Rockjan commented 4 years ago

@Rockjan let me know after testing the latest 1.5.3 cocoapods if you get this issue, please. #68

OK! Maybe we will start the testing next week.