Move ContentVideoCaptureDeviceCore from src/content to src/media

Move ContentVideoCaptureDeviceCore from src/content to src/media

This CL moves ContentVideoCaptureDeviceCore related classes from
src/content/browser/media/capture to src/media/capture. That will
benefit the class under media which can use core and machine directly
without depending on content.

Committed: https://crrev.com/a596ea09f52f40f721fb48e72d24a779e3caeed2
Cr-Commit-Position: refs/heads/master@{#334804}

[jiajia.qin, 2015-06-01 09:47:17]

jiajia.qin@intel.com changed reviewers:
+ dalecurtis@chromium.org, miu@chromium.org

[jiajia.qin, 2015-06-01 09:47:18]

Please take a look. Thanks.

[jiajia.qin, 2015-06-04 05:51:25]

On 2015/06/01 09:47:18, jiajia.qin wrote:
> Please take a look. Thanks.

Any feedback? :)

[DaleCurtis, 2015-06-04 21:33:49]

dalecurtis@chromium.org changed required reviewers:
+ miu@chromium.org

[DaleCurtis, 2015-06-04 21:33:50]

The move lgtm, but I defer to miu@ since this is pretty much all his code and
he's reviewing the other CL using this stuff.

[miu, 2015-06-05 01:13:37]

Sorry for the delay.

On the whole, this looks great!  Thanks for the contribution!  :)

Please add a media/capture/OWNERS file with one line (so I can retain
ownership):

  miu@chromium.org

Also, a few small things:

https://codereview.chromium.org/1162863003/diff/1/content/browser/media/captu...
File content/browser/media/capture/aura_window_capture_machine.h (right):

https://codereview.chromium.org/1162863003/diff/1/content/browser/media/captu...
content/browser/media/capture/aura_window_capture_machine.h:77: typedef
media::ThreadSafeCaptureOracle::CaptureFrameCallback
nit: With C++11, this style is now preferred:

  using CaptureFrameCallback =
media::ThreadSafeCaptureOracle::CaptureFrameCallback;

https://codereview.chromium.org/1162863003/diff/1/content/content_browser.gypi
File content/content_browser.gypi (right):

