Add support for more resolutions on iOS/macOS

webrtc/sdk/objc/Framework/Classes/avfoundationvideocapturer.mm

 /*
  *  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.
  */
 
 #include "avfoundationvideocapturer.h"
 
 #import <AVFoundation/AVFoundation.h>
 #import <Foundation/Foundation.h>
 #if TARGET_OS_IPHONE
 #import <UIKit/UIKit.h>
 #endif
 
 #import "RTCDispatcher+Private.h"
 #import "WebRTC/RTCLogging.h"
 #if TARGET_OS_IPHONE
 #import "WebRTC/UIDevice+RTCDevice.h"
 #endif
 
 #include "webrtc/base/bind.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/common_video/include/corevideo_frame_buffer.h"
 
-// TODO(tkchin): support other formats.
-static NSString *const kDefaultPreset = AVCaptureSessionPreset640x480;
-static NSString *const kIPhone4SPreset = AVCaptureSessionPreset352x288;
-static cricket::VideoFormat const kDefaultFormat =
-    cricket::VideoFormat(640,
-                         480,
-                         cricket::VideoFormat::FpsToInterval(30),
-                         cricket::FOURCC_NV12);
-// iPhone4S is too slow to handle 30fps.
-static cricket::VideoFormat const kIPhone4SFormat =
-    cricket::VideoFormat(352,
-                         288,
-                         cricket::VideoFormat::FpsToInterval(15),
-                         cricket::FOURCC_NV12);
+struct AVCaptureSessionPresetResolution {

[tkchin_webrtc, 2016/08/11 16:51:52]

Some notes about this file:

ObjC++ is gnarly and should be avoided as much as possible except to interop
with WebRTC's video APIs.
This file has both ObjC and C++ impls because previously everything was in one
directory, and we wanted to hide the internal ObjC impl because no one should be
using it. This is now a relic because we have a public header directory now
(Framework/Headers/WebRTC). It would be better to split out
RTCAVFoundationVideoCapturerInternal into its own file in this Classes/ private
directory, and only use ObjC-isms there (ie use NSObject containers instead of
structs). webrtc::AVFoundationVideoCapturer can then be just the ObjC++ bridge
between ObjC and C++.

Essentially, in objc/ dirs, avoid C++ unless you absolutely have to use it,
otherwise we'll end up in a confusing ObjC++ tangle.

[kthelgason, 2016/08/11 17:42:04]

That's good to know, thanks. I'll revise this. Perhaps splitting the classes up
between files should be done in a separate CL?

+  NSString *sessionPreset;
+  int width;
+  int height;
+  cricket::VideoFormat getVideoFormat(int framerate) {
+    return cricket::VideoFormat(height, width,
+                                cricket::VideoFormat::FpsToInterval(framerate),
+                                cricket::FOURCC_NV12);
+  }
+};
+
+static const AVCaptureSessionPresetResolution kSupportedPresets[] = {

[tkchin_webrtc, 2016/08/11 16:51:52]

Not all of these is supported for every device. The presets need to be queried
at runtime. This file should be mac-compatible as well, so while we're adding
these, might as well add some of the rest.

kAvailablePresets?

[kthelgason, 2016/08/11 17:42:04]

Acknowledged.

+  { AVCaptureSessionPreset352x288, 352, 288},
+  { AVCaptureSessionPreset640x480, 640, 480},
+  { AVCaptureSessionPreset1280x720, 1280, 720}
+};
+
+static AVCaptureSessionPresetResolution const kDefaultPreset =
+  kSupportedPresets[1];
+static AVCaptureSessionPresetResolution const kIPhone4SPreset =
+  kSupportedPresets[0];
+
+// mapping from cricket::VideoFormat to AVCaptureSession presets
+static NSString *GetSessionPresetForVideoFormat(
+  const cricket::VideoFormat& format) {
+  for (const auto preset : kSupportedPresets) {
+    if (format.width == preset.width && format.height == preset.height) {
+      return preset.sessionPreset;
+    }
+  }
+  // Nothing found, use default
+  return kDefaultPreset.sessionPreset;
+}
 
 // This class used to capture frames using AVFoundation APIs on iOS. It is meant
 // to be owned by an instance of AVFoundationVideoCapturer. The reason for this
 // because other webrtc objects own cricket::VideoCapturer, which is not
 // ref counted. To prevent bad behavior we do not expose this class directly.
 @interface RTCAVFoundationVideoCapturerInternal : NSObject
     <AVCaptureVideoDataOutputSampleBufferDelegate>
 
 @property(nonatomic, readonly) AVCaptureSession *captureSession;
 @property(nonatomic, readonly) dispatch_queue_t frameQueue;
