Add CoreVideoFrameBuffer.

Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=

Committed: https://crrev.com/7d06a8cfe420772aa0bb5658f7b28396ab9b1e26
Cr-Commit-Position: refs/heads/master@{#12224}

[tkchin_webrtc, 2016-04-01 00:37:33]

Patchset #1 (id:1) has been deleted

[tkchin_webrtc, 2016-04-01 00:38:07]

Description was changed from

==========
Add CoreVideoFrameBuffer.

- Makes h264 decoder output CoreVideoFrameBuffer
- Makes renderer convert frame buffer if it is not i420

BUG=
==========

to

==========
Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=
==========

[tkchin_webrtc, 2016-04-01 00:38:07]

tkchin@webrtc.org changed reviewers:
+ haysc@webrtc.org, pbos@webrtc.org

[tkchin_webrtc, 2016-04-01 00:39:20]

Patchset #1 (id:20001) has been deleted

[tkchin_webrtc, 2016-04-01 00:52:57]

Patchset #1 (id:40001) has been deleted

[tkchin_webrtc, 2016-04-01 01:01:41]

Patchset #1 (id:60001) has been deleted

[tkchin_webrtc, 2016-04-01 01:01:45]

Patchset #1 (id:80001) has been deleted

[tkchin_webrtc, 2016-04-01 01:37:50]

Patchset #2 (id:120001) has been deleted

[tkchin_webrtc, 2016-04-01 01:40:57]

Patchset #2 (id:140001) has been deleted

[tkchin_webrtc, 2016-04-01 01:45:07]

Patchset #2 (id:160001) has been deleted

[tkchin_webrtc, 2016-04-01 02:10:30]

Patchset #3 (id:200001) has been deleted

[pbos-webrtc, 2016-04-01 11:56:11]

looks good overall, I think they shouldn't be ifdeffed into existing files
though

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoFrame.mm (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoFrame.mm:43: if (self.i420Buffer) {
Prefer return nullptr on !self.i420buffer, here and below, but up to you

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:166: #if defined(WEBRTC_MAC) ||
defined(WEBRTC_IOS)
This class should go into a separate header I think.

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/vide...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/vide...
webrtc/common_video/video_frame_buffer.cc:270: #if defined(WEBRTC_MAC) ||
defined(WEBRTC_IOS)
I want this in a separate file that's only compiled on mac/ios.

[tkchin_webrtc, 2016-04-01 16:09:29]

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/api/objc/RTCVideo...
File webrtc/api/objc/RTCVideoFrame.mm (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/api/objc/RTCVideo...
webrtc/api/objc/RTCVideoFrame.mm:43: if (self.i420Buffer) {
On 2016/04/01 11:56:10, pbos-webrtc wrote:
> Prefer return nullptr on !self.i420buffer, here and below, but up to you

Done.

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/incl...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/incl...
webrtc/common_video/include/video_frame_buffer.h:166: #if defined(WEBRTC_MAC) ||
defined(WEBRTC_IOS)
On 2016/04/01 11:56:10, pbos-webrtc wrote:
> This class should go into a separate header I think.

Done.

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/vide...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/1853503003/diff/220001/webrtc/common_video/vide...
webrtc/common_video/video_frame_buffer.cc:270: #if defined(WEBRTC_MAC) ||
defined(WEBRTC_IOS)
On 2016/04/01 11:56:10, pbos-webrtc wrote:
> I want this in a separate file that's only compiled on mac/ios.

Done.

[pbos-webrtc, 2016-04-04 11:15:46]

pbos@webrtc.org changed reviewers:
+ magjed@webrtc.org

[pbos-webrtc, 2016-04-04 11:15:46]

magjed@: Where do you think platform-specific instances of VideoFrame should go?
(What folder?)

Does this apply similarly to Android in any way that this should be aligned
with?

[magjed_webrtc, 2016-04-04 14:02:43]

On 2016/04/04 11:15:46, pbos-webrtc wrote:
> magjed@: Where do you think platform-specific instances of VideoFrame should
go?
> (What folder?)
> 
> Does this apply similarly to Android in any way that this should be aligned
> with?

I think it's ok if the file is placed in webrtc/common_video/. The Android
version is placed in webrtc/api/java/jni/native_handle_impl.h/cc.

My only concern is that the name core_video_frame_buffer.h might be confusing. I
understand that 'core' is for CoreVideo, but maybe the name or path should
include something with mac or osx.

[magjed_webrtc, 2016-04-04 14:03:30]

Cool, lgtm.

https://codereview.webrtc.org/1853503003/diff/240001/webrtc/common_video/incl...
File webrtc/common_video/include/core_video_frame_buffer.h (right):

https://codereview.webrtc.org/1853503003/diff/240001/webrtc/common_video/incl...
webrtc/common_video/include/core_video_frame_buffer.h:23: virtual
~CoreVideoFrameBuffer();
nit: Override instead of virtual?

[pbos-webrtc, 2016-04-04 14:06:34]

maybe call the filename "corevideo_frame_buffer" for readability

[pbos-webrtc, 2016-04-04 14:06:43]

lgtm as well

[tkchin_webrtc, 2016-04-04 19:12:51]

corevideo_frame_buffer makes sense. Done.

Thanks!

https://codereview.webrtc.org/1853503003/diff/240001/webrtc/common_video/incl...
File webrtc/common_video/include/core_video_frame_buffer.h (right):

https://codereview.webrtc.org/1853503003/diff/240001/webrtc/common_video/incl...
webrtc/common_video/include/core_video_frame_buffer.h:23: virtual
~CoreVideoFrameBuffer();
On 2016/04/04 14:03:30, magjed_webrtc wrote:
> nit: Override instead of virtual?

Done.

[tkchin_webrtc, 2016-04-04 19:15:18]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-04-04 19:15:19]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pbos@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1853503003/#ps260001 (title:
"Rename file for readability.")

[commit-bot: I haz the power, 2016-04-04 19:15:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1853503003/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1853503003/260001

[commit-bot: I haz the power, 2016-04-04 21:10:46]

Description was changed from

==========
Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=
==========

to

==========
Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=
==========

[commit-bot: I haz the power, 2016-04-04 21:10:47]

Committed patchset #5 (id:260001)

[commit-bot: I haz the power, 2016-04-04 21:10:54]

Description was changed from

==========
Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=
==========

to

==========
Add CoreVideoFrameBuffer.

- Makes vt h264 decoder output CoreVideoFrameBuffer
- Makes iOS renderer convert frame buffer if it is not i420

BUG=

Committed: https://crrev.com/7d06a8cfe420772aa0bb5658f7b28396ab9b1e26
Cr-Commit-Position: refs/heads/master@{#12224}
==========

[commit-bot: I haz the power, 2016-04-04 21:10:56]

Patchset 5 (id:??) landed as
https://crrev.com/7d06a8cfe420772aa0bb5658f7b28396ab9b1e26
Cr-Commit-Position: refs/heads/master@{#12224}