https://codereview.chromium.org/1162863003/diff/1/content/content_browser.gyp...
content/content_browser.gypi:988:
'browser/media/capture/feedback_signal_accumulator.cc',
Please move feedback_signal_accumulator* to the new dir as well.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
File media/capture/media_video_capture_device_core.cc (right):

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.cc:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
nit: Please don't update the year here, since the file is not new, it's just
being moved.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.cc:271: // If capture_machine is
not NULL, then we need to return to the UI thread to
Please remove this comment, since the posting of tasks to the UI thread is an
issue handled externally.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
File media/capture/media_video_capture_device_core.h (right):

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.h:133: class MEDIA_EXPORT
MediaVideoCaptureDeviceCore
naming: Could you name this class ScreenCaptureDeviceCore?  (The old naming
wasn't so great, either.)

Also, the header/cc files should be named screen_capture_device_core.h/.cc.

[jiajia.qin, 2015-06-05 02:31:34]

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
File media/capture/media_video_capture_device_core.h (right):

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.h:133: class MEDIA_EXPORT
MediaVideoCaptureDeviceCore
On 2015/06/05 01:13:37, miu wrote:
> naming: Could you name this class ScreenCaptureDeviceCore?  (The old naming
> wasn't so great, either.)
> 
> Also, the header/cc files should be named screen_capture_device_core.h/.cc.

Hi, miu. I am considering that maybe VideoCaptureDeviceAndroid(which locates in
src/media/video/capture/android/video_capture_device_android.h) also can utilize
core and machine. But one problem is that
ThreadSafeCaptureOracle::DidCaptureFrame seems not provide a rotation parameter.
I think we can strengthen its ability so that VideoCaptureDeviceAndroid can
leverage it. So, keep this class name unchanged is also acceptable. If we change
it to ScreenCaptureDeviceCore, it seems that only screen capture can utilize it.
How do you think?

[miu, 2015-06-05 19:26:11]

On 2015/06/05 02:31:34, jiajia.qin wrote:
> So, keep this class name unchanged is also acceptable. If we change
> it to ScreenCaptureDeviceCore, it seems that only screen capture can utilize
it.
> How do you think?

Wait...Is VideoCaptureDeviceAndroid going to be for video camera capture, or
just screen capture?  The algorithms here are only relevant for screen capture
(e.g., desktop, window, or tab caputre).

[jiajia.qin, 2015-06-08 02:55:00]

On 2015/06/05 19:26:11, miu wrote:
> On 2015/06/05 02:31:34, jiajia.qin wrote:
> > So, keep this class name unchanged is also acceptable. If we change
> > it to ScreenCaptureDeviceCore, it seems that only screen capture can utilize
> it.
> > How do you think?
> 
> Wait...Is VideoCaptureDeviceAndroid going to be for video camera capture, or
> just screen capture?  The algorithms here are only relevant for screen capture
> (e.g., desktop, window, or tab caputre).

VideoCaptureDeviceAndroid is for video camera capture not screen capture.
Sorry, I am not familiar with your algorithm. But from implementation logic,
ScreenCaptureDeviceAndroid is very similar with VideoCaptureDeviceAndroid. So
I think it can be applied to VideoCaptureDeviceAndroid too:) 

Maybe I am wrong. Anyway, I will rename the class name as your suggested.

[jiajia.qin, 2015-06-08 04:58:24]

https://codereview.chromium.org/1162863003/diff/1/content/browser/media/captu...
File content/browser/media/capture/aura_window_capture_machine.h (right):

https://codereview.chromium.org/1162863003/diff/1/content/browser/media/captu...
content/browser/media/capture/aura_window_capture_machine.h:77: typedef
media::ThreadSafeCaptureOracle::CaptureFrameCallback
On 2015/06/05 01:13:36, miu wrote:
> nit: With C++11, this style is now preferred:
> 
>   using CaptureFrameCallback =
> media::ThreadSafeCaptureOracle::CaptureFrameCallback;

Done.

https://codereview.chromium.org/1162863003/diff/1/content/content_browser.gypi
File content/content_browser.gypi (right):

https://codereview.chromium.org/1162863003/diff/1/content/content_browser.gyp...
content/content_browser.gypi:988:
'browser/media/capture/feedback_signal_accumulator.cc',
On 2015/06/05 01:13:36, miu wrote:
> Please move feedback_signal_accumulator* to the new dir as well.

Done.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
File media/capture/media_video_capture_device_core.cc (right):

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.cc:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
On 2015/06/05 01:13:36, miu wrote:
> nit: Please don't update the year here, since the file is not new, it's just
> being moved.

Done.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.cc:271: // If capture_machine is
not NULL, then we need to return to the UI thread to
On 2015/06/05 01:13:36, miu wrote:
> Please remove this comment, since the posting of tasks to the UI thread is an
> issue handled externally.

Done.

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
File media/capture/media_video_capture_device_core.h (right):

https://codereview.chromium.org/1162863003/diff/1/media/capture/media_video_c...
media/capture/media_video_capture_device_core.h:133: class MEDIA_EXPORT
MediaVideoCaptureDeviceCore
On 2015/06/05 01:13:37, miu wrote:
> naming: Could you name this class ScreenCaptureDeviceCore?  (The old naming
> wasn't so great, either.)
> 
> Also, the header/cc files should be named screen_capture_device_core.h/.cc.

Done.

[miu, 2015-06-08 22:01:25]

On 2015/06/08 02:55:00, jiajia.qin wrote:
> On 2015/06/05 19:26:11, miu wrote:
> > Wait...Is VideoCaptureDeviceAndroid going to be for video camera capture, or
> > just screen capture?  The algorithms here are only relevant for screen
capture
> > (e.g., desktop, window, or tab caputre).
> 
> VideoCaptureDeviceAndroid is for video camera capture not screen capture.
> Sorry, I am not familiar with your algorithm. But from implementation logic,
> ScreenCaptureDeviceAndroid is very similar with VideoCaptureDeviceAndroid. So
> I think it can be applied to VideoCaptureDeviceAndroid too:) 

It may or may not be a good idea.  I'm actually not sure.  ;-)

On the one hand, the algorithms assume content is being screen captured from a
compositor (i.e., something that operates around your display hardware's vsync
intervals).  For example, it solves problems like detecting that a 24 FPS video
being rendered on a 60 FPS display.  With normal webcam hardware, that would be
a non-issue.

On the other hand, there is a lot of new stuff going in that looks at how
"overloaded" the system is, and tries to dial back the resolution and frame
rate.  That might be beneficial to normal camera video as well.

For now, yes, let's go with the ScreenCaptureDeviceCore naming.  If it proves
beneficial (or, at least, not harmful) for android camera video, we can discuss
name changes at that time...

[miu, 2015-06-08 22:07:26]

Looks good, except for a couple things:

1. You'll need to rebase against trunk again (e.g., because of
https://codereview.chromium.org/1157193002/).

2. The trybots are having compile issues that need to be resolved.

[jiajia.qin, 2015-06-10 08:27:31]

Patchset #4 (id:60001) has been deleted

[jiajia.qin, 2015-06-10 10:49:46]

On 2015/06/08 22:07:26, miu wrote:
> Looks good, except for a couple things:
> 
> 1. You'll need to rebase against trunk again (e.g., because of
> https://codereview.chromium.org/1157193002/).
> 
> 2. The trybots are having compile issues that need to be resolved.

Hi, miu
I have fixed most compile errors. But there is still one I have
no idea. You can see PS#5 trybot results. I don't know why
web_contents_video_capture_device.cc:296: error: undefined reference
to 'media::ThreadSafeCaptureOracle::ObserveEventAndDecideCapture

I have added the corresponding header file but it still can't find
the corresponding member functions. I think it should be missed the
deps for media/capturing in BUID.gn somewhere in content. However,
even I add //media in content/browser/BUILD.gn. It still has such
error. Could you please give me some guidance where the problem is?
Thank you very much!

[jiajia.qin, 2015-06-11 07:00:55]

Patchset #6 (id:120001) has been deleted

[jiajia.qin, 2015-06-11 08:07:49]

Patchset #6 (id:140001) has been deleted

[jiajia.qin, 2015-06-11 10:23:12]

On 2015/06/10 10:49:46, jiajia.qin wrote:
> On 2015/06/08 22:07:26, miu wrote:
> > Looks good, except for a couple things:
> > 
> > 1. You'll need to rebase against trunk again (e.g., because of
> > https://codereview.chromium.org/1157193002/).
> > 
> > 2. The trybots are having compile issues that need to be resolved.
> 
> Hi, miu
> I have fixed most compile errors. But there is still one I have
> no idea. You can see PS#5 trybot results. I don't know why
> web_contents_video_capture_device.cc:296: error: undefined reference
> to 'media::ThreadSafeCaptureOracle::ObserveEventAndDecideCapture
> 
> I have added the corresponding header file but it still can't find
> the corresponding member functions. I think it should be missed the
> deps for media/capturing in BUID.gn somewhere in content. However,
> even I add //media in content/browser/BUILD.gn. It still has such
> error. Could you please give me some guidance where the problem is?
> Thank you very much!

Hi, miu. This error has been fixed. The solution is to add 'MEDIA_EXPORT'
before class 'ThreadSafeCaptureOracle'. Now, everything is fine.
Please have a final review. Thanks :)

