Fixed that RTCCameraPreviewView did not rotate the video on device rotation.

webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m

 /*
  *  Copyright 2015 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 "WebRTC/RTCCameraPreviewView.h"
 
 #import <AVFoundation/AVFoundation.h>
+#import <UIKit/UIKit.h>
 
 #import "RTCDispatcher+Private.h"
 
 @implementation RTCCameraPreviewView
 
 @synthesize captureSession = _captureSession;
+@synthesize shouldAutoRotate = _shouldAutoRotate;
 
 + (Class)layerClass {
   return [AVCaptureVideoPreviewLayer class];
 }
 
 - (void)setCaptureSession:(AVCaptureSession *)captureSession {
   if (_captureSession == captureSession) {
     return;
   }
   _captureSession = captureSession;
   AVCaptureVideoPreviewLayer *previewLayer = [self previewLayer];
   [RTCDispatcher dispatchAsyncOnType:RTCDispatcherTypeCaptureSession
                                block:^{
     previewLayer.session = captureSession;
   }];
 }
 
+- (void) setShouldAutoRotate:(BOOL)shouldAutoRotate {

[daniela-webrtc, 2017/04/06 09:45:37]

Remove space.
Hint: use `git cl format before` submitting.

[tkchin_webrtc, 2017/04/06 17:39:22]

Is that what your team has decided on now? Is there a .clang-format somewhere?

[meetAkshay99, 2017/04/07 05:19:29]

I tried that but got following error:
YAML:8:11: error: unknown enumerated scalar
Language: ObjC
          ^~~~
Error reading /Users/akshayshah/WebRTC/src/.clang-format: Invalid argument

+    if (shouldAutoRotate) {

[daniela-webrtc, 2017/04/06 09:45:37]

You are not assigning the synthesized ivar _shouldAutoRotate anywhere in the
setter. Doesn't the compiler complain about this?

[meetAkshay99, 2017/04/06 10:24:34]

My bad. Yes thats required.

+        [[NSNotificationCenter defaultCenter] addObserver:self

[daniela-webrtc, 2017/04/06 09:45:37]

This seems like overhead - (Un)Registering for notifications etc inside the
view. It's also finicky and can cause observations leaks (you are not removing
the observer in dealloc for instance).
It's common for UIViews to perform this kind of updates in `layoutSubview` and
pass the responsibility for updating the layout upon orientation change to the
superviews and their hosting controllers.

[meetAkshay99, 2017/04/06 10:24:34]

That is what I was having earlier. A method was defined and the view controller
was responsible for calling that method to make sure the video orientation is
correct. However, Other reviewers commented that RTCCameraPreviewView should
handle it internally. Hence I went by this approach.
And yes, the observer should had been removed in dealloc too.

[daniela-webrtc, 2017/04/06 14:44:46]

I think the other reviewers meant that there is no reason to expose specific
method for this purpose other than the _shouldAutoRotate property and that
RTCCameraView should handle the rotations internally.
I agree with that. My suggestion was to check the device orientation in
`layoutSubviews`.
Basically: setCorrectVideoOrientation -> layoutSubviews.

[meetAkshay99, 2017/04/07 05:19:29]

Yeah, updating to call setCorrectVideoOrientation from layoutSubviews. I agree,
its a better solution than handling device orientation notifications.

+                                                 selector:@selector(deviceOrientationDidChange:)
+                                                     name:UIDeviceOrientationDidChangeNotification
+                                                   object:nil];
+        [[UIDevice currentDevice] beginGeneratingDeviceOrientationNotifications];
+    } else {
+        [[UIDevice currentDevice] endGeneratingDeviceOrientationNotifications];
+        [[NSNotificationCenter defaultCenter] removeObserver:self
+                                                    name:UIDeviceOrientationDidChangeNotification
+                                                      object:nil];
+    }
+}
+
+- (void)deviceOrientationDidChange:(NSNotification *)notification {
+    // Get current device orientation.
+    UIDeviceOrientation deviceOrientation = [UIDevice currentDevice].orientation;
+    AVCaptureVideoPreviewLayer *previewLayer = [self previewLayer];
+
+    // Set the video orientation based on device orientation.
+    if (deviceOrientation == UIInterfaceOrientationPortraitUpsideDown) {

[Chuck, 2017/04/06 19:06:46]

This header docs say you should only set this property if
isVideoOrientationSupported is YES.

"This property is only applicable to AVCaptureConnection instances involving
video. In such connections, the videoOrientation property may only be set if
-isVideoOrientationSupported returns YES."

[meetAkshay99, 2017/04/07 05:19:29]

Added a check for the same.

+        [previewLayer.connection setVideoOrientation:AVCaptureVideoOrientationPortraitUpsideDown];

[Chuck, 2017/04/06 19:06:46]

Please use property syntax here:
previewLayer.connection.videoOrientation = 

[meetAkshay99, 2017/04/07 05:19:29]

I would update it to property syntax, but would like to know - is there a
specific reason for doing so?

[Chuck, 2017/04/07 12:57:40]

It's modern objective-c syntax and to match the style of the rest of the code.

+    } else if (deviceOrientation == UIInterfaceOrientationPortrait) {
+        [previewLayer.connection setVideoOrientation:AVCaptureVideoOrientationPortrait];
+    } else if (deviceOrientation == UIInterfaceOrientationLandscapeLeft) {
+        [previewLayer.connection setVideoOrientation:AVCaptureVideoOrientationLandscapeLeft];
+    } else {
+        [previewLayer.connection setVideoOrientation:AVCaptureVideoOrientationLandscapeRight];
+    }
+}
+
 #pragma mark - Private
 
 - (AVCaptureVideoPreviewLayer *)previewLayer {
   return (AVCaptureVideoPreviewLayer *)self.layer;
 }
 
 @end