Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(864)

Unified Diff: webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m

Issue 2812613003: ObjC: Remove RTCOpenGLVideoRenderer (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m
diff --git a/webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m b/webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m
index 537397f37866484a02d2af09a6083934353ca106..b9b7e1d019b489f89a3997ed2106724fb325ea12 100644
--- a/webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m
+++ b/webrtc/sdk/objc/Framework/Classes/RTCEAGLVideoView.m
@@ -12,7 +12,7 @@
#import <GLKit/GLKit.h>
-#import "RTCOpenGLVideoRenderer.h"
+#import "RTCShader+Private.h"
#import "WebRTC/RTCVideoFrame.h"
// RTCDisplayLinkTimer wraps a CADisplayLink and is set to fire every two screen
@@ -88,7 +88,6 @@
// from the display link callback so atomicity is required.
@property(atomic, strong) RTCVideoFrame *videoFrame;
@property(nonatomic, readonly) GLKView *glkView;
-@property(nonatomic, readonly) RTCOpenGLVideoRenderer *glRenderer;
@end
@implementation RTCEAGLVideoView {
@@ -97,12 +96,16 @@
// This flag should only be set and read on the main thread (e.g. by
// setNeedsDisplay)
BOOL _isDirty;
+
+ BOOL _isInitialized;
sakal 2017/04/11 11:00:21 Why is this only needed on iOS and not on Mac?
magjed_webrtc 2017/04/11 12:05:43 It was obvious from the code that it wasn't necess
+ id<RTCShader> _i420Shader;
+ id<RTCShader> _nv12Shader;
+ RTCVideoFrame *_lastDrawnFrame;
}
@synthesize delegate = _delegate;
@synthesize videoFrame = _videoFrame;
@synthesize glkView = _glkView;
-@synthesize glRenderer = _glRenderer;
- (instancetype)initWithFrame:(CGRect)frame {
if (self = [super initWithFrame:frame]) {
@@ -125,7 +128,6 @@
glContext = [[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2];
}
_glContext = glContext;
- _glRenderer = [[RTCOpenGLVideoRenderer alloc] initWithContext:_glContext];
// GLKView manages a framebuffer for us.
_glkView = [[GLKView alloc] initWithFrame:CGRectZero
@@ -200,7 +202,27 @@
- (void)glkView:(GLKView *)view drawInRect:(CGRect)rect {
// The renderer will draw the frame to the framebuffer corresponding to the
// one used by |view|.
- [_glRenderer drawFrame:self.videoFrame];
+ RTCVideoFrame *frame = self.videoFrame;
+ if (!_isInitialized || !frame || frame == _lastDrawnFrame) {
sakal 2017/04/11 11:00:21 Is this something we expect to happen or should we
magjed_webrtc 2017/04/11 12:05:43 Yes, it can happen that we try to render the same
+ return;
+ }
+ [self ensureGLContext];
+ glClear(GL_COLOR_BUFFER_BIT);
+ id<RTCShader> shader = nil;
+ if (frame.nativeHandle) {
+ if (!_nv12Shader) {
+ _nv12Shader = [[RTCNativeNV12Shader alloc] initWithContext:_glContext];
+ }
+ shader = _nv12Shader;
+ } else {
+ if (!_i420Shader) {
+ _i420Shader = [[RTCI420Shader alloc] initWithContext:_glContext];
+ }
+ shader = _i420Shader;
+ }
+ if (shader && [shader drawFrame:frame]) {
sakal 2017/04/11 11:00:21 Log on failure?
magjed_webrtc 2017/04/11 12:05:43 We didn't do it previously, but I guess we can add
+ _lastDrawnFrame = frame;
+ }
}
#pragma mark - RTCVideoRenderer
@@ -223,7 +245,7 @@
- (void)displayLinkTimerDidFire {
// Don't render unless video frame have changed or the view content
// has explicitly been marked dirty.
- if (!_isDirty && _glRenderer.lastDrawnFrame == self.videoFrame) {
+ if (!_isDirty && _lastDrawnFrame == self.videoFrame) {
return;
}
@@ -242,7 +264,10 @@
- (void)setupGL {
self.videoFrame = nil;
- [_glRenderer setupGL];
+ if (!_isInitialized) {
sakal 2017/04/11 11:00:21 I don't like the way isInitialized is used. It see
magjed_webrtc 2017/04/11 12:05:43 It looks like we don't need it at all.
+ [self ensureGLContext];
+ _isInitialized = YES;
+ }
_timer.isPaused = NO;
}
@@ -250,7 +275,12 @@
self.videoFrame = nil;
_timer.isPaused = YES;
[_glkView deleteDrawable];
- [_glRenderer teardownGL];
+ if (_isInitialized) {
+ [self ensureGLContext];
+ _i420Shader = nil;
+ _nv12Shader = nil;
+ _isInitialized = NO;
+ }
}
- (void)didBecomeActive {
@@ -261,4 +291,11 @@
[self teardownGL];
}
+- (void)ensureGLContext {
+ NSAssert(_glContext, @"context shouldn't be nil");
+ if ([EAGLContext currentContext] != _glContext) {
+ [EAGLContext setCurrentContext:_glContext];
+ }
+}
+
@end

Powered by Google App Engine
This is Rietveld 408576698