From 61e78fca6cb3499fda9fce4a19d0f62ead8afbe8 Mon Sep 17 00:00:00 2001 From: "fischman@webrtc.org" Date: Mon, 31 Mar 2014 20:16:49 +0000 Subject: [PATCH] AppRTCDemo(iOS): remote-video reliability fixes Previously GAE Channel callbacks would be handled by JS string-encoding the payload into a URL. Unfortunately this is limited to the (undocumented, silently problematic) maximum URL length UIWebView supports. Replaced this scheme by a notification from JS to ObjC and a getter from ObjC to JS (which happens out-of-line to avoid worrying about UIWebView's re-entrancy, or lack thereof). Part of this change also moved from a combination of: JSON, URL-escaping, and ad-hoc :-separated values to simply JSON. Also incidentally: - Removed outdated TODO about onRenegotiationNeeded, which is unneeded - Move handling of PeerConnection callbacks to the main queue to avoid having to think about concurrency too hard. - Replaced a bunch of NSOrderedSame with isEqualToString for clearer code and not having to worry about the fact that [nil compare:@"foo"]==NSOrderedSame is always true (yay ObjC!). - Auto-scroll messages view. BUG=3117 R=noahric@google.com Review URL: https://webrtc-codereview.appspot.com/10899006 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5814 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../ios/AppRTCDemo/APPRTCAppDelegate.m | 164 +++++++++--------- .../ios/AppRTCDemo/APPRTCViewController.h | 6 +- .../ios/AppRTCDemo/APPRTCViewController.m | 26 +-- .../ios/AppRTCDemo/GAEChannelClient.h | 2 +- .../ios/AppRTCDemo/GAEChannelClient.m | 59 ++++--- .../en.lproj/APPRTCViewController.xib | 24 +-- talk/examples/ios/AppRTCDemo/ios_channel.html | 30 ++-- 7 files changed, 168 insertions(+), 143 deletions(-) diff --git a/talk/examples/ios/AppRTCDemo/APPRTCAppDelegate.m b/talk/examples/ios/AppRTCDemo/APPRTCAppDelegate.m index 8400778ac1..e8077b9620 100644 --- a/talk/examples/ios/AppRTCDemo/APPRTCAppDelegate.m +++ b/talk/examples/ios/AppRTCDemo/APPRTCAppDelegate.m @@ -64,19 +64,23 @@ } - (void)peerConnectionOnError:(RTCPeerConnection*)peerConnection { - NSLog(@"PCO onError."); - NSAssert(NO, @"PeerConnection failed."); + dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onError."); + NSAssert(NO, @"PeerConnection failed."); + }); } - (void)peerConnection:(RTCPeerConnection*)peerConnection signalingStateChanged:(RTCSignalingState)stateChanged { - NSLog(@"PCO onSignalingStateChange: %d", stateChanged); + dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onSignalingStateChange: %d", stateChanged); + }); } - (void)peerConnection:(RTCPeerConnection*)peerConnection addedStream:(RTCMediaStream*)stream { - NSLog(@"PCO onAddStream."); dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onAddStream."); NSAssert([stream.audioTracks count] <= 1, @"Expected at most 1 audio stream"); NSAssert([stream.videoTracks count] <= 1, @@ -90,49 +94,57 @@ - (void)peerConnection:(RTCPeerConnection*)peerConnection removedStream:(RTCMediaStream*)stream { - NSLog(@"PCO onRemoveStream."); + dispatch_async(dispatch_get_main_queue(), + ^(void) { NSLog(@"PCO onRemoveStream."); }); } - (void)peerConnectionOnRenegotiationNeeded:(RTCPeerConnection*)peerConnection { - NSLog(@"PCO onRenegotiationNeeded."); - // TODO(hughv): Handle this. + dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onRenegotiationNeeded - ignoring because AppRTC has a " + "predefined negotiation strategy"); + }); } - (void)peerConnection:(RTCPeerConnection*)peerConnection gotICECandidate:(RTCICECandidate*)candidate { - NSLog(@"PCO onICECandidate.\n Mid[%@] Index[%d] Sdp[%@]", - candidate.sdpMid, - candidate.sdpMLineIndex, - candidate.sdp); - NSDictionary* json = @{ - @"type" : @"candidate", - @"label" : [NSNumber numberWithInt:candidate.sdpMLineIndex], - @"id" : candidate.sdpMid, - @"candidate" : candidate.sdp - }; - NSError* error; - NSData* data = - [NSJSONSerialization dataWithJSONObject:json options:0 error:&error]; - if (!error) { - [_delegate sendData:data]; - } else { - NSAssert(NO, - @"Unable to serialize JSON object with error: %@", - error.localizedDescription); - } + dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onICECandidate.\n Mid[%@] Index[%d] Sdp[%@]", + candidate.sdpMid, + candidate.sdpMLineIndex, + candidate.sdp); + NSDictionary* json = @{ + @"type" : @"candidate", + @"label" : [NSNumber numberWithInt:candidate.sdpMLineIndex], + @"id" : candidate.sdpMid, + @"candidate" : candidate.sdp + }; + NSError* error; + NSData* data = + [NSJSONSerialization dataWithJSONObject:json options:0 error:&error]; + if (!error) { + [_delegate sendData:data]; + } else { + NSAssert(NO, + @"Unable to serialize JSON object with error: %@", + error.localizedDescription); + } + }); } - (void)peerConnection:(RTCPeerConnection*)peerConnection iceGatheringChanged:(RTCICEGatheringState)newState { - NSLog(@"PCO onIceGatheringChange. %d", newState); + dispatch_async(dispatch_get_main_queue(), + ^(void) { NSLog(@"PCO onIceGatheringChange. %d", newState); }); } - (void)peerConnection:(RTCPeerConnection*)peerConnection iceConnectionChanged:(RTCICEConnectionState)newState { - NSLog(@"PCO onIceConnectionChange. %d", newState); - if (newState == RTCICEConnectionConnected) - [self displayLogMessage:@"ICE Connection Connected."]; - NSAssert(newState != RTCICEConnectionFailed, @"ICE Connection failed!"); + dispatch_async(dispatch_get_main_queue(), ^(void) { + NSLog(@"PCO onIceConnectionChange. %d", newState); + if (newState == RTCICEConnectionConnected) + [self displayLogMessage:@"ICE Connection Connected."]; + NSAssert(newState != RTCICEConnectionFailed, @"ICE Connection failed!"); + }); } - (void)displayLogMessage:(NSString*)message { @@ -198,6 +210,7 @@ } - (void)displayLogMessage:(NSString*)message { + NSAssert([NSThread isMainThread], @"Called off main thread!"); NSLog(@"%@", message); [self.viewController displayText:message]; } @@ -263,7 +276,7 @@ [lms addAudioTrack:[self.peerConnectionFactory audioTrackWithID:@"ARDAMSa0"]]; [self.peerConnection addStream:lms constraints:constraints]; - [self displayLogMessage:@"onICEServers - add local stream."]; + [self displayLogMessage:@"onICEServers - added local stream."]; } #pragma mark - GAEMessageHandler methods @@ -286,24 +299,15 @@ [self displayLogMessage:@"PC - createOffer."]; } -- (void)onMessage:(NSString*)data { - NSString* message = [self unHTMLifyString:data]; - NSError* error; - NSDictionary* objects = [NSJSONSerialization - JSONObjectWithData:[message dataUsingEncoding:NSUTF8StringEncoding] - options:0 - error:&error]; - NSAssert(!error, - @"%@", - [NSString stringWithFormat:@"Error: %@", error.description]); - NSAssert([objects count] > 0, @"Invalid JSON object"); - NSString* value = [objects objectForKey:@"type"]; +- (void)onMessage:(NSDictionary*)messageData { + NSString* type = messageData[@"type"]; + NSAssert(type, @"Missing type: %@", messageData); [self displayLogMessage:[NSString stringWithFormat:@"GAE onMessage type - %@", - value]]; - if ([value compare:@"candidate"] == NSOrderedSame) { - NSString* mid = [objects objectForKey:@"id"]; - NSNumber* sdpLineIndex = [objects objectForKey:@"label"]; - NSString* sdp = [objects objectForKey:@"candidate"]; + type]]; + if ([type isEqualToString:@"candidate"]) { + NSString* mid = messageData[@"id"]; + NSNumber* sdpLineIndex = messageData[@"label"]; + NSString* sdp = messageData[@"candidate"]; RTCICECandidate* candidate = [[RTCICECandidate alloc] initWithMid:mid index:sdpLineIndex.intValue @@ -313,16 +317,16 @@ } else { [self.peerConnection addICECandidate:candidate]; } - } else if (([value compare:@"offer"] == NSOrderedSame) || - ([value compare:@"answer"] == NSOrderedSame)) { - NSString* sdpString = [objects objectForKey:@"sdp"]; + } else if ([type isEqualToString:@"offer"] || + [type isEqualToString:@"answer"]) { + NSString* sdpString = messageData[@"sdp"]; RTCSessionDescription* sdp = [[RTCSessionDescription alloc] - initWithType:value + initWithType:type sdp:[APPRTCAppDelegate preferISAC:sdpString]]; [self.peerConnection setRemoteDescriptionWithDelegate:self sessionDescription:sdp]; [self displayLogMessage:@"PC - setRemoteDescription."]; - } else if ([value compare:@"bye"] == NSOrderedSame) { + } else if ([type isEqualToString:@"bye"]) { [self closeVideoUI]; UIAlertView* alertView = [[UIAlertView alloc] initWithTitle:@"Remote end hung up" @@ -332,7 +336,7 @@ otherButtonTitles:nil]; [alertView show]; } else { - NSAssert(NO, @"Invalid message: %@", data); + NSAssert(NO, @"Invalid message: %@", messageData); } } @@ -342,8 +346,8 @@ } - (void)onError:(int)code withDescription:(NSString*)description { - [self displayLogMessage:[NSString stringWithFormat:@"GAE onError: %@", - description]]; + [self displayLogMessage:[NSString stringWithFormat:@"GAE onError: %d, %@", + code, description]]; [self closeVideoUI]; } @@ -400,8 +404,8 @@ [newMLine addObject:[origMLineParts objectAtIndex:origPartIndex++]]; [newMLine addObject:isac16kRtpMap]; for (; origPartIndex < [origMLineParts count]; ++origPartIndex) { - if ([isac16kRtpMap compare:[origMLineParts objectAtIndex:origPartIndex]] != - NSOrderedSame) { + if (![isac16kRtpMap + isEqualToString:[origMLineParts objectAtIndex:origPartIndex]]) { [newMLine addObject:[origMLineParts objectAtIndex:origPartIndex]]; } } @@ -415,20 +419,20 @@ - (void)peerConnection:(RTCPeerConnection*)peerConnection didCreateSessionDescription:(RTCSessionDescription*)origSdp error:(NSError*)error { - if (error) { - [self displayLogMessage:@"SDP onFailure."]; - NSAssert(NO, error.description); - return; - } - - [self displayLogMessage:@"SDP onSuccess(SDP) - set local description."]; - RTCSessionDescription* sdp = [[RTCSessionDescription alloc] - initWithType:origSdp.type - sdp:[APPRTCAppDelegate preferISAC:origSdp.description]]; - [self.peerConnection setLocalDescriptionWithDelegate:self - sessionDescription:sdp]; - [self displayLogMessage:@"PC setLocalDescription."]; dispatch_async(dispatch_get_main_queue(), ^(void) { + if (error) { + [self displayLogMessage:@"SDP onFailure."]; + NSAssert(NO, error.description); + return; + } + [self displayLogMessage:@"SDP onSuccess(SDP) - set local description."]; + RTCSessionDescription* sdp = [[RTCSessionDescription alloc] + initWithType:origSdp.type + sdp:[APPRTCAppDelegate preferISAC:origSdp.description]]; + [self.peerConnection setLocalDescriptionWithDelegate:self + sessionDescription:sdp]; + + [self displayLogMessage:@"PC setLocalDescription."]; NSDictionary* json = @{@"type" : sdp.type, @"sdp" : sdp.description}; NSError* error; NSData* data = @@ -442,14 +446,14 @@ - (void)peerConnection:(RTCPeerConnection*)peerConnection didSetSessionDescriptionWithError:(NSError*)error { - if (error) { - [self displayLogMessage:@"SDP onFailure."]; - NSAssert(NO, error.description); - return; - } - - [self displayLogMessage:@"SDP onSuccess() - possibly drain candidates"]; dispatch_async(dispatch_get_main_queue(), ^(void) { + if (error) { + [self displayLogMessage:@"SDP onFailure."]; + NSAssert(NO, error.description); + return; + } + + [self displayLogMessage:@"SDP onSuccess() - possibly drain candidates"]; if (!self.client.initiator) { if (self.peerConnection.remoteDescription && !self.peerConnection.localDescription) { diff --git a/talk/examples/ios/AppRTCDemo/APPRTCViewController.h b/talk/examples/ios/AppRTCDemo/APPRTCViewController.h index c42a37293e..f5fcee41f5 100644 --- a/talk/examples/ios/AppRTCDemo/APPRTCViewController.h +++ b/talk/examples/ios/AppRTCDemo/APPRTCViewController.h @@ -32,9 +32,9 @@ // The view controller that is displayed when AppRTCDemo is loaded. @interface APPRTCViewController : UIViewController -@property(weak, nonatomic) IBOutlet UITextField* textField; -@property(weak, nonatomic) IBOutlet UITextView* textInstructions; -@property(weak, nonatomic) IBOutlet UITextView* textOutput; +@property(weak, nonatomic) IBOutlet UITextField* roomInput; +@property(weak, nonatomic) IBOutlet UITextView* instructionsView; +@property(weak, nonatomic) IBOutlet UITextView* logView; @property(weak, nonatomic) IBOutlet UIView* blackView; @property(nonatomic, strong) APPRTCVideoView* remoteVideoView; diff --git a/talk/examples/ios/AppRTCDemo/APPRTCViewController.m b/talk/examples/ios/AppRTCDemo/APPRTCViewController.m index 3cebb7239c..0ac92823f9 100644 --- a/talk/examples/ios/AppRTCDemo/APPRTCViewController.m +++ b/talk/examples/ios/AppRTCDemo/APPRTCViewController.m @@ -41,8 +41,8 @@ [super viewDidLoad]; self.statusBarOrientation = [UIApplication sharedApplication].statusBarOrientation; - self.textField.delegate = self; - [self.textField becomeFirstResponder]; + self.roomInput.delegate = self; + [self.roomInput becomeFirstResponder]; } - (void)viewDidLayoutSubviews { @@ -59,18 +59,20 @@ - (void)displayText:(NSString*)text { dispatch_async(dispatch_get_main_queue(), ^(void) { NSString* output = - [NSString stringWithFormat:@"%@\n%@", self.textOutput.text, text]; - self.textOutput.text = output; + [NSString stringWithFormat:@"%@\n%@", self.logView.text, text]; + self.logView.text = output; + [self.logView + scrollRangeToVisible:NSMakeRange([self.logView.text length], 0)]; }); } - (void)resetUI { - [self.textField resignFirstResponder]; - self.textField.text = nil; - self.textField.hidden = NO; - self.textInstructions.hidden = NO; - self.textOutput.hidden = YES; - self.textOutput.text = nil; + [self.roomInput resignFirstResponder]; + self.roomInput.text = nil; + self.roomInput.hidden = NO; + self.instructionsView.hidden = NO; + self.logView.hidden = YES; + self.logView.text = nil; self.blackView.hidden = YES; [_remoteVideoView renderVideoTrackInterface:nil]; @@ -145,8 +147,8 @@ enum { return; } textField.hidden = YES; - self.textInstructions.hidden = YES; - self.textOutput.hidden = NO; + self.instructionsView.hidden = YES; + self.logView.hidden = NO; // TODO(hughv): Instead of launching a URL with apprtc scheme, change to // prepopulating the textField with a valid URL missing the room. This allows // the user to have the simplicity of just entering the room or the ability to diff --git a/talk/examples/ios/AppRTCDemo/GAEChannelClient.h b/talk/examples/ios/AppRTCDemo/GAEChannelClient.h index 49a928d821..8c7d5d3472 100644 --- a/talk/examples/ios/AppRTCDemo/GAEChannelClient.h +++ b/talk/examples/ios/AppRTCDemo/GAEChannelClient.h @@ -33,7 +33,7 @@ @protocol GAEMessageHandler - (void)onOpen; -- (void)onMessage:(NSString *)data; +- (void)onMessage:(NSDictionary*)data; - (void)onClose; - (void)onError:(int)code withDescription:(NSString *)description; diff --git a/talk/examples/ios/AppRTCDemo/GAEChannelClient.m b/talk/examples/ios/AppRTCDemo/GAEChannelClient.m index 1b5e55924b..fcd0787ef6 100644 --- a/talk/examples/ios/AppRTCDemo/GAEChannelClient.m +++ b/talk/examples/ios/AppRTCDemo/GAEChannelClient.m @@ -63,41 +63,54 @@ #pragma mark - UIWebViewDelegate method ++ (NSDictionary*)jsonStringToDictionary:(NSString*)str { + NSData* data = [str dataUsingEncoding:NSUTF8StringEncoding]; + NSError* error; + NSDictionary* dict = + [NSJSONSerialization JSONObjectWithData:data options:0 error:&error]; + NSAssert(!error, @"Invalid JSON? %@", str); + return dict; +} + - (BOOL)webView:(UIWebView*)webView shouldStartLoadWithRequest:(NSURLRequest*)request navigationType:(UIWebViewNavigationType)navigationType { NSString* scheme = [request.URL scheme]; - if ([scheme compare:@"js-frame"] != NSOrderedSame) { + NSAssert(scheme, @"scheme is nil: %@", request); + if (![scheme isEqualToString:@"js-frame"]) { return YES; } - NSString* resourceSpecifier = [request.URL resourceSpecifier]; - NSRange range = [resourceSpecifier rangeOfString:@":"]; - NSString* method; - NSString* message; - if (range.length == 0 && range.location == NSNotFound) { - method = resourceSpecifier; - } else { - method = [resourceSpecifier substringToIndex:range.location]; - message = [resourceSpecifier substringFromIndex:range.location + 1]; - } + dispatch_async(dispatch_get_main_queue(), ^(void) { - if ([method compare:@"onopen"] == NSOrderedSame) { + NSString* queuedMessage = [webView + stringByEvaluatingJavaScriptFromString:@"popQueuedMessage();"]; + NSAssert([queuedMessage length], @"Empty queued message from JS"); + + NSDictionary* queuedMessageDict = + [GAEChannelClient jsonStringToDictionary:queuedMessage]; + NSString* method = queuedMessageDict[@"type"]; + NSAssert(method, @"Missing method: %@", queuedMessageDict); + NSDictionary* payload = queuedMessageDict[@"payload"]; // May be nil. + + if ([method isEqualToString:@"onopen"]) { [self.delegate onOpen]; - } else if ([method compare:@"onmessage"] == NSOrderedSame) { - [self.delegate onMessage:message]; - } else if ([method compare:@"onclose"] == NSOrderedSame) { + } else if ([method isEqualToString:@"onmessage"]) { + NSDictionary* payloadData = + [GAEChannelClient jsonStringToDictionary:payload[@"data"]]; + [self.delegate onMessage:payloadData]; + } else if ([method isEqualToString:@"onclose"]) { [self.delegate onClose]; - } else if ([method compare:@"onerror"] == NSOrderedSame) { - // TODO(hughv): Get error. - int code = -1; - NSString* description = message; - [self.delegate onError:code withDescription:description]; + } else if ([method isEqualToString:@"onerror"]) { + NSNumber* codeNumber = payload[@"code"]; + int code = [codeNumber intValue]; + NSAssert([codeNumber isEqualToNumber:[NSNumber numberWithInt:code]], + @"Unexpected non-integral code: %@", payload); + [self.delegate onError:code withDescription:payload[@"description"]]; } else { - NSAssert( - NO, @"Invalid message sent from UIWebView: %@", resourceSpecifier); + NSAssert(NO, @"Invalid message sent from UIWebView: %@", queuedMessage); } }); - return YES; + return NO; } @end diff --git a/talk/examples/ios/AppRTCDemo/en.lproj/APPRTCViewController.xib b/talk/examples/ios/AppRTCDemo/en.lproj/APPRTCViewController.xib index 92d2adebff..62807fe1a0 100644 --- a/talk/examples/ios/AppRTCDemo/en.lproj/APPRTCViewController.xib +++ b/talk/examples/ios/AppRTCDemo/en.lproj/APPRTCViewController.xib @@ -178,7 +178,7 @@ - textField + roomInput @@ -186,7 +186,7 @@ - textInstructions + instructionsView @@ -194,7 +194,7 @@ - textOutput + logView @@ -660,25 +660,25 @@ UIViewController UIView - UITextField - UITextView - UITextView + UITextField + UITextView + UITextView blackView UIView - - textField + + roomInput UITextField - - textInstructions + + instructionsView UITextView - - textOutput + + logView UITextView diff --git a/talk/examples/ios/AppRTCDemo/ios_channel.html b/talk/examples/ios/AppRTCDemo/ios_channel.html index a55b8f48bc..86846dd687 100644 --- a/talk/examples/ios/AppRTCDemo/ios_channel.html +++ b/talk/examples/ios/AppRTCDemo/ios_channel.html @@ -6,10 +6,11 @@ Helper HTML that redirects Google AppEngine's Channel API to Objective C. This is done by hosting this page in an iOS application. The hosting class creates a UIWebView control and implements the UIWebViewDelegate - protocol. Then when there is a channel message, it is encoded in an IFRAME. - That IFRAME is added to the DOM which triggers a navigation event - |shouldStartLoadWithRequest| in Objective C which can then be routed in the - application as desired. + protocol. Then when there is a channel message it is queued in JS, + and an IFRAME is added to the DOM, triggering a navigation event + |shouldStartLoadWithRequest| in Objective C which can then fetch the + message using |popQueuedMessage|. This queuing is necessary to avoid URL + length limits in UIWebView (which are undocumented). -->