Implement screen capture for android

Implement screen capture for android

This CL adds screen capture support for android. The screen data is captured
by android.media.projection APIs. In this CL, a new class ScreenCapturerAndroid
is created which is inherited from webrtc::ScreenCapturer.

BUG=487935

[jiajia.qin, 2015-05-14 06:01:27]

jiajia.qin@intel.com changed reviewers:
+ dalecurtis@chromium.org, darin@chromium.org, sergeyu@chromium.org,
wolenetz@chromium.org

[jiajia.qin, 2015-05-14 06:46:58]

Hi, this is a rough version. Currently, the screen data can be shown correctly
using BUG=487935's steps. But it stills has problem when multi-times execute
'refresh and click "start to capture screen" button'. I want to hear opinions of
yours about this feature that whether I am on the right direction. So I send
this CL in a early stage.

Here is my concerns:
1. MediaProjectionManager API must depend on an activity since it needs to call
'startActivityForResult()' to prompt the user whether to allow screen capture.
And the result of this activity should be passed to getMediaProjection so that
we can get the MediaProjection object. Currently, this part is put in
ScreenCapture.java. However, in
MediaCaptureDevicesDispatcher::ProcessScreenCaptureAccessRequest, I directly let
user_approved = true. But in ScreenCapturerAndroid::Start, it's the real place
to prompt user whether to allow screen capture.
2. Currently, ScreenCapturerAndroid is inherited from webrtc::ScreenCapturer.
And in DesktopCaptureDevice::Create, when it's OS_ANDROID, a
ScreenCapturerAndroid instance will be created. Another option is to let
ScreenCapturerAndroid inherit VideoCaptureDevice which is similar with
VideoCaptureDeviceAndroid. And in
VideoCaptureManager::DoStartDeviceOnDeviceThread, case
MEDIA_DESKTOP_VIDEO_CAPTURE, when it's OS_ANDROID and desktop_id.type =
DesktopMediaID::TYPE_SCREEN, create an ScreenCapturerAndroid instance.

[DaleCurtis, 2015-05-14 16:13:33]

dalecurtis@chromium.org changed reviewers:
+ miu@chromium.org, qinmin@chromium.org
- darin@chromium.org, sergeyu@chromium.org, wolenetz@chromium.org

[DaleCurtis, 2015-05-14 16:13:34]

sergeyu,darin, wolenetz, me, +mfoltz -> cc
+miu, qinmin as reviewers.

[DaleCurtis, 2015-05-14 16:13:47]

dalecurtis@chromium.org changed reviewers:
- dalecurtis@chromium.org

[miu, 2015-05-20 03:33:27]

Note: Please read the LAST comment first, as it may invalidate many of the
earlier ones.

https://codereview.chromium.org/1140113002/diff/1/content/DEPS
File content/DEPS (right):

https://codereview.chromium.org/1140113002/diff/1/content/DEPS#newcode35
content/DEPS:35: "+media/screen_capture/android",
This should probably be placed much deeper in the directory hierarchy.  How
about in content/browser/media/capture/DEPS?

https://codereview.chromium.org/1140113002/diff/1/content/browser/media/captu...
File content/browser/media/capture/desktop_capture_device.cc (right):

https://codereview.chromium.org/1140113002/diff/1/content/browser/media/captu...
content/browser/media/capture/desktop_capture_device.cc:191: // available which
is similar with VideoCapture
What happens if the screen does not update for a long while?  Will this stop the
providing frames?

The comment here explains the issue that affects tab capture on all platforms,
and also desktop capture on ChromeOS:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/me...

https://codereview.chromium.org/1140113002/diff/1/content/browser/media/captu...
content/browser/media/capture/desktop_capture_device.cc:212: #if
!defined(OS_ANDROID)
Why don't these statements apply to the android platform?  At the least, provide
a comment here.

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/ScreenCapture.java (right):

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:61:
nativeOnFrameAvailable(mNativeScreenCapturerAndroid,
You need to pass extra values here:

1. image.getCropRect() is needed since you cannot guarantee the ByteBuffer
contains tightly-packed pixels.

2. image.getTimestamp() should be passed instead of just timestamping the
media::VideoFrames with "TickTime::Now()" in the C++ module.  Why throw away the
perfect timestamp value?  ;-)

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:124: //mWidth
= size.x;
Did you mean to fix these LOC rather than hard-code the width and height?

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:173:
mImageReader = ImageReader.newInstance(mWidth, mHeight, PixelFormat.RGBA_8888,
2);
Could you add a comment here about why "2" is a reasonable value?  I suspect the
point is to allow the Chrome thread to memcpy the image data inside the call to
nativeOnFrameAvailable() while the image reader thread is busy capturing the
next frame.

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:175:
HandlerThread thread = new HandlerThread("ScreenPreview");
ScreenPreview?

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
File media/base/android/java/src/org/chromium/media/ScreenCaptureFactory.java
(right):

