Add loopback option and improve UX of AppRTCMobile for Mac.

webrtc/examples/objc/AppRTCMobile/mac/APPRTCViewController.m

 /*
  *  Copyright 2014 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #import "APPRTCViewController.h"
 
 #import <AVFoundation/AVFoundation.h>
 
 #import "WebRTC/RTCNSGLVideoView.h"
 #import "WebRTC/RTCVideoTrack.h"
 
 #import "ARDAppClient.h"
 
-static NSUInteger const kContentWidth = 1280;
-static NSUInteger const kContentHeight = 720;
-static NSUInteger const kRoomFieldWidth = 80;
-static NSUInteger const kLogViewHeight = 280;
-static NSUInteger const kPreviewWidth = 490;
+static NSUInteger const kContentWidth = 900;
+static NSUInteger const kRoomFieldWidth = 200;
+static NSUInteger const kActionItemHeight = 30;
+static NSUInteger const kBottomViewHeight = 200;
 
 @class APPRTCMainView;
 @protocol APPRTCMainViewDelegate
 
 - (void)appRTCMainView:(APPRTCMainView*)mainView

[tkchin_webrtc, 2016/10/19 00:57:17]

at some point we should change APPRTC->ARD or whatever new 3 letter prefix we
think is good for AppRTCMobile.
(ARD was selected for AppRTCDemo)

-        didEnterRoomId:(NSString*)roomId;
+        didEnterRoomId:(NSString*)roomId
+              loopback:(BOOL)isLoopback;
 
 @end
 
 @interface APPRTCMainView : NSView
 
 @property(nonatomic, weak) id<APPRTCMainViewDelegate> delegate;
 @property(nonatomic, readonly) RTCNSGLVideoView* localVideoView;
 @property(nonatomic, readonly) RTCNSGLVideoView* remoteVideoView;
 
 - (void)displayLogMessage:(NSString*)message;
 
 @end
 
 @interface APPRTCMainView () <NSTextFieldDelegate, RTCNSGLVideoViewDelegate>
 @end
 @implementation APPRTCMainView  {
   NSScrollView* _scrollView;
-  NSTextField* _roomLabel;
+  NSView* _actionItemsView;
+  NSButton* _connectButton;
+  NSButton* _loopbackButton;
   NSTextField* _roomField;
   NSTextView* _logView;
   CGSize _localVideoSize;
   CGSize _remoteVideoSize;
 }
 
 @synthesize delegate = _delegate;
 @synthesize localVideoView = _localVideoView;
 @synthesize remoteVideoView = _remoteVideoView;
 
-+ (BOOL)requiresConstraintBasedLayout {
-  return YES;
+
+- (void)displayLogMessage:(NSString*)message {

[tkchin_webrtc, 2016/10/19 00:57:17]

NSString *)

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+  _logView.string =
+  [NSString stringWithFormat:@"%@%@\n", _logView.string, message];

[magjed_webrtc, 2016/10/17 15:22:41]

nit: This line should be indented with 4 spaces.

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+  NSRange range = NSMakeRange([_logView.string length], 0);

[tkchin_webrtc, 2016/10/19 00:57:17]

nit: dot syntax for properties

(when using dot syntax, the implicit assumption is that it is a fast method.
Using the message usually means that the operation could take a while)

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+  [_logView scrollRangeToVisible:range];
 }
 
+#pragma mark - Internal

[tkchin_webrtc, 2016/10/19 00:57:17]

we've usually called it - Private
there are a bunch of legacy +Internal files still, but should really be named
+Private

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+
 - (instancetype)initWithFrame:(NSRect)frame {
   if (self = [super initWithFrame:frame]) {
     [self setupViews];
   }
   return self;
 }
 
++ (BOOL)requiresConstraintBasedLayout {
+  return YES;
+}
+
 - (void)updateConstraints {
   NSParameterAssert(

[tkchin_webrtc, 2016/10/19 00:57:17]

consider filing a bug to replace NSParameterAssert with a custom assert macro
that is more useful (e.g. that will break properly in debugger and provide
useful callstack)

[daniela-webrtc, 2016/10/19 14:45:13]

https://bugs.chromium.org/p/webrtc/issues/detail?id=6561

-      _roomField != nil && _scrollView != nil && _remoteVideoView != nil);
+      _roomField != nil &&
+      _scrollView != nil &&
+      _remoteVideoView != nil &&
+      _localVideoView != nil &&
+      _actionItemsView!= nil &&
+      _connectButton != nil &&
+      _loopbackButton != nil);
+
   [self removeConstraints:[self constraints]];
   NSDictionary* viewsDictionary =
-      NSDictionaryOfVariableBindings(_roomLabel,
-                                     _roomField,
+      NSDictionaryOfVariableBindings(_roomField,
                                      _scrollView,
                                      _remoteVideoView,
-                                     _localVideoView);
+                                     _localVideoView,
+                                     _actionItemsView,
+                                     _connectButton,
+                                     _loopbackButton);
 
   NSSize remoteViewSize = [self remoteVideoViewSize];
   NSDictionary* metrics = @{
-    @"kLogViewHeight" : @(kLogViewHeight),
-    @"kPreviewWidth" : @(kPreviewWidth),
-    @"kRoomFieldWidth" : @(kRoomFieldWidth),
     @"remoteViewWidth" : @(remoteViewSize.width),
     @"remoteViewHeight" : @(remoteViewSize.height),
-    @"localViewHeight" : @(remoteViewSize.height),
-    @"scrollViewWidth" : @(kContentWidth - kPreviewWidth),
+    @"kBottomViewHeight" : @(kBottomViewHeight),
+    @"localViewHeight" : @(remoteViewSize.height/3),

[tkchin_webrtc, 2016/10/19 00:57:17]

nit .height / 3

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+    @"localViewWidth" : @(remoteViewSize.width/3),
+    @"kRoomFieldWidth" : @(kRoomFieldWidth),
+    @"kActionItemHeight" : @(kActionItemHeight)
   };
   // Declare this separately to avoid compiler warning about splitting string
   // within an NSArray expression.
-  NSString* verticalConstraint =
-      @"V:|-[_roomLabel]-[_roomField]-[_scrollView(kLogViewHeight)]"
-       "-[_remoteVideoView(remoteViewHeight)]-|";
+  NSString* verticalConstraintLeft =

[tkchin_webrtc, 2016/10/19 00:57:17]

NSString *vertical

ditto for rest of file. I know this was what it used to be in this file, but
we've since figured out our style story and this never got updated

[daniela-webrtc, 2016/10/19 14:45:13]

Bug filled 
https://bugs.chromium.org/p/webrtc/issues/detail?id=6561

+      @"V:|-[_remoteVideoView(remoteViewHeight)]-[_scrollView(kBottomViewHeight)]-|";
+  NSString* verticalConstraintRight =
+      @"V:|-[_remoteVideoView(remoteViewHeight)]-[_actionItemsView(kBottomViewHeight)]-|";
   NSArray* constraintFormats = @[
-      verticalConstraint,
-      @"V:[_localVideoView]-[_remoteVideoView]",
-      @"V:[_localVideoView(kLogViewHeight)]",
-      @"|-[_roomLabel]",
-      @"|-[_roomField(kRoomFieldWidth)]",
-      @"|-[_scrollView(scrollViewWidth)]",
-      @"[_scrollView]-[_localVideoView]",
-      @"|-[_remoteVideoView(remoteViewWidth)]-|",
-      @"[_localVideoView(kPreviewWidth)]-|",
+      verticalConstraintLeft,
+      verticalConstraintRight,
+      @"H:|-[_remoteVideoView(remoteViewWidth)]-|",

[tkchin_webrtc, 2016/10/19 00:57:17]

not looking too closely at these, if they work then all good
consider using anchors instead though, and programmatically getting constraints
by using constraintEqualToAnchor

+      @"V:|-[_localVideoView(localViewHeight)]",
+      @"H:|-[_localVideoView(localViewWidth)]",
+      @"H:|-[_scrollView(==_actionItemsView)]-[_actionItemsView]-|"
   ];
-  for (NSString* constraintFormat in constraintFormats) {
-    NSArray* constraints =
-        [NSLayoutConstraint constraintsWithVisualFormat:constraintFormat
-                                                options:0
-                                                metrics:metrics
-                                                  views:viewsDictionary];
-    for (NSLayoutConstraint* constraint in constraints) {
-      [self addConstraint:constraint];
-    }
-  }
+
+  NSArray* actionItemsConstraints = @[
+      @"H:|-[_roomField(kRoomFieldWidth)]-[_loopbackButton(kRoomFieldWidth)]",
+      @"H:|-[_connectButton(kRoomFieldWidth)]",
+      @"V:|-[_roomField(kActionItemHeight)]-[_connectButton(kActionItemHeight)]",
+      @"V:|-[_loopbackButton(kActionItemHeight)]",
+      ];
+
+  [APPRTCMainView addConstraints:constraintFormats
+                          toView:self
+                 viewsDictionary:viewsDictionary
+                         metrics:metrics];
+  [APPRTCMainView addConstraints:actionItemsConstraints
+                          toView:_actionItemsView
+                 viewsDictionary:viewsDictionary
+                         metrics:metrics];
   [super updateConstraints];
 }
 
-- (void)displayLogMessage:(NSString*)message {
-  _logView.string =
-      [NSString stringWithFormat:@"%@%@\n", _logView.string, message];
-  NSRange range = NSMakeRange([_logView.string length], 0);
-  [_logView scrollRangeToVisible:range];
-}
+#pragma mark - Constraints helper
 
-#pragma mark - NSControl delegate
-
-- (void)controlTextDidEndEditing:(NSNotification*)notification {
-  NSDictionary* userInfo = [notification userInfo];
-  NSInteger textMovement = [userInfo[@"NSTextMovement"] intValue];
-  if (textMovement == NSReturnTextMovement) {
-    [self.delegate appRTCMainView:self didEnterRoomId:_roomField.stringValue];
++ (void)addConstraints:(NSArray*)constraints toView:(NSView*)view
+       viewsDictionary:(NSDictionary*)viewsDictionary
+               metrics:(NSDictionary*)metrics{

[kthelgason, 2016/10/17 09:30:09]

nit: missing space

+  for (NSString* constraintFormat in constraints) {
+    NSArray* constraints =
+    [NSLayoutConstraint constraintsWithVisualFormat:constraintFormat
+                                            options:0
+                                            metrics:metrics
+                                              views:viewsDictionary];
+    for (NSLayoutConstraint* constraint in constraints) {
+      [view addConstraint:constraint];
+    }
   }
 }
 
+#pragma mark - Control actions
+
+- (void)startCall:(id)sender {
+

[tkchin_webrtc, 2016/10/19 00:57:17]

nit: remove blank line

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+  NSString* roomString = _roomField.stringValue;
+  //generate room id for loopback options

[magjed_webrtc, 2016/10/17 15:22:41]

nit: You should format your comment like:
// Generate room id for loopback options.

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+  if (_loopbackButton.intValue && [roomString isEqualToString:@""]) {
+    roomString = [[NSUUID UUID] UUIDString];

[tkchin_webrtc, 2016/10/19 00:57:16]

ditto dot syntax for properties

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+    roomString = [roomString stringByReplacingOccurrencesOfString:@"-" withString:@""];
+  }
+
+  [self.delegate appRTCMainView:self
+                 didEnterRoomId:roomString
+                       loopback:_loopbackButton.intValue];
+}
+
 #pragma mark - RTCNSGLVideoViewDelegate
 
 - (void)videoView:(RTCNSGLVideoView*)videoView
     didChangeVideoSize:(NSSize)size {
   if (videoView == _remoteVideoView) {
     _remoteVideoSize = size;
   } else if (videoView == _localVideoView) {
     _localVideoSize = size;
   } else {
     return;
   }
+
   [self setNeedsUpdateConstraints:YES];
 }
 
 #pragma mark - Private
 
 - (void)setupViews {
   NSParameterAssert([[self subviews] count] == 0);
 
-  _roomLabel = [[NSTextField alloc] initWithFrame:NSZeroRect];
-  [_roomLabel setTranslatesAutoresizingMaskIntoConstraints:NO];
-  [_roomLabel setBezeled:NO];
-  [_roomLabel setDrawsBackground:NO];
-  [_roomLabel setEditable:NO];
-  [_roomLabel setStringValue:@"Enter AppRTC room id:"];
-  [self addSubview:_roomLabel];
-
-  _roomField = [[NSTextField alloc] initWithFrame:NSZeroRect];
-  [_roomField setTranslatesAutoresizingMaskIntoConstraints:NO];
-  [self addSubview:_roomField];
-  [_roomField setEditable:YES];
-  [_roomField setDelegate:self];
-
   _logView = [[NSTextView alloc] initWithFrame:NSZeroRect];
-  [_logView setMinSize:NSMakeSize(0, kLogViewHeight)];
+  [_logView setMinSize:NSMakeSize(0, kBottomViewHeight)];
   [_logView setMaxSize:NSMakeSize(FLT_MAX, FLT_MAX)];
   [_logView setVerticallyResizable:YES];
   [_logView setAutoresizingMask:NSViewWidthSizable];
   NSTextContainer* textContainer = [_logView textContainer];
   NSSize containerSize = NSMakeSize(kContentWidth, FLT_MAX);
   [textContainer setContainerSize:containerSize];
   [textContainer setWidthTracksTextView:YES];
   [_logView setEditable:NO];
 
+  [self setupActionItemsView];
+
   _scrollView = [[NSScrollView alloc] initWithFrame:NSZeroRect];
   [_scrollView setTranslatesAutoresizingMaskIntoConstraints:NO];
   [_scrollView setHasVerticalScroller:YES];
   [_scrollView setDocumentView:_logView];
   [self addSubview:_scrollView];
 
   NSOpenGLPixelFormatAttribute attributes[] = {
     NSOpenGLPFADoubleBuffer,
     NSOpenGLPFADepthSize, 24,
     NSOpenGLPFAOpenGLProfile,
     NSOpenGLProfileVersion3_2Core,
     0
   };
   NSOpenGLPixelFormat* pixelFormat =
       [[NSOpenGLPixelFormat alloc] initWithAttributes:attributes];
   _remoteVideoView = [[RTCNSGLVideoView alloc] initWithFrame:NSZeroRect
                                                  pixelFormat:pixelFormat];
   [_remoteVideoView setTranslatesAutoresizingMaskIntoConstraints:NO];
   _remoteVideoView.delegate = self;
   [self addSubview:_remoteVideoView];
 
   _localVideoView = [[RTCNSGLVideoView alloc] initWithFrame:NSZeroRect
                                                  pixelFormat:pixelFormat];
   [_localVideoView setTranslatesAutoresizingMaskIntoConstraints:NO];
   _localVideoView.delegate = self;
   [self addSubview:_localVideoView];
 }
 
+- (void)setupActionItemsView {
+  _actionItemsView = [[NSView alloc] initWithFrame:NSZeroRect];
+  [_actionItemsView setTranslatesAutoresizingMaskIntoConstraints:NO];
+  [self addSubview:_actionItemsView];
+
+  _roomField = [[NSTextField alloc] initWithFrame:NSZeroRect];
+  [_roomField setTranslatesAutoresizingMaskIntoConstraints:NO];
+  _roomField.placeholderString = @"Enter AppRTC room id";
+  [_actionItemsView addSubview:_roomField];
+  [_roomField setEditable:YES];
+
+  _connectButton = [[NSButton alloc] initWithFrame:NSZeroRect];
+  [_connectButton setTranslatesAutoresizingMaskIntoConstraints:NO];
+  _connectButton.title = @"Start call";
+  _connectButton.bezelStyle = NSRoundedBezelStyle;
+  _connectButton.target = self;
+  _connectButton.action = @selector(startCall:);
+  [_actionItemsView addSubview:_connectButton];
+
+  _loopbackButton = [[NSButton alloc] initWithFrame:NSZeroRect];
+  [_loopbackButton setTranslatesAutoresizingMaskIntoConstraints:NO];
+  _loopbackButton.title = @"Loopback";
+  [_loopbackButton setButtonType:NSSwitchButton];
+  [_actionItemsView addSubview:_loopbackButton];
+}
+
 - (NSSize)remoteVideoViewSize {
-  NSInteger width = MAX(_remoteVideoSize.width, kContentWidth);
+  if (!_remoteVideoView.bounds.size.width) {
+    return NSMakeSize(kContentWidth, 0);
+  }
+  NSInteger width = MAX(_remoteVideoView.bounds.size.width, kContentWidth);
   NSInteger height = (width/16) * 9;
   return NSMakeSize(width, height);
 }
 
 @end
 
 @interface APPRTCViewController ()
     <ARDAppClientDelegate, APPRTCMainViewDelegate>
 @property(nonatomic, readonly) APPRTCMainView* mainView;
 @end
 
 @implementation APPRTCViewController {
   ARDAppClient* _client;
   RTCVideoTrack* _localVideoTrack;
   RTCVideoTrack* _remoteVideoTrack;
 }
 
 - (void)dealloc {
   [self disconnect];
 }
 
+- (void)viewDidAppear {
+  [super viewDidAppear];
+  [self displayUsageInstructions];
+}
+
 - (void)loadView {
   APPRTCMainView* view = [[APPRTCMainView alloc] initWithFrame:NSZeroRect];
   [view setTranslatesAutoresizingMaskIntoConstraints:NO];
   view.delegate = self;
   self.view = view;
 }
 
 - (void)windowWillClose:(NSNotification*)notification {
   [self disconnect];
 }
 
+#pragma mark - Usage
+
+- (void)displayUsageInstructions {
+  [self.mainView displayLogMessage:

[tkchin_webrtc, 2016/10/19 00:57:17]

might be worth just having this as a label somewhere on the UI, up to you

+   @"To start call:\n"
+   @"• Enter AppRTC room id\n"
+   @"• Loopback with or without AppRTC room id"];

[magjed_webrtc, 2016/10/17 15:22:41]

nit: I found this instruction slightly confusing. Can you change it to something
like:
• Enter AppRTC room id (not necessary for loopback)
• Start call

[daniela-webrtc, 2016/10/19 14:45:13]

Done.

+}
+
 #pragma mark - ARDAppClientDelegate
 
 - (void)appClient:(ARDAppClient *)client
     didChangeState:(ARDAppClientState)state {
   switch (state) {
     case kARDAppClientStateConnected:
-      NSLog(@"Client connected.");
+      [self.mainView displayLogMessage:@"Client connected."];
       break;
     case kARDAppClientStateConnecting:
-      NSLog(@"Client connecting.");
+      [self.mainView displayLogMessage:@"Client connecting."];
       break;
     case kARDAppClientStateDisconnected:
-      NSLog(@"Client disconnected.");
+      [self.mainView displayLogMessage:@"Client disconnected."];
       [self resetUI];
       _client = nil;
       break;
   }
 }
 
 - (void)appClient:(ARDAppClient *)client
     didChangeConnectionState:(RTCIceConnectionState)state {
 }
 
@@ skipping 15 matching lines @@
   [self disconnect];
 }
 
 - (void)appClient:(ARDAppClient *)client
       didGetStats:(NSArray *)stats {
 }
 
 #pragma mark - APPRTCMainViewDelegate
 
 - (void)appRTCMainView:(APPRTCMainView*)mainView
-        didEnterRoomId:(NSString*)roomId {
+        didEnterRoomId:(NSString*)roomId
+              loopback:(BOOL)isLoopback {
+
+  if ([roomId isEqualToString:@""]) {
+    [self.mainView displayLogMessage:@"Missing room id"];
+    return;
+  }
+
   [_client disconnect];
   ARDAppClient *client = [[ARDAppClient alloc] initWithDelegate:self];
   [client connectToRoomWithId:roomId
-                   isLoopback:NO
+                   isLoopback:isLoopback
                   isAudioOnly:NO
             shouldMakeAecDump:NO
         shouldUseLevelControl:NO];
   _client = client;
 }
 
 #pragma mark - Private
 
 - (APPRTCMainView*)mainView {
   return (APPRTCMainView*)self.view;
@@ skipping 13 matching lines @@
   [self.mainView.remoteVideoView renderFrame:nil];
   [self.mainView.localVideoView renderFrame:nil];
 }
 
 - (void)disconnect {
   [self resetUI];
   [_client disconnect];
 }
 
 @end