Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682
Committed: https://crrev.com/45c8b8940042bd2574c39920804ade8343cefdba
Cr-Commit-Position: refs/heads/master@{#14885}

[nisse-webrtc, 2016-10-19 08:17:46]

Description was changed from

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced by webrtc::VideoFrame.

WIP, need to figure out how to avoid breaking applications. Cleanest
would be to make the old classes aliases of webrtc::VideoFrame, but
that will likely break any applications using forward declarations.

If that's not possible, we'll have to do this in smaller steps.

On top of cl https://codereview.webrtc.org/2315663002/

BUG=webrtc:5682
==========

to

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

BUG=webrtc:5682
==========

[nisse-webrtc, 2016-10-19 08:18:59]

nisse@webrtc.org changed reviewers:
+ perkj@webrtc.org, pthatcher@webrtc.org

[nisse-webrtc, 2016-10-19 08:18:59]

This is a follow up to delete all use of cricket::*VideoFrame within webrtc.

[pthatcher1, 2016-10-24 17:34:20]

lgtm

https://codereview.chromium.org/2383093002/diff/20001/webrtc/media/engine/web...
File webrtc/media/engine/webrtcvideocapturer.cc (right):

https://codereview.chromium.org/2383093002/diff/20001/webrtc/media/engine/web...
webrtc/media/engine/webrtcvideocapturer.cc:356: sample.width(),
sample.height());
One line?

[nisse-chromium (ooo August 14), 2016-10-25 07:32:47]

nisse@chromium.org changed reviewers:
+ mflodman@webrtc.org, nisse@chromium.org, tkchin@webrtc.org

[nisse-chromium (ooo August 14), 2016-10-25 07:32:48]

OWNER's approval needed.

Zeke, please have a look at the objc changes.

Per, you own the examples subdirectory.

Magnus, you own the webrtc/video_frame.h header. Only a deleted comment there,
though, so you don't need to review this if you are too busy.

https://codereview.webrtc.org/2383093002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideocapturer.cc (right):

https://codereview.webrtc.org/2383093002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideocapturer.cc:356: sample.width(),
sample.height());
On 2016/10/24 17:34:20, pthatcher1 wrote:
> One line?

Done.

[perkj_webrtc, 2016-10-25 10:53:37]

Great to have this done! 