[jiajia.qin, 2015-06-12 07:09:23]

I noticed that win_chromium_x64_dbg trybot is failed. But it seems not
introduced by my patch. Miu, could you please look at current patch if it's good
for you?

[miu, 2015-06-12 17:09:50]

Sorry for the delay.  lgtm % a few things:

1. Looks like the screen_capture_device_core.* files lost their "A+" diff type. 
Try using `git cl upload --similarity=25` or a smaller number, if appropriate,
to allow it to detect the file is based on an earlier file.  This preserves
source code history.

2. I'm afraid you'll have to rebase again, due to
https://codereview.chromium.org/1183553002/ landing.

https://codereview.chromium.org/1162863003/diff/160001/media/capture/thread_s...
File media/capture/thread_safe_capture_oracle.cc (right):

https://codereview.chromium.org/1162863003/diff/160001/media/capture/thread_s...
media/capture/thread_safe_capture_oracle.cc:1: #include
"media/capture/thread_safe_capture_oracle.h"
Missing copyright comment at top of file.

[miu, 2015-06-12 22:48:34]

BTW--I'll be unavailable next week, so if there are any significant changes
after my lgtm, please ask dalecurtis@ or another src/media/OWNER to take a look.

[jiajia.qin, 2015-06-17 03:17:07]

Patchset #7 (id:180001) has been deleted

[jiajia.qin, 2015-06-17 06:35:57]

The CQ bit was checked by jiajia.qin@intel.com

[jiajia.qin, 2015-06-17 06:35:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, miu@chromium.org
Link to the patchset: https://codereview.chromium.org/1162863003/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2015-06-17 06:36:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1162863003/200001

[commit-bot: I haz the power, 2015-06-17 06:36:45]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 07:02:35]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 07:32:50]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 08:02:53]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 08:33:05]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 08:43:08]

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

[commit-bot: I haz the power, 2015-06-17 08:43:10]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jiajia.qin, 2015-06-17 09:52:59]

The CQ bit was checked by jiajia.qin@intel.com

[commit-bot: I haz the power, 2015-06-17 09:53:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1162863003/200001

[commit-bot: I haz the power, 2015-06-17 09:53:25]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 10:02:55]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 10:33:51]

The author jiajia.qin@intel.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-06-17 10:43:15]

Committed patchset #7 (id:200001)

[commit-bot: I haz the power, 2015-06-17 10:44:09]

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