@@ skipping 300 matching lines @@
 #pragma mark - Private
 
 - (BOOL)setupCaptureSession {
   AVCaptureSession *captureSession = [[AVCaptureSession alloc] init];
 #if defined(__IPHONE_7_0) && __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_7_0
   NSString *version = [[UIDevice currentDevice] systemVersion];
   if ([version integerValue] >= 7) {
     captureSession.usesApplicationAudioSession = NO;
   }
 #endif
-  NSString *preset = kDefaultPreset;
+  NSString *preset = kDefaultPreset.sessionPreset;
 #if TARGET_OS_IPHONE
   if ([UIDevice deviceType] == RTCDeviceTypeIPhone4S) {
-    preset = kIPhone4SPreset;
+    preset = kIPhone4SPreset.sessionPreset;
   }
 #endif
   if (![captureSession canSetSessionPreset:preset]) {
     RTCLogError(@"Session preset unsupported.");
     return NO;
   }
   captureSession.sessionPreset = preset;
 
   // Add the output.
   AVCaptureVideoDataOutput *videoDataOutput = [self videoDataOutput];
@@ skipping 192 matching lines @@
 
 struct AVFoundationFrame {
   AVFoundationFrame(CVImageBufferRef buffer, int64_t time)
     : image_buffer(buffer), capture_time(time) {}
   CVImageBufferRef image_buffer;
   int64_t capture_time;
 };
 
 AVFoundationVideoCapturer::AVFoundationVideoCapturer()
     : _capturer(nil), _startThread(nullptr) {
-  // Set our supported formats. This matches preset.
+  // Set our supported formats. This matches kSupportedPresets
+  _capturer =
+    [[RTCAVFoundationVideoCapturerInternal alloc] initWithCapturer:this];
+
   std::vector<cricket::VideoFormat> supported_formats;
+  int framerate = 30;

[tkchin_webrtc, 2016/08/11 16:51:52]

There's no guarantee that you will get these frame rates. The presets will set a
default and later if you want to use something different you need to set it on
the device.

[kthelgason, 2016/08/11 17:42:04]

I understand. I'd pretty much come to that conclusion as well, but I kept it
like this as the previous code sets these framerates explicitly. Would you
suggest removing this always using 30 for the `cricket::VideoFormat`s?

+
 #if TARGET_OS_IPHONE
   if ([UIDevice deviceType] == RTCDeviceTypeIPhone4S) {
-    supported_formats.push_back(cricket::VideoFormat(kIPhone4SFormat));
     set_enable_video_adapter(false);
-  } else {
-    supported_formats.push_back(cricket::VideoFormat(kDefaultFormat));
+    framerate = 15;
   }
-#else
-  supported_formats.push_back(cricket::VideoFormat(kDefaultFormat));
 #endif
+
+  for (auto preset : kSupportedPresets) {
+    if ([_capturer.captureSession canSetSessionPreset:preset.sessionPreset]) {

[tkchin_webrtc, 2016/08/11 17:17:55]

I'm not sure we want to go down the preset route just yet for doing supported
formats, we probably should check what else the preset is setting. The usual way
of iterating formats is:
https://developer.apple.com/library/ios/documentation/AVFoundation/Reference/...

+      supported_formats.push_back(preset.getVideoFormat(framerate));
+    }
+  }
+
   SetSupportedFormats(supported_formats);
-  _capturer =
-      [[RTCAVFoundationVideoCapturerInternal alloc] initWithCapturer:this];
 }
 
 AVFoundationVideoCapturer::~AVFoundationVideoCapturer() {
   _capturer = nil;
 }
 
 cricket::CaptureState AVFoundationVideoCapturer::Start(
     const cricket::VideoFormat& format) {
   if (!_capturer) {
     LOG(LS_ERROR) << "Failed to create AVFoundation capturer.";
     return cricket::CaptureState::CS_FAILED;
   }
   if (_capturer.isRunning) {
     LOG(LS_ERROR) << "The capturer is already running.";
     return cricket::CaptureState::CS_FAILED;
   }
-  if (format != kDefaultFormat && format != kIPhone4SFormat) {
-    LOG(LS_ERROR) << "Unsupported format provided.";
+
+  NSString *desiredPreset = GetSessionPresetForVideoFormat(format);
+
+  [_capturer.captureSession beginConfiguration];
+  if (![_capturer.captureSession canSetSessionPreset:desiredPreset]) {
+    LOG(LS_ERROR) << "Unsupported video format.";

[tkchin_webrtc, 2016/08/11 16:51:52]

do you need beginconfiguration to check session preset? Calls to begin should be
matched with commit.

[kthelgason, 2016/08/11 17:42:04]

From Apple's documentation: "After calling beginConfiguration, you can for
example add or remove outputs, alter the sessionPreset, or configure individual
capture input or output properties". 

I will add a `commitConfiguration` call before the return in the failure case as
well.

     return cricket::CaptureState::CS_FAILED;
   }
+  _capturer.captureSession.sessionPreset = desiredPreset;
+  [_capturer.captureSession commitConfiguration];
 
   // Keep track of which thread capture started on. This is the thread that
   // frames need to be sent to.
   RTC_DCHECK(!_startThread);
   _startThread = rtc::Thread::Current();
 
   SetCaptureFormat(&format);
+
+

[tkchin_webrtc, 2016/08/11 16:51:52]

nit: remove blank line

[kthelgason, 2016/08/11 17:42:04]

Acknowledged.

   // This isn't super accurate because it takes a while for the AVCaptureSession
   // to spin up, and this call returns async.
   // TODO(tkchin): make this better.
   [_capturer start];
   SetCaptureState(cricket::CaptureState::CS_RUNNING);
 
   return cricket::CaptureState::CS_STARTING;
 }
 
 void AVFoundationVideoCapturer::Stop() {
@@ skipping 94 matching lines @@
   }
 
   OnFrame(cricket::WebRtcVideoFrame(buffer, webrtc::kVideoRotation_0,
                                     translated_camera_time_us, 0),
           captured_width, captured_height);
 
   CVBufferRelease(image_buffer);
 }
 
 }  // namespace webrtc