A few ns comments / questions?.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/api/mediastreamint...
File webrtc/api/mediastreaminterface.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/api/mediastreamint...
webrtc/api/mediastreaminterface.h:101: public
rtc::VideoSourceInterface<VideoFrame> {
It make sense to have not write webrtc::VideoFrame here since that is the ns it
exists in. But can you please then to the same in all files you modified in the
api/folder.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/examples/peerconne...
File webrtc/examples/peerconnection/client/main_wnd.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/examples/peerconne...
webrtc/examples/peerconnection/client/main_wnd.h:105: class VideoRenderer :
public rtc::VideoSinkInterface<VideoFrame> {
webrtc::

This is not in the webrtc ns.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/examples/peerconne...
webrtc/examples/peerconnection/client/main_wnd.h:120: void OnFrame(const
VideoFrame& frame) override;
dito

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
File webrtc/media/base/testutils.cc (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
webrtc/media/base/testutils.cc:26: #include "webrtc/video_frame.h"
and remove this include

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
File webrtc/media/base/testutils.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
webrtc/media/base/testutils.h:33: class VideoFrame;
why forward declare? style guide recommend include as much as possible.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/videoca...
File webrtc/media/base/videocapturer.cc (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/videoca...
webrtc/media/base/videocapturer.cc:21: #include "webrtc/video_frame.h"
include in header instead.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/videoca...
File webrtc/media/base/videocapturer.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/videoca...
webrtc/media/base/videocapturer.h:33: class VideoFrame;
dito- prefer include if possible.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
File webrtc/media/devices/gdivideorenderer.cc (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
webrtc/media/devices/gdivideorenderer.cc:20: #include "webrtc/video_frame.h"
remove

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
File webrtc/media/devices/gdivideorenderer.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
webrtc/media/devices/gdivideorenderer.h:24: class VideoFrame;
dito

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
webrtc/media/devices/gdivideorenderer.h:35: void OnFrame(const VideoFrame&
frame) override;
webrtc::

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gtkv...
File webrtc/media/devices/gtkvideorenderer.cc (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gtkv...
webrtc/media/devices/gtkvideorenderer.cc:14: #include "webrtc/video_frame.h"
remove

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gtkv...
File webrtc/media/devices/gtkvideorenderer.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gtkv...
webrtc/media/devices/gtkvideorenderer.h:24: class VideoFrame;
and this

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/media.gyp
File webrtc/media/media.gyp (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/media.gyp#ne...
webrtc/media/media.gyp:135: 'all_dependent_settings': {
why is this needed?  There is no change made to  BUILD.gn

[nisse-webrtc, 2016-10-25 11:21:01]

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/api/mediastreamint...
File webrtc/api/mediastreaminterface.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/api/mediastreamint...
webrtc/api/mediastreaminterface.h:101: public
rtc::VideoSourceInterface<VideoFrame> {
On 2016/10/25 10:53:36, perkj_webrtc wrote:
> It make sense to have not write webrtc::VideoFrame here since that is the ns
it
> exists in. But can you please then to the same in all files you modified in
the
> api/folder. 

Done, in all of api/.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/examples/peerconne...
File webrtc/examples/peerconnection/client/main_wnd.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/examples/peerconne...
webrtc/examples/peerconnection/client/main_wnd.h:105: class VideoRenderer :
public rtc::VideoSinkInterface<VideoFrame> {
On 2016/10/25 10:53:36, perkj_webrtc wrote:
> webrtc::
> 
> This is not in the webrtc ns.

Done. But why did it compile without webrtc:: ???

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
File webrtc/media/base/testutils.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
webrtc/media/base/testutils.h:33: class VideoFrame;
On 2016/10/25 10:53:36, perkj_webrtc wrote:
> why forward declare? style guide recommend include as much as possible.

Really? Quoting
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md :

"Unlike the Google style guide, Chromium style prefers forward declarations to
#includes where possible. This can reduce compile times and result in fewer
files needing recompilation when a header changes."

I don't have a strong opinion either way, but it's good to be consistent.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
File webrtc/media/devices/gdivideorenderer.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/devices/gdiv...
webrtc/media/devices/gdivideorenderer.h:35: void OnFrame(const VideoFrame&
frame) override;
On 2016/10/25 10:53:36, perkj_webrtc wrote:
> webrtc::

Done.

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/media.gyp
File webrtc/media/media.gyp (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/media.gyp#ne...
webrtc/media/media.gyp:135: 'all_dependent_settings': {
On 2016/10/25 10:53:36, perkj_webrtc wrote:
> why is this needed?  There is no change made to  BUILD.gn

Ooops, left over change. Probably needed for some issue later solved in a
different way. Reverted.

[perkj_webrtc, 2016-10-27 15:56:25]

lgtm % git cl format?

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
File webrtc/media/base/testutils.h (right):

https://codereview.webrtc.org/2383093002/diff/40001/webrtc/media/base/testuti...
webrtc/media/base/testutils.h:33: class VideoFrame;
On 2016/10/25 11:21:01, nisse-webrtc wrote:
> On 2016/10/25 10:53:36, perkj_webrtc wrote:
> > why forward declare? style guide recommend include as much as possible.
> 
> Really? Quoting
> https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md
:
> 
> "Unlike the Google style guide, Chromium style prefers forward declarations to
> #includes where possible. This can reduce compile times and result in fewer
> files needing recompilation when a header changes."
> 
> I don't have a strong opinion either way, but it's good to be consistent.

interesting. I guess we follow chrome.

https://codereview.webrtc.org/2383093002/diff/60001/webrtc/media/base/fakemed...
File webrtc/media/base/fakemediaengine.h (right):

https://codereview.webrtc.org/2383093002/diff/60001/webrtc/media/base/fakemed...
webrtc/media/base/fakemediaengine.h:513: sinks() const {
git cl format?

[nisse-webrtc, 2016-10-28 08:39:49]

https://codereview.webrtc.org/2383093002/diff/60001/webrtc/media/base/fakemed...
File webrtc/media/base/fakemediaengine.h (right):

https://codereview.webrtc.org/2383093002/diff/60001/webrtc/media/base/fakemed...
webrtc/media/base/fakemediaengine.h:513: sinks() const {
On 2016/10/27 15:56:25, perkj_webrtc wrote:
> git cl format?

I also don't think that's very pretty, but that's how git cl format wants it.
(There are a couple of other changes cit cl format wants to make, though. I can
review formatting, while waiting for the other reviewers).

Also at a couple of places, it wants formatting like

void RemoveSink(...) override{};

with no space between "override" and "{}", which is ugly. Is that a bug in git
cl format, or is this use of override somehow not the right style?

[nisse-webrtc, 2016-11-01 09:46:02]

nisse@webrtc.org changed reviewers:
+ stefan@webrtc.org
- nisse@chromium.org

[nisse-webrtc, 2016-11-01 09:46:03]

Ping?

Owner's approval needed for the objc files, and for the webrtc/video_frame.h
header. Stefan, can you have a look at the latter?

[perkj_webrtc, 2016-11-01 18:41:24]

On 2016/11/01 09:46:03, nisse-webrtc wrote:
> Ping?
> 
> Owner's approval needed for the objc files, and for the webrtc/video_frame.h
> header. Stefan, can you have a look at the latter?

I would recommend TBR if you have heard nothing tomorrow.

[tkchin_webrtc, 2016-11-01 20:06:33]

On 2016/11/01 18:41:24, perkj_webrtc wrote:
> On 2016/11/01 09:46:03, nisse-webrtc wrote:
> > Ping?
> > 
> > Owner's approval needed for the objc files, and for the webrtc/video_frame.h
> > header. Stefan, can you have a look at the latter?
> 
> I would recommend TBR if you have heard nothing tomorrow.

lgtm

sorry, thought I'd already lgtm-ed this CL before somehow

[nisse-webrtc, 2016-11-02 08:27:38]

Description was changed from

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

BUG=webrtc:5682
==========

to

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682
==========

[nisse-webrtc, 2016-11-02 08:35:33]

https://codereview.webrtc.org/2383093002/diff/100001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2383093002/diff/100001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvideoengine2.cc:1594: webrtc::VideoFrame
video_frame(frame.video_frame_buffer(),
Constructing this object is unnecessary, but we can fix that in a follow up
(ultimately, we shouldn't have an OnFrame method here at all).

I intend to land this cl now.

[nisse-webrtc, 2016-11-02 08:35:46]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-11-02 08:35:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, perkj@webrtc.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2383093002/#ps100001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-11-02 08:35:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-02 08:38:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-02 08:38:46]

Try jobs failed on following builders:
  ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/12088)
  ios64_sim_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/12108)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/14441)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/19673)

[nisse-webrtc, 2016-11-02 09:02:53]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-11-02 09:02:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, tkchin@webrtc.org, perkj@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2383093002/#ps120001 (title:
"objc: Add missing include of video_frame_buffer.h.")

[commit-bot: I haz the power, 2016-11-02 09:03:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-02 10:20:22]

Description was changed from

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682
==========

to

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682
==========

[commit-bot: I haz the power, 2016-11-02 10:20:23]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-11-02 10:20:36]

Description was changed from

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682
==========

to

==========
Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.

Replaced with webrtc::VideoFrame.

TBR=mflodman@webrtc.org
BUG=webrtc:5682

Committed: https://crrev.com/45c8b8940042bd2574c39920804ade8343cefdba
Cr-Commit-Position: refs/heads/master@{#14885}
==========

[commit-bot: I haz the power, 2016-11-02 10:20:37]

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

[nisse-webrtc, 2016-11-02 10:39:39]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.webrtc.org/2471783002/ by nisse@webrtc.org.

The reason for reverting is: Breaks chrome, see
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Builder/build...

Analysis: Chrome uses cricket::VideoFrame, without explicitly including
webrtc/media/base/videoframe.h, and breaks when that file is no longer included
by any other webrtc headers. Will reland after updating Chrome..