https://codereview.chromium.org/1140113002/diff/1/media/base/android/java/src...
media/base/android/java/src/org/chromium/media/ScreenCaptureFactory.java:20:
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
nit: This expression doesn't need its own method.

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
File media/screen_capture/android/DEPS (right):

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
media/screen_capture/android/DEPS:1: include_rules = [
Looks like you're creating a new media/screen_capture directory hierarchy.  You
should probably ping dalecurtis@ or someone from the media/OWNERS file to make
sure this is okay.

In my opinion, it would be better to name this media/capture (to be consistent
with existing efforts in content/browser/media/capture).  In the future, it may
make sense to refactor some of the code out of libcontent and into this new
directory structure here.

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
File media/screen_capture/android/screen_capture_factory_android.h (right):

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
media/screen_capture/android/screen_capture_factory_android.h:27: }  //
namespace media
nit: Add an extra empty line above this line.

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
File media/screen_capture/android/screen_capturer_android.h (right):

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
media/screen_capture/android/screen_capturer_android.h:18: class
ScreenCapturerAndroid : public webrtc::ScreenCapturer {
After looking at all the code, I really don't think you should inherit from
webrtc::ScreenCapturer for this implementation.  webrtc::ScreenCapturer is a
single-threaded, poll-based interface; but the Android platform is providing a
multi-threaded, pushed-based interface.  Also:

1. None of the webrtc::ScreenCapturer methods, except for Start(), needs to be
implemented (or is doing what the interface declares they should do).

2. This change is introducing a huge amount of new dependencies on
third_party/webrtc all over the build/DEPS and C++ files.  Every class you're
using in the webrtc namespace has an analogue in Chromium code (e.g.,
webrtc::TickTime --> base::TimeTicks).  In other words, you could simplify this
change a lot by depending only on code found in the Chromium project.

My suggestions: I'd move this ScreenCapturerAndroid class to
content/browser/media/capture, rename it to ScreenCaptureMachineAndroid, and
have it implement the content::VideoCaptureMachine interface:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/me...

For a good example of how that interface is implemented, see
AuraWindowCaptureMachine::Start() and Stop().

Also, note that using the content::ThreadSafeOracleProxy class, as
AuraWindowCaptureMachine does, will allow you to leverage all the hard work
that's gone into the capture implementation:

1. Smooth sampling of frames.  For example, when Android produces frames at 60
FPS, but the consumer can only handle 24 FPS, what should be done?

2. Detecting animated content for perfect frame timestamp rewriting.  Meaning,
Android will capture frames and provide compositor presentation timestamps (from
Image.getTimestamp()).  However, consumers of your screen capture will do better
to receive smoothed-out timestamps instead of 60Hz-rounded ones.  In other
words, when a 24 FPS video is playing, you want your all the frame timestamps to
be 41.7 ms apart, not 33.3 then 50.0 then 33.3 then 50.0...

3. All the in-progress work the team is doing to improve the user experience. 
In particular, there's an effort which will automatically change capture
resolutions in CPU- or network-constrained user environments throughout a
capture session.  I expect this will be just as, if not more, important on
Android than it is on the desktop platforms.  If you implement a
ScreenCaptureMachineAndroid, we'll be forced to upgrade the android experience
along with the desktop platforms as we land code changes.  ;-)

[jiajia.qin, 2015-05-20 12:07:16]

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
File media/screen_capture/android/screen_capturer_android.h (right):

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
media/screen_capture/android/screen_capturer_android.h:18: class
ScreenCapturerAndroid : public webrtc::ScreenCapturer {
On 2015/05/20 03:33:27, miu wrote:
> After looking at all the code, I really don't think you should inherit from
> webrtc::ScreenCapturer for this implementation.  webrtc::ScreenCapturer is a
> single-threaded, poll-based interface; but the Android platform is providing a
> multi-threaded, pushed-based interface.  Also:
> 
> 1. None of the webrtc::ScreenCapturer methods, except for Start(), needs to be
> implemented (or is doing what the interface declares they should do).
> 
> 2. This change is introducing a huge amount of new dependencies on
> third_party/webrtc all over the build/DEPS and C++ files.  Every class you're
> using in the webrtc namespace has an analogue in Chromium code (e.g.,
> webrtc::TickTime --> base::TimeTicks).  In other words, you could simplify
this
> change a lot by depending only on code found in the Chromium project.

Agree with you. Inheriting from webrtc::ScreenCapturer was my initial thought.
At
the beginning, I was considering to put these implementation into
'webrtc/modules/desktop_capture'. But I found that I couldn't use jni to call
the
android API and benefit chromium. So I moved the code to src/media/ but still
inherited from webrtc::ScreenCapturer.

Currently, I tried another method which let ScreenCapturerAndroid inherit
VideoCaptureDevice.
Since I almost have finished the function in this way, I submit it as PS#2.
In this method, ScreenCapturerAndroid implements media::VideoCaptureDevice
interface.
I basically refer to media::VideoCaptureDeviceAndroid as the example.
Please ignore the number '2' in class name or file name. It's just a distinguish
with the
first method. I want to give you a preview, so I submit it. 

  
> My suggestions: I'd move this ScreenCapturerAndroid class to
> content/browser/media/capture, rename it to ScreenCaptureMachineAndroid, and
> have it implement the content::VideoCaptureMachine interface:

Thanks, miu. I will investigate this method tomorrow and see if it is better.
Please also check PS#2 since it also solves the issues you list above. 

>https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/media/capture/content_video_capture_device_core.h&rcl=1432082561&l=105
> 
> For a good example of how that interface is implemented, see
> AuraWindowCaptureMachine::Start() and Stop().
> 
> Also, note that using the content::ThreadSafeOracleProxy class, as
> AuraWindowCaptureMachine does, will allow you to leverage all the hard work
> that's gone into the capture implementation:
> 
> 1. Smooth sampling of frames.  For example, when Android produces frames at 60
> FPS, but the consumer can only handle 24 FPS, what should be done?
> 
> 2. Detecting animated content for perfect frame timestamp rewriting.  Meaning,
> Android will capture frames and provide compositor presentation timestamps
(from
> Image.getTimestamp()).  However, consumers of your screen capture will do
better
> to receive smoothed-out timestamps instead of 60Hz-rounded ones.  In other
> words, when a 24 FPS video is playing, you want your all the frame timestamps
to
> be 41.7 ms apart, not 33.3 then 50.0 then 33.3 then 50.0...
> 
> 3. All the in-progress work the team is doing to improve the user experience. 
> In particular, there's an effort which will automatically change capture
> resolutions in CPU- or network-constrained user environments throughout a
> capture session.  I expect this will be just as, if not more, important on
> Android than it is on the desktop platforms.  If you implement a
> ScreenCaptureMachineAndroid, we'll be forced to upgrade the android experience
> along with the desktop platforms as we land code changes.  ;-)

