Issue 1542473003: Move Objective-C video renderers to webrtc/api/objc.

Issue 1542473003: Move Objective-C video renderers to webrtc/api/objc. (Closed)

Created:
5 years ago by hjon

Modified:
4 years, 11 months ago

Reviewers:
tkchin_webrtc

CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com

Base URL:
https://chromium.googlesource.com/external/webrtc.git@updateI420Frame

Target Ref:
refs/pending/heads/master

Project:
webrtc

Visibility:
Public.

More Reviews

Description

Move Objective-C video renderers to webrtc/api/objc. BUG= R=tkchin@webrtc.org Committed: https://crrev.com/e799badacc29a7567091f213a8b319f2581eff69 Patch from Jon Hjelle <hjon@andyet.net>;. Cr-Commit-Position: refs/heads/master@{#11209}

Patch Set 2 : Update style and fix build #

Total comments: 8

Patch Set 3 : Changes based on feedback #

Patch Set 5 : Merge master and fix atomic property reference #

Patch Set 6 : Merge master #

Created: 4 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+237 lines, -158 lines)			Patch
M	webrtc/api/BUILD.gn	View	1 2 3 4 5	3 chunks	+26 lines, -0 lines	0 comments	Download
M	webrtc/api/api.gyp	View	1 2 3 4 5	2 chunks	+27 lines, -0 lines	0 comments	Download
A +	webrtc/api/objc/RTCEAGLVideoView.h	View	1 2 3	1 chunk	+14 lines, -5 lines	0 comments	Download
A +	webrtc/api/objc/RTCEAGLVideoView.m	View	1 2	10 chunks	+32 lines, -50 lines	0 comments	Download
A +	webrtc/api/objc/RTCNSGLVideoView.h	View	1 2 3	1 chunk	+14 lines, -6 lines	0 comments	Download
A +	webrtc/api/objc/RTCNSGLVideoView.m	View	1 2 3 4	5 chunks	+26 lines, -43 lines	0 comments	Download
A	webrtc/api/objc/RTCOpenGLVideoRenderer.h	View	1 2	1 chunk	+58 lines, -0 lines	0 comments	Download
A +	webrtc/api/objc/RTCOpenGLVideoRenderer.mm	View	1 2	14 chunks	+30 lines, -48 lines	0 comments	Download
A +	webrtc/api/objc/RTCVideoRenderer.h	View	1 2 3	1 chunk	+10 lines, -6 lines	0 comments	Download

Messages

