Session based capturing for Camera1Capturer.

Session based capturing for Camera1Capturer.

BUG=webrtc:6148
Committed: https://crrev.com/3442579fd777a1b036a01e6d2bd53887dbddceed
Cr-Commit-Position: refs/heads/master@{#14318}

[sakal, 2016-08-10 13:08:15]

Description was changed from

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148
==========

to

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148
==========

[sakal, 2016-08-10 13:08:15]

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

[sakal, 2016-08-11 07:55:09]

Magnus, PTAL.

[magjed_webrtc, 2016-08-11 12:57:14]

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
File webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java (right):

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java:16: import
android.os.Handler;
You have some unused imports here, no?

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java:33: return
Camera1Enumerator.getSupportedFormats(getCameraIndex());
Inline getCameraIndex() and remove the function.

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
File webrtc/api/android/java/src/org/webrtc/Camera1Session.java (right):

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:68:
capturerObserver.onCapturerStarted(false);
I guess this call could be moved into
CameraCapturer.createSessionCallback.onFailure, but it's very arbitrary where
different callbacks are called.

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:244: // Note:
stopPreview or other driver code might deadlock. Deadlock in
If the app calls startCapture and stopCapture and the device deadlocks here,
then the next startCapture/stopCapture will deadlock. Maybe we should add a
timeout to CameraCapturer.stopCapture() for that reason to really make sure we
never deadlock any app thread.

And BTW, can you remove this comment from CameraCapturer that is now out of sync
with the code:
// Use postOnCameraThread() instead of posting directly to the handler - this
way all
// callbacks with a specifed token can be removed at once.

[sakal, 2016-08-15 09:09:49]

Patchset #6 (id:100001) has been deleted

[sakal, 2016-08-15 09:11:58]

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
File webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java (right):

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java:16: import
android.os.Handler;
On 2016/08/11 12:57:13, magjed_webrtc wrote:
> You have some unused imports here, no?

Yeah, my plugin had malfunctioned and I have learned to rely on it. Done.

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Capturer.java:33: return
Camera1Enumerator.getSupportedFormats(getCameraIndex());
On 2016/08/11 12:57:13, magjed_webrtc wrote:
> Inline getCameraIndex() and remove the function.

Done.

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
File webrtc/api/android/java/src/org/webrtc/Camera1Session.java (right):

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:68:
capturerObserver.onCapturerStarted(false);
On 2016/08/11 12:57:13, magjed_webrtc wrote:
> I guess this call could be moved into
> CameraCapturer.createSessionCallback.onFailure, but it's very arbitrary where
> different callbacks are called.

Yes, it is very arbitrary. The reason I would like to keep this call in this
file is that onCapturerStarted(true) is also called from this file.

https://codereview.webrtc.org/2187293002/diff/80001/webrtc/api/android/java/s...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:244: // Note:
stopPreview or other driver code might deadlock. Deadlock in
On 2016/08/11 12:57:13, magjed_webrtc wrote:
> If the app calls startCapture and stopCapture and the device deadlocks here,
> then the next startCapture/stopCapture will deadlock. Maybe we should add a
> timeout to CameraCapturer.stopCapture() for that reason to really make sure we
> never deadlock any app thread.
> 
> And BTW, can you remove this comment from CameraCapturer that is now out of
sync
> with the code:
> // Use postOnCameraThread() instead of posting directly to the handler - this
> way all
> // callbacks with a specifed token can be removed at once.

I'd rather handle this issue in a separate CL.

[magjed_webrtc, 2016-08-15 09:31:13]

lgtm if you address my comments in separate CLs.

[magjed_webrtc, 2016-08-30 18:02:12]

What's the status of this CL? Is anything blocking you from landing this?

[sakal, 2016-09-01 11:00:27]

I'd like a to do a bit more testing on the Camera2Session before landing this.
One bug was just found and fixed today.

[sakal, 2016-09-19 10:47:09]

Patchset #10 (id:200001) has been deleted

[sakal, 2016-09-19 13:22:24]

Patchset #10 (id:220001) has been deleted

[sakal, 2016-09-19 13:22:30]

Patchset #10 (id:240001) has been deleted

[sakal, 2016-09-19 13:27:16]

Patchset #10 (id:260001) has been deleted

[magjed_webrtc, 2016-09-19 14:32:16]

https://codereview.webrtc.org/2187293002/diff/300001/webrtc/api/android/java/...
File webrtc/api/android/java/src/org/webrtc/Camera1Session.java (right):

https://codereview.webrtc.org/2187293002/diff/300001/webrtc/api/android/java/...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:236:
events.onCameraError(Camera1Session.this, errorMessage);
Can we stop before notifying the callback, i.e. move this to the bottom of the
function?

[sakal, 2016-09-20 07:11:59]

https://codereview.webrtc.org/2187293002/diff/300001/webrtc/api/android/java/...
File webrtc/api/android/java/src/org/webrtc/Camera1Session.java (right):

https://codereview.webrtc.org/2187293002/diff/300001/webrtc/api/android/java/...
webrtc/api/android/java/src/org/webrtc/Camera1Session.java:236:
events.onCameraError(Camera1Session.this, errorMessage);
On 2016/09/19 14:32:16, magjed_webrtc wrote:
> Can we stop before notifying the callback, i.e. move this to the bottom of the
> function?

Done.

[magjed_webrtc, 2016-09-20 07:37:59]

lgtm

[sakal, 2016-09-20 10:45:12]

The CQ bit was checked by sakal@webrtc.org

[sakal, 2016-09-20 10:45:12]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2187293002/#ps360001 (title:
"Add missing setPictureSize.")

[commit-bot: I haz the power, 2016-09-20 10:45:26]

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-09-20 12:45:53]

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

[commit-bot: I haz the power, 2016-09-20 12:45:54]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[sakal, 2016-09-21 08:26:19]

The CQ bit was checked by sakal@webrtc.org

[commit-bot: I haz the power, 2016-09-21 08:26:23]

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-09-21 08:34:59]

Description was changed from

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148
==========

to

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148
==========

[commit-bot: I haz the power, 2016-09-21 08:35:00]

Committed patchset #14 (id:360001)

[commit-bot: I haz the power, 2016-09-21 08:35:05]

Description was changed from

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148
==========

to

==========
Session based capturing for Camera1Capturer.

BUG=webrtc:6148

Committed: https://crrev.com/3442579fd777a1b036a01e6d2bd53887dbddceed
Cr-Commit-Position: refs/heads/master@{#14318}
==========

[commit-bot: I haz the power, 2016-09-21 08:35:05]

Patchset 14 (id:??) landed as
https://crrev.com/3442579fd777a1b036a01e6d2bd53887dbddceed
Cr-Commit-Position: refs/heads/master@{#14318}