[mark a. foltz, 2015-05-20 15:01:28]

mfoltz@chromium.org changed reviewers:
+ avayvod@chromium.org, mfoltz@chromium.org, sergeyu@chromium.org

[mark a. foltz, 2015-05-20 15:01:29]

+avayvod for Cast and +sergeyu for general review of desktop capture code

- Is this accessible by getUserMedia() from the open Web?  I believe this
feature is only accessible from whitelisted origins on desktop Chrome.

- What is the user permission and notification model for Android?  How does the
user terminate screen capture?

[jiajia.qin, 2015-05-21 01:31:38]

On 2015/05/20 15:01:29, mfoltz_ooo_until_5-26 wrote:
> +avayvod for Cast and +sergeyu for general review of desktop capture code
> 
> - Is this accessible by getUserMedia() from the open Web?  I believe this
> feature is only accessible from whitelisted origins on desktop Chrome.

Sorry, I don't understand your question. I think it should be same accessible
scope with getUserMedia's video capture. 

 
> - What is the user permission and notification model for Android? 

I use android.media.projection API to implement screen capturing function.
The API example I referred is
https://github.com/googlesamples/android-ScreenCapture.
This usage need to call startActivityForResult() which must use
MediaProjectionManager.createScreenCaptureIntent as the parameter to start an
activity to start screen capture. The activity will prompt the user whether to
allow screen capture. I think this is the user permission and notification
model for android.

> - How does the user terminate screen capture?

I think this is similar with video capture. For end users, they can treat the
capture stream as the same no mater it is captured from camera or the screen.
stopScreenCapture on java side can stop the screen capture.

[juberti2, 2015-05-21 02:23:10]

juberti@chromium.org changed reviewers:
+ jiayl@chromium.org

[Sergey Ulanov, 2015-05-21 21:50:16]

On Wed, May 20, 2015 at 6:31 PM, <jiajia.qin@intel.com> wrote:

> On 2015/05/20 15:01:29, mfoltz_ooo_until_5-26 wrote:
>
>> +avayvod for Cast and +sergeyu for general review of desktop capture code
>>
>
>  - Is this accessible by getUserMedia() from the open Web?  I believe this
>> feature is only accessible from whitelisted origins on desktop Chrome.
>>
>
> Sorry, I don't understand your question. I think it should be same
> accessible
> scope with getUserMedia's video capture.


See screen_capture_enabled flag in media_capture_devices_dispatcher.cc . On
desktop direct screen capture is allowed only for very narrow set of
extensions. It's not available for drive-by web (and probably never will be
in its current form). On Android there are no extensions, so this feature
can work only when you run chrome with the
--enable-usermedia-screen-capturing flag. I'd like to understand what's the
end goal and how you plan to make this feature usable. As it stands right
now a simple confirmation prompt is not enough to gate access to this
feature for drive-by web, so we won't be able to simply remove the flag to
enable this feature for everybody. Beside that if you plan to make this
feature available for drive-by web then it should go through the proper
Blink approval process first, see
http://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process .

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jiajia.qin, 2015-05-22 02:14:48]

On 2015/05/21 21:50:16, Sergey Ulanov wrote:
> On Wed, May 20, 2015 at 6:31 PM, <mailto:jiajia.qin@intel.com> wrote:
> 
> > On 2015/05/20 15:01:29, mfoltz_ooo_until_5-26 wrote:
> >
> >> +avayvod for Cast and +sergeyu for general review of desktop capture code
> >>
> >
> >  - Is this accessible by getUserMedia() from the open Web?  I believe this
> >> feature is only accessible from whitelisted origins on desktop Chrome.
> >>
> >
> > Sorry, I don't understand your question. I think it should be same
> > accessible
> > scope with getUserMedia's video capture.
> 
> 
> See screen_capture_enabled flag in media_capture_devices_dispatcher.cc . On
> desktop direct screen capture is allowed only for very narrow set of
> extensions. It's not available for drive-by web (and probably never will be
> in its current form).

Just curious. Why it can't be available for dive-by web?

> On Android there are no extensions, so this feature
> can work only when you run chrome with the
> --enable-usermedia-screen-capturing flag. I'd like to understand what's the
> end goal and how you plan to make this feature usable.

We have some clients who want to have this feature for android. Personally,
I also think it's a very useful feature. So I begin this work. My end goal is
that let android support screen capture feature.

About "how you plan to make this feature usable", I have submitted PS#2 which
is my current method. 

> As it stands right
> now a simple confirmation prompt is not enough to gate access to this
> feature for drive-by web, so we won't be able to simply remove the flag to
> enable this feature for everybody. 

So, could you please give some suggestions in what kind of permission
it can be enough to gate access to this feature for drive-by web?

Another question, if I don't plan to make this feature available for
drive-by web, Can this CL in current way, which need
'--enable-usermedia-screen-capturing' flag, be merged?

[mcasas, 2015-05-22 02:30:58]

mcasas@chromium.org changed reviewers:
+ mcasas@chromium.org

[mcasas, 2015-05-22 02:30:59]

https://codereview.chromium.org/1140113002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/1140113002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager.cc:424: if
(desktop_id.type == DesktopMediaID::TYPE_SCREEN) {
indent.

No need for {}

https://codereview.chromium.org/1140113002/diff/20001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/ScreenCapture.java (right):

https://codereview.chromium.org/1140113002/diff/20001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:40: public
class ScreenCapture extends Fragment {
ScreenCapture looks a lot to the naked eye like 
VideoCaptureCamera2 [1]; CrImageReaderListener here
is also very much like there.

Suggestion: Make ScreenCapture and its Factory
derive from VideoCaptureDeviceAndroid2.


[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android...

https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
File media/screen_capture/android/screen_capturer2_android.h (right):

https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
media/screen_capture/android/screen_capturer2_android.h:1: // Copyright (c) 2015
The Chromium Authors. All rights reserved.
Remove (c)

https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
media/screen_capture/android/screen_capturer2_android.h:21: class
ScreenCapturer2Android : public VideoCaptureDevice {
This class is very similar to VideoCaptureDeviceAndroid [1]

Please refactor the code so that either 
ScreenCapturer2Android derives from VideoCaptureDeviceAndroid
or both derive from a common interface/class.

Same comments for the Factory.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...

[jiajia.qin, 2015-05-22 02:46:25]

On 2015/05/22 02:30:59, mcasas wrote:

>
https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
> media/screen_capture/android/screen_capturer2_android.h:21: class
> ScreenCapturer2Android : public VideoCaptureDevice {
> This class is very similar to VideoCaptureDeviceAndroid [1]
> 
> Please refactor the code so that either 
> ScreenCapturer2Android derives from VideoCaptureDeviceAndroid
> or both derive from a common interface/class.
> 
> Same comments for the Factory.
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...

Hi, mcasas. Thanks for your comments. Do you notice miu's last comment? He
suggested that "move this ScreenCapturerAndroid class to
content/browser/media/capture, rename it to ScreenCaptureMachineAndroid, and
have it implement the content::VideoCaptureMachine interface". In that case, we
can use 'content::ThreadSafeOracleProxy' class. What's your opinion?

[jiajia.qin, 2015-05-22 03:21:28]

On 2015/05/22 02:46:25, jiajia.qin wrote:
> On 2015/05/22 02:30:59, mcasas wrote:
> 
> >
>
https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
> > media/screen_capture/android/screen_capturer2_android.h:21: class
> > ScreenCapturer2Android : public VideoCaptureDevice {
> > This class is very similar to VideoCaptureDeviceAndroid [1]
> > 
> > Please refactor the code so that either 
> > ScreenCapturer2Android derives from VideoCaptureDeviceAndroid
> > or both derive from a common interface/class.
> > 
> > Same comments for the Factory.
> > 
> > [1]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...
> 
> Hi, mcasas. Thanks for your comments. Do you notice miu's last comment? He
> suggested that "move this ScreenCapturerAndroid class to
> content/browser/media/capture, rename it to ScreenCaptureMachineAndroid, and
> have it implement the content::VideoCaptureMachine interface". In that case,
we
> can use 'content::ThreadSafeOracleProxy' class. What's your opinion?

Seems that VideoCaptureDeviceAndroid and ScreenCapturerAndroid both can utilize
content::ContentVideoaptureDeviceCore just like
content::DesktopCaptureDeviceAura does.

[jiajia.qin, 2015-05-22 09:59:14]

Hi, miu.
Currently, I am trying to refactor my patch with your suggestion. Create a new
class ScreenCaptureDeviceAndroid which inherited from media::VideoCaptureDevice.
Meanwhile, it contains a member variable content::ContentVideoCaptureDeviceCore
core_ which just like content::DesktopCaptureDeviceAura does. Another class is
ScreenCaptureMachineAndroid which inherited from content::VideoCaptureMachine.

However, I met a problem. I found that ThreadSafeCaptureOracle only have a
DidCaptureFrame method in which it will call
client_->onIncomingCapturedVideoFrame. But there is no other method which calls
client_->onIncomingCapturedData. And in my case, I need a such kind of method,
let's just call it DidCaptureData. I want to ask why this function haven't been
exposed or you are developing it?

[mcasas, 2015-05-22 15:27:26]

On 2015/05/22 02:46:25, jiajia.qin wrote:
> On 2015/05/22 02:30:59, mcasas wrote:
> 
> >
>
https://codereview.chromium.org/1140113002/diff/20001/media/screen_capture/an...
> > media/screen_capture/android/screen_capturer2_android.h:21: class
> > ScreenCapturer2Android : public VideoCaptureDevice {
> > This class is very similar to VideoCaptureDeviceAndroid [1]
> > 
> > Please refactor the code so that either 
> > ScreenCapturer2Android derives from VideoCaptureDeviceAndroid
> > or both derive from a common interface/class.
> > 
> > Same comments for the Factory.
> > 
> > [1]
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/captur...
> 
> Hi, mcasas. Thanks for your comments. Do you notice miu's last comment? He
> suggested that "move this ScreenCapturerAndroid class to
> content/browser/media/capture, rename it to ScreenCaptureMachineAndroid, and
> have it implement the content::VideoCaptureMachine interface". In that case,
we
> can use 'content::ThreadSafeOracleProxy' class. What's your opinion?


Oh I think I understand miu@s comments. If you bury the C++ code
behind the current screen capture code, you can reuse the
existing infra to e.g. select Desktop capture from
JScript GetUserMedia() etc, which is a good thing.

However in general please try not to replicate code.
My comments still apply to Android Java code, where
big chunks are common with VideoCaptureDeviceAndroid2.
Please have another look and try to extend the existing
classes. If this CL gets too large, you can split it
in one with Android + JNI connectors and then other for 
C++ code. Also, C++ JNI code lives in media/ whereas
screen capture lives in content/, and I don't think
you want to add JNI connectors to content/; in that
case you can add a screen capture android device under
 media/video/capture/android and make a proxy/front 
end for it under content/browser/media/capture 
following miu@ suggestions.

[miu, 2015-05-22 23:41:20]

On 2015/05/22 09:59:14, jiajia.qin wrote:
> However, I met a problem. I found that ThreadSafeCaptureOracle only have a
> DidCaptureFrame method in which it will call
> client_->onIncomingCapturedVideoFrame. But there is no other method which
calls
> client_->onIncomingCapturedData. And in my case, I need a such kind of method,
> let's just call it DidCaptureData. I want to ask why this function haven't
been
> exposed or you are developing it?

OnIncomingCapturedData() is an old interface for directly copying from a
webcam's mapped video memory.  Internally, it copies the data into a new
VideoFrame (doing RGB-->YUV conversion), and calls
OnIncomingCapturedVideoFrame().

ThreadSafeCaptureOracle::ObserveEventAndDecideCapture() creates a VideoFrame for
you (the |storage| output argument).  All you need to do is populate it.  I
think your OnFrameAvailable() method will need to look something like:

#include "media/base/yuv_convert.h"
#include "third_party/libyuv/include/libyuv.h"

void ScreenCapturer2Android::OnFrameAvailable(JNIEnv* env,
                                              jobject obj,
                                              jbyteArray data,
                                              jint length) {
  // Ask the oracle if this frame should be captured.
  scoped_refptr<media::VideoFrame> frame;
  CaptureFrameCallback capture_callback;
  if (!oracle_->ObserveEventAndDecideCapture(..., &frame, &capture_callback))
    return;

  jbyte* buffer = env->GetByteArrayElements(data, NULL);
  DCHECK(buffer);

  // If needed, scale the captured frame to the required size.
  uint8* source_argb = reinterpret_cast<uint8_t*>(buffer);
  int source_stride = android_argb_frame_stride;
  scoped_ptr<uint8[]> scaled_frame_data;
  if (gfx::Size(android_argb_frame_width, android_argb_frame_height) != 
      frame->visible_rect().size()) {
    scaled_frame_data.reset(new uint8[frame->visible_rect().GetArea() * 4]);
    libyuv::ARGBScale(source_argb,
                      source_stride,
                      android_argb_frame_width,
                      android_argb_frame_height,
                      scaled_frame_data.get(),
                      frame->visible_rect().width() * 4,
                      frame->visible_rect().width(),
                      frame->visible_rect().height(),
                      libyuv::kFilterBilinear);
    source_argb = scaled_frame_data.get();
    source_stride = frame->visible_rect().width() * 4;
  }

  // Populate the VideoFrame.
  DCHECK(frame->format() == media::VideoFrame::I420 ||
         frame->format() == media::VideoFrame::YV12);
  base::TimeTicks timestamp = ...NEED THIS PLUMBED THROUGH JNI CALL...;
  media::ConvertRGB32ToYUV(source_argb,
                           frame->visible_data(media::VideoFrame::kYPlane),
                           frame->visible_data(media::VideoFrame::kUPlane),
                           frame->visible_data(media::VideoFrame::kVPlane),
                           frame->visible_rect().width(),
                           frame->visible_rect().height(),
                           source_stride,
                           frame->stride(media::VideoFrame::kYPlane),
                           frame->stride(media::VideoFrame::kUPlane));

  // Deliver the populated VideoFrame.
  capture_callback.Run(frame, timestamp, true /* success */);
}

[jiajia.qin, 2015-05-27 03:45:06]

Ping miu
I have refactored the code as you suggested. But I did some change for class
ContentVideoCaptureDeviceCore. I let machine decide whether running on UI thread
or device thread rather than directly running on UI thread in
ContentVideoCaptureDeviceCore. I did this change is that I found screen capture
can't run in UI thread because in java side, the fragmentTransaction will fail
if running in UI thread.

Ping mcasas:
About your suggestion "Make ScreenCapture and its Factory
derive from VideoCaptureCamera2" in java code. I think ScreenCapture has lots of
difference with  VideoCaptureCamera2 even though they both have ImageReader and
ImageReaderListener. But the onImageAvailable in  CrImageReaderListener is
different. The startCapture and stopCapture is also very different. I think
maybe it's ok just like current implementation?

[Sergey Ulanov, 2015-05-27 19:36:31]

On 2015/05/22 02:14:48, jiajia.qin wrote:
> On 2015/05/21 21:50:16, Sergey Ulanov wrote:
> > On Wed, May 20, 2015 at 6:31 PM, <mailto:jiajia.qin@intel.com> wrote:
> > 
> > > On 2015/05/20 15:01:29, mfoltz_ooo_until_5-26 wrote:
> > >
> > >> +avayvod for Cast and +sergeyu for general review of desktop capture code
> > >>
> > >
> > >  - Is this accessible by getUserMedia() from the open Web?  I believe this
> > >> feature is only accessible from whitelisted origins on desktop Chrome.
> > >>
> > >
> > > Sorry, I don't understand your question. I think it should be same
> > > accessible
> > > scope with getUserMedia's video capture.
> > 
> > 
> > See screen_capture_enabled flag in media_capture_devices_dispatcher.cc . On
> > desktop direct screen capture is allowed only for very narrow set of
> > extensions. It's not available for drive-by web (and probably never will be
> > in its current form).
> 
> Just curious. Why it can't be available for dive-by web?

(1) it's not standard (note that there is a W3C working draft for a different
API: http://www.w3.org/TR/screen-capture/ , but it's just a draft and AFAIK no
browsers have implemented it)
(2) privacy/security risks: users often click "Ok" on the confirmation prompt
without reading the question.


> > On Android there are no extensions, so this feature
> > can work only when you run chrome with the
> > --enable-usermedia-screen-capturing flag. I'd like to understand what's the
> > end goal and how you plan to make this feature usable.
> 
> We have some clients who want to have this feature for android. Personally,
> I also think it's a very useful feature. So I begin this work. My end goal is
> that let android support screen capture feature.
> 
> About "how you plan to make this feature usable", I have submitted PS#2 which
> is my current method. 

That doesn't make the feature usable because the flag is still required.


> 
> > As it stands right
> > now a simple confirmation prompt is not enough to gate access to this
> > feature for drive-by web, so we won't be able to simply remove the flag to
> > enable this feature for everybody. 
> 
> So, could you please give some suggestions in what kind of permission
> it can be enough to gate access to this feature for drive-by web?

Sorry, I don't have good suggestions. On desktop we provide desktop capture
extension API: https://developer.chrome.com/extensions/desktopCapture . It also
supports delegation when a drive-by webpage can get access to desktop capture by
talking to a cooperating extension. It's a bit more secure than the screen
capture API in two ways:
 1. It shows window picker dialog, so the user has to explicitly select a screen
or window to share instead of just clicking "Ok",
 2. It depends on extension being installed and extensions have permission
prompt when being installed.

we would probably want the similar properties to have this feature enabled on
Android, but it's more challenging because there are no extensions and it's
harder to get UI right (there is always only one screen and no windows, so
window picker dialog doesn't make much sense)

> Another question, if I don't plan to make this feature available for
> drive-by web, Can this CL in current way, which need
> '--enable-usermedia-screen-capturing' flag, be merged?

It would affect all users negatively (bigger binary, larger attack surface)
without any benefits, so I don't think it's worth adding this functionality
unless there is a plan to make it usable without the flag. E.g. I wouldn't mind
landing it if the end goal was to implement W3C screen capture spec, but as far
as I know it's still long way from being finished.

[jiajia.qin, 2015-05-28 03:19:05]

> > > On Android there are no extensions, so this feature
> > > can work only when you run chrome with the
> > > --enable-usermedia-screen-capturing flag. I'd like to understand what's
the
> > > end goal and how you plan to make this feature usable.
> > 
> > We have some clients who want to have this feature for android. Personally,
> > I also think it's a very useful feature. So I begin this work. My end goal
is
> > that let android support screen capture feature.
> > 
> > About "how you plan to make this feature usable", I have submitted PS#2
which
> > is my current method. 
> 
> That doesn't make the feature usable because the flag is still required.

The differ is this feature can be used with this flag. But if we don't add this
patch,
even users open this flag, they still can't use this feature in android.

There are many downstream project which based on content level of chromium.
We can let them decide whether to open that flag and with what kind of security
module.
But in chromium side, we can enable the low level implementation and make sure
this
path is complete. Otherwise, the downstream project have to maintain their own
screen capture module and do lots of rebase work going with upstream.


> > 
> > > As it stands right
> > > now a simple confirmation prompt is not enough to gate access to this
> > > feature for drive-by web, so we won't be able to simply remove the flag to
> > > enable this feature for everybody. 
> > 
> > So, could you please give some suggestions in what kind of permission
> > it can be enough to gate access to this feature for drive-by web?
> 
> Sorry, I don't have good suggestions. On desktop we provide desktop capture
> extension API: https://developer.chrome.com/extensions/desktopCapture . It
also
> supports delegation when a drive-by webpage can get access to desktop capture
by
> talking to a cooperating extension. It's a bit more secure than the screen
> capture API in two ways:
>  1. It shows window picker dialog, so the user has to explicitly select a
screen
> or window to share instead of just clicking "Ok",
>  2. It depends on extension being installed and extensions have permission
> prompt when being installed.
> 
> we would probably want the similar properties to have this feature enabled on
> Android, but it's more challenging because there are no extensions and it's
> harder to get UI right (there is always only one screen and no windows, so
> window picker dialog doesn't make much sense)

You mean, it will become more security if we have more than once to prompt
end user to capture the screen?

I can implement this feature with twice prompts. The first time will be similar
with video capture. There will be a UI bar at the bottom of the tab to prompt
the user that an address wants to access the whole screen.
The second time will happen in calling media.projection API because this API
itself will start an activity to prompt user whether to allow screen capture.
Thus, we can let this feature usable. How do you think?


> > Another question, if I don't plan to make this feature available for
> > drive-by web, Can this CL in current way, which need
> > '--enable-usermedia-screen-capturing' flag, be merged?
> 
> It would affect all users negatively (bigger binary, larger attack surface)
> without any benefits, so I don't think it's worth adding this functionality
> unless there is a plan to make it usable without the flag. E.g. I wouldn't
mind
> landing it if the end goal was to implement W3C screen capture spec, but as
far
> as I know it's still long way from being finished.

If you agree with my above answer, maybe it is not a problem to continue this
CL.
But, I don't agree with your point that
'It would affect all users negatively (bigger binary, larger attack surface)
 without any benefits'.
bigger binary? I don't think there will be bigger binary. I didn't introduce any
third party library, why it will have bigger binary?
It's only hundreds of lines of code.
larger attack surface? I think desktop should have the save risk. But, we still
enabled it. Yes?

[jiajia.qin, 2015-05-28 10:51:26]

Hi, miu
I plan to move content_video_capture_device_core.h from src/content to src/media
so that the class under media can use it without depending on content. But I
need VideoCaptureMachine runs on the device thread which just like what I did in
PS#3 and let machine instance itself to decide which thread to run. Do you think
is it reasonable?

[Sergey Ulanov, 2015-06-01 21:34:26]

On 2015/05/28 03:19:05, jiajia.qin wrote:
> > > > On Android there are no extensions, so this feature
> > > > can work only when you run chrome with the
> > > > --enable-usermedia-screen-capturing flag. I'd like to understand what's
> the
> > > > end goal and how you plan to make this feature usable.
> > > 
> > > We have some clients who want to have this feature for android.
Personally,
> > > I also think it's a very useful feature. So I begin this work. My end goal
> is
> > > that let android support screen capture feature.
> > > 
> > > About "how you plan to make this feature usable", I have submitted PS#2
> which
> > > is my current method. 
> > 
> > That doesn't make the feature usable because the flag is still required.
> 
> The differ is this feature can be used with this flag. But if we don't add
this
> patch,
> even users open this flag, they still can't use this feature in android.

That flag exists for testing/development, not for the users.


> 
> There are many downstream project which based on content level of chromium.
> We can let them decide whether to open that flag and with what kind of
security
> module.
> But in chromium side, we can enable the low level implementation and make sure
> this
> path is complete. Otherwise, the downstream project have to maintain their own
> screen capture module and do lots of rebase work going with upstream.

If you want this just for downstream projects then I suggest you hide the
feature behind a compile-time flag, disabled by default. That would be
acceptable, at least for now, until we have a good plan on how this feature
should work in the future.


> > > 
> > > > As it stands right
> > > > now a simple confirmation prompt is not enough to gate access to this
> > > > feature for drive-by web, so we won't be able to simply remove the flag
to
> > > > enable this feature for everybody. 
> > > 
> > > So, could you please give some suggestions in what kind of permission
> > > it can be enough to gate access to this feature for drive-by web?
> > 
> > Sorry, I don't have good suggestions. On desktop we provide desktop capture
> > extension API: https://developer.chrome.com/extensions/desktopCapture . It
> also
> > supports delegation when a drive-by webpage can get access to desktop
capture
> by
> > talking to a cooperating extension. It's a bit more secure than the screen
> > capture API in two ways:
> >  1. It shows window picker dialog, so the user has to explicitly select a
> screen
> > or window to share instead of just clicking "Ok",
> >  2. It depends on extension being installed and extensions have permission
> > prompt when being installed.
> > 
> > we would probably want the similar properties to have this feature enabled
on
> > Android, but it's more challenging because there are no extensions and it's
> > harder to get UI right (there is always only one screen and no windows, so
> > window picker dialog doesn't make much sense)
> 
> You mean, it will become more security if we have more than once to prompt
> end user to capture the screen?

No.


> 
> I can implement this feature with twice prompts. The first time will be
similar
> with video capture. There will be a UI bar at the bottom of the tab to prompt
> the user that an address wants to access the whole screen.
> The second time will happen in calling media.projection API because this API
> itself will start an activity to prompt user whether to allow screen capture.
> Thus, we can let this feature usable. How do you think?

Again, getting security/privacy right is the main challenge with this feature.
This CL is not the right place for this discussion. I suggest you start with a
short design doc (e.g. post a link in the bug crbug.com/487935). You will get
more useful feedback that way. Also if you plan to make it available for
drive-by web then it's a platform change that would need to go through the
blink's approval process:
http://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process .


> > > Another question, if I don't plan to make this feature available for
> > > drive-by web, Can this CL in current way, which need
> > > '--enable-usermedia-screen-capturing' flag, be merged?
> > 
> > It would affect all users negatively (bigger binary, larger attack surface)
> > without any benefits, so I don't think it's worth adding this functionality
> > unless there is a plan to make it usable without the flag. E.g. I wouldn't
> mind
> > landing it if the end goal was to implement W3C screen capture spec, but as
> far
> > as I know it's still long way from being finished.
> 
> If you agree with my above answer, maybe it is not a problem to continue this
> CL.
> But, I don't agree with your point that
> 'It would affect all users negatively (bigger binary, larger attack surface)
>  without any benefits'.
> bigger binary? I don't think there will be bigger binary. I didn't introduce
any
> third party library, why it will have bigger binary?
> It's only hundreds of lines of code.
> larger attack surface? I think desktop should have the save risk. But, we
still
> enabled it. Yes?

On desktop the feature is enabled for some extensions/apps (without the flag) so
it's not useless there.
The effect on binary size is small, but it's still there. My point is that
unless there is a plan on how the users will benefit from this feature the net
effect of this change is negative. It would be completely acceptable to land
this CL and keep the flag for now if you had some plan on how it will be used in
the future without the flag. Maybe you do have such plan, but I just don't see
it. Also I wouldn't mind getting this CL landed behind a compile-time flag
disabled by default. If compile-time flag is not acceptable then please start
with a design document that defines what the longer term goal is.

Sorry if my comments seem discouraging. I do appreciate your work on this CL. I
just want to make sure there is some plan for this feature going forward, so
your efforts are not wasted.

[jiajia.qin, 2015-06-04 05:47:54]

> > 
> > I can implement this feature with twice prompts. The first time will be
> similar
> > with video capture. There will be a UI bar at the bottom of the tab to
prompt
> > the user that an address wants to access the whole screen.
> > The second time will happen in calling media.projection API because this API
> > itself will start an activity to prompt user whether to allow screen
capture.
> > Thus, we can let this feature usable. How do you think?
> 
> Again, getting security/privacy right is the main challenge with this feature.
> This CL is not the right place for this discussion. I suggest you start with a
> short design doc (e.g. post a link in the bug crbug.com/487935). You will get
> more useful feedback that way. Also if you plan to make it available for
> drive-by web then it's a platform change that would need to go through the
> blink's approval process:
> http://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process .
> 

Hi Sergey. Thanks for your advice. I have wrote a doc
https://docs.google.com/document/d/13-OKb_MyTwYlCULEeRxtDD_N8TqBhHaOWC6ZIYOrC...
 and paste it in the bug crbug.com/487935.

I am considering to use a compile-time flag as you suggested. But I want to
wait other reviewers' comments about this or other issues so that I can modify
them together.

There is one point I am a little confused. Video capture only have an InfoBar
to prompt end user whether to allow to access camera. End users only need to
click ok or block just like screen capture did. Why it is secure for android?
But for screen capture, we need more security?
 
Another question is if I use a compile-time flag to hide the feature. Do I need
to go through the blink's approval process?

[jiajia.qin, 2015-06-18 08:00:34]

Hi, Sergey
There is no any feedback since my last reply. I also pasted the short design doc
https://docs.google.com/document/d/13-OKb_MyTwYlCULEeRxtDD_N8TqBhHaOWC6ZIYOrC...
to the bug http://code.google.com/p/chromium/issues/detail?id=487935. But it
seems there is no response. So, I plan to rebase my code and add the
compile-flag as you suggested. Then, let you review again. How is your opinion?

[mcasas, 2015-06-19 23:22:23]

Seeing that you're still moving things around
I'm going to hold my review, but one thing is
standing out as things are: the desktop/tab
video capture -as it is now- is located in 
content/ because it needs access to either
compositor or Aura, to grab a reference to 
whatever is capturing, and that access is not
available in media. However the rest of the 
actions are similar to media/video/capture M.O.

What I'd suggest is a larger refactoring in 
which:
- desktop/tab capture classes are moved to 
media/video/capture.
- to access the platform (Aura or not), make
this classes define an interface, that will
be implemented in content/browser/media/capture/.
This implementation is created in VideoCaptureManager
at the moment of VideoCaptureDevice instantiation
[1] instead of the #ifdef cascade.

WDYT?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
File media/screen_capture/android/screen_capture_device_android.h (right):

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
media/screen_capture/android/screen_capture_device_android.h:1: // Copyright
2015 The Chromium Authors. All rights reserved.
If really needed, I'd put these new files
under media/video/capture/screen.

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
media/screen_capture/android/screen_capture_device_android.h:9: #include
"content/browser/media/capture/content_video_capture_device_core.h"
media/ cannot depend on content/

[miu, 2015-06-22 20:11:07]

On 2015/06/19 at 23:22:23, mcasas wrote:
> What I'd suggest is a larger refactoring in 

FYI--This was done in https://codereview.chromium.org/1162863003 (started after,
but committed before, this change).

[vashisthg, 2015-08-11 11:35:05]

vashisthg@gmail.com changed reviewers:
+ vashisthg@gmail.com

[vashisthg, 2015-08-11 11:35:06]

https://codereview.chromium.org/1140113002/diff/20001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/ScreenCapture.java (right):

https://codereview.chromium.org/1140113002/diff/20001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:99:
ScreenCapture(Context context, long nativeScreenCapturer2Android) {
Why does this fragment not have a no argument constructor?

[whywhat, 2015-08-17 13:58:46]

https://codereview.chromium.org/1140113002/diff/40001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/BaseChromiumApplication.java
(right):

https://codereview.chromium.org/1140113002/diff/40001/base/android/java/src/o...
base/android/java/src/org/chromium/base/BaseChromiumApplication.java:42:
mActivity = activity;
One should use the ApplicationStatus.getLastTrackedFocusedActivity for correct
activity tracking.

https://codereview.chromium.org/1140113002/diff/40001/content/browser/media/c...
File content/browser/media/capture/aura_window_capture_machine.cc (right):

https://codereview.chromium.org/1140113002/diff/40001/content/browser/media/c...
content/browser/media/capture/aura_window_capture_machine.cc:175:
BrowserThread::PostTask(
This refactoring (changing a sync method to async) seems like something that
could be done in a separate change for easier reviewing?

https://codereview.chromium.org/1140113002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_manager.cc (right):

https://codereview.chromium.org/1140113002/diff/40001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_manager.cc:424: if
(desktop_id.type == DesktopMediaID::TYPE_SCREEN)
nit: fix indent (add 2 spaces)?
is it okay to leave video_capture_device unassigned if the desktop_id.type is
anything else than TYPE_SCREEN?

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
File media/base/android/java/src/org/chromium/media/ScreenCapture.java (right):

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:18: //import
android.media.projection.MediaProjection.Callback;
nit: Uncomment or remove?

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:71: if (image
!= null) {
nit: a one liner

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:82: if
(mVirtualDisplay == null) {
nit: one liner

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:187: // start
screen capture.
nit: follow Javadoc style for public/protected class members

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:190:
Log.i(TAG, "startCapture");
nit: s/Log.i/Log.d?

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:217:
Log.i(TAG, "stopCapture");
ditto

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:219:
mMediaProjection.stop();
nit: fits on one line

https://codereview.chromium.org/1140113002/diff/40001/media/base/android/java...
media/base/android/java/src/org/chromium/media/ScreenCapture.java:231: public
native void nativeOnActivityResult(long nativeScreenCaptureMachineAndroid,
Do these or @CalledByNative methdos have to be public? Use the least visibility
possible.

https://codereview.chromium.org/1140113002/diff/40001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1140113002/diff/40001/media/media.gyp#newcode683
media/media.gyp:683: ['enable_webrtc==1 and OS=="android"', {
the 'enable_webrtc' condition doesn't have a matching one in the GN file and
also for the jni headers rule below.
Have you tried compiling with enable_webrtc=0?

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
File media/screen_capture/android/DEPS (left):

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
media/screen_capture/android/DEPS:2: "+chromecast/android",
why remove this line? bad merge?

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
File media/screen_capture/android/screen_capture_machine_android.cc (right):

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
media/screen_capture/android/screen_capture_machine_android.cc:48: if
(!oracle_decision) {
nit: no need for {} for a one line if statement.

https://codereview.chromium.org/1140113002/diff/40001/media/screen_capture/an...
media/screen_capture/android/screen_capture_machine_android.cc:54: return;
ditto

[jiajia.qin, 2015-08-18 09:08:00]

Thanks all of yours review. Currently, I am busy on other things. But I hope I
can pick up this CL two weeks later. Sorry for the slow response.

[renyuzhuo, 2016-08-10 10:02:23]

renyuzhuo0@gmail.com changed reviewers:
+ renyuzhuo0@gmail.com

[renyuzhuo, 2016-08-10 10:02:24]

i can't get the code, where is the jni/ScreenCapture_jni.h,thank you!

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
File media/screen_capture/android/screen_capturer_android.cc (right):

https://codereview.chromium.org/1140113002/diff/1/media/screen_capture/androi...
media/screen_capture/android/screen_capturer_android.cc:10: #include
"jni/ScreenCapture_jni.h"
where is the jni/ScreenCapture_jni.h,thank you!