Total messages: 12 (3 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

hjon

Updated copyright and made some small style changes, but this is largely a simple move ...

5 years ago (2015-12-20 23:27:51 UTC) #2

tkchin_webrtc

On 2015/12/20 23:27:51, hjon wrote: > Updated copyright and made some small style changes, but ...

5 years ago (2015-12-21 12:46:55 UTC) #3

tkchin_webrtc

https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCOpenGLVideoRenderer.h File webrtc/api/objc/RTCOpenGLVideoRenderer.h (right): https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCOpenGLVideoRenderer.h#newcode33 webrtc/api/objc/RTCOpenGLVideoRenderer.h:33: - (instancetype)initWithContext:(EAGLContext*)context NS_DESIGNATED_INITIALIZER; update * https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCVideoRenderer.h File webrtc/api/objc/RTCVideoRenderer.h (right): ...

5 years ago (2015-12-21 12:57:50 UTC) #4

hjon

For context on my comments, these are the compiler errors I received: ../../../webrtc/api/objc/RTCOpenGLVideoRenderer.mm:352:20: error: comparison ...

5 years ago (2015-12-21 20:01:44 UTC) #5

For context on my comments, these are the compiler errors I received:

../../../webrtc/api/objc/RTCOpenGLVideoRenderer.mm:352:20: error: comparison of
integers of different signs: 'NSInteger' (aka 'int') and 'NSUInteger' (aka
'unsigned int') [-Werror,-Wsign-compare]
  if (frame.yPitch != frame.width || frame.uPitch != frame.chromaWidth ||
      ~~~~~~~~~~~~ ^  ~~~~~~~~~~~
../../../webrtc/api/objc/RTCOpenGLVideoRenderer.mm:352:51: error: comparison of
integers of different signs: 'NSInteger' (aka 'int') and 'NSUInteger' (aka
'unsigned int') [-Werror,-Wsign-compare]
  if (frame.yPitch != frame.width || frame.uPitch != frame.chromaWidth ||
                                     ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~
../../../webrtc/api/objc/RTCOpenGLVideoRenderer.mm:353:20: error: comparison of
integers of different signs: 'NSInteger' (aka 'int') and 'NSUInteger' (aka
'unsigned int') [-Werror,-Wsign-compare]
      frame.vPitch != frame.chromaWidth) {
      ~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~
../../../webrtc/api/objc/RTCOpenGLVideoRenderer.mm:377:14: error: comparison of
integers of different signs: 'NSInteger' (aka 'int') and 'NSUInteger' (aka
'unsigned int') [-Werror,-Wsign-compare]
  if (stride != width) {
      ~~~~~~ ^  ~~~~~

https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCOpenGLVide...
File webrtc/api/objc/RTCOpenGLVideoRenderer.h (right):

https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCOpenGLVide...
webrtc/api/objc/RTCOpenGLVideoRenderer.h:33: -
(instancetype)initWithContext:(EAGLContext*)context NS_DESIGNATED_INITIALIZER;
On 2015/12/21 12:57:49, tkchin_webrtc wrote:
> update *

Done.

https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCVideoRende...
File webrtc/api/objc/RTCVideoRenderer.h (right):

https://codereview.webrtc.org/1542473003/diff/1/webrtc/api/objc/RTCVideoRende...
webrtc/api/objc/RTCVideoRenderer.h:20: @protocol RTCVideoRenderer<NSObject>
On 2015/12/21 12:57:50, tkchin_webrtc wrote:
> space before <

Done.

https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCOpenGL...
File webrtc/api/objc/RTCOpenGLVideoRenderer.mm (right):

https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCOpenGL...
webrtc/api/objc/RTCOpenGLVideoRenderer.mm:354: (NSUInteger)frame.vPitch !=
frame.chromaWidth) {
Due to compiler warnings, I cast these to NSUInteger, but I don't know enough
about the context to know if something different should be done.

https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCOpenGL...
webrtc/api/objc/RTCOpenGLVideoRenderer.mm:378: if ((NSUInteger)stride != width)
{
Due to compiler warnings, I cast this to NSUInteger, but I don't know enough
about the context to know if something different should be done.

tkchin_webrtc

https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCEAGLVideoView.m File webrtc/api/objc/RTCEAGLVideoView.m (right): https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCEAGLVideoView.m#newcode198 webrtc/api/objc/RTCEAGLVideoView.m:198: [_glRenderer drawFrame:_i420Frame]; ditto self.i420Frame https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCEAGLVideoView.m#newcode213 webrtc/api/objc/RTCEAGLVideoView.m:213: _i420Frame = frame; ...

4 years, 11 months ago (2016-01-05 17:19:08 UTC) #6

hjon

https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCEAGLVideoView.m File webrtc/api/objc/RTCEAGLVideoView.m (right): https://codereview.webrtc.org/1542473003/diff/20001/webrtc/api/objc/RTCEAGLVideoView.m#newcode198 webrtc/api/objc/RTCEAGLVideoView.m:198: [_glRenderer drawFrame:_i420Frame]; On 2016/01/05 17:19:08, tkchin_webrtc wrote: > ditto ...

4 years, 11 months ago (2016-01-06 00:07:18 UTC) #7

tkchin_webrtc

https://codereview.webrtc.org/1542473003/diff/60001/webrtc/api/objc/RTCNSGLVideoView.m File webrtc/api/objc/RTCNSGLVideoView.m (right): https://codereview.webrtc.org/1542473003/diff/60001/webrtc/api/objc/RTCNSGLVideoView.m#newcode72 webrtc/api/objc/RTCNSGLVideoView.m:72: if (!_glRenderer) { ditto atomic required, so use property

4 years, 11 months ago (2016-01-11 21:19:36 UTC) #8

tkchin_webrtc

Committed patchset #6 (id:100001) manually as e799badacc29a7567091f213a8b319f2581eff69 (presubmit successful).

4 years, 11 months ago (2016-01-11 21:47:21 UTC) #10

commit-bot: I haz the power

4 years, 11 months ago (2016-01-11 21:47:24 UTC) #12

Message was sent while issue was closed.

Patchset 6 (id:??) landed as
https://crrev.com/e799badacc29a7567091f213a8b319f2581eff69
Cr-Commit-Position: refs/heads/master@{#11209}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages