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

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

Index: webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m
diff --git a/webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m b/webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m
index 659973ff1804c280306221ff88a706dc9252f4b1..738da4700b6b0cbde0796ff0f2be94a8368f0c08 100644
--- a/webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m
+++ b/webrtc/sdk/objc/Framework/Classes/RTCCameraPreviewView.m
@@ -11,12 +11,14 @@
 #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];
@@ -34,6 +36,38 @@ - (void)setCaptureSession:(AVCaptureSession *)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 {