Android SurfaceTextureHelper: Add stopListening() function

Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519
Committed: https://crrev.com/81e8e374ecfedaa3753518f488329b74c5bccf82
Cr-Commit-Position: refs/heads/master@{#11855}

[magjed_webrtc, 2016-03-02 08:13:18]

Patchset #1 (id:1) has been deleted

[magjed_webrtc, 2016-03-02 08:15:23]

Description was changed from

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). Then we want to call startListening() in
startCapture() and stopListening() in stopCapture().

BUG=webrtc:5519
==========

to

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519
==========

[magjed_webrtc, 2016-03-02 08:15:49]

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

[magjed_webrtc, 2016-03-02 08:16:07]

perkj@ - Please take a look.

[perkj_webrtc, 2016-03-02 12:08:46]

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
File webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java
(right):

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java:274:
private static void stopListeningOnHandlerThread(final SurfaceTextureHelper
surfaceTextureHelper)
Should this be done in SurfaceTextureHelper.stopListen if we are on a thread !=
getHandler() ? 
It seems a bit strange to be able to call startListen on any thread but
stopListen on an internal thread.

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/java/android/o...
File webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java (right):

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/java/android/o...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:354: if
(handler.getLooper().getThread() != Thread.currentThread()) {
Move helper code from the test here to allow this to be called on the same
thread as startListening?

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/java/android/o...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:403: public void
disconnect() {
Can this also be renamed dispose to make it clearer the difference to
stopListening?

[magjed_webrtc, 2016-03-02 19:38:54]

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
File webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java
(right):

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java:274:
private static void stopListeningOnHandlerThread(final SurfaceTextureHelper
surfaceTextureHelper)
On 2016/03/02 12:08:46, perkj_webrtc wrote:
> Should this be done in SurfaceTextureHelper.stopListen if we are on a thread
!=
> getHandler() ? 
> It seems a bit strange to be able to call startListen on any thread but
> stopListen on an internal thread.

The reason for only allowing stopListening() to be called on the handler thread
is to avoid deadlock prone locking and/or blocking while still guaranteeing that
onTextureFrameAvailable() won't be called after the function returns. This
helper function provides the convenience of being able to call it on any thread,
but it's also deadlock prone. I have looked at the tests to make sure they won't
deadlock from this, so I think it's ok, but this kind of deadlock prone helper
function is not something I want to provide in the production code unless it's
needed (and it's not needed (yet)).

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/java/android/o...
File webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java (right):

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/java/android/o...
webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java:403: public void
disconnect() {
On 2016/03/02 12:08:46, perkj_webrtc wrote:
> Can this also be renamed dispose to make it clearer the difference to
> stopListening?
> 

Done.

[perkj_webrtc, 2016-03-03 08:58:19]

lgtm

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
File webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java
(right):

https://codereview.webrtc.org/1755573002/diff/20001/webrtc/api/androidtests/s...
webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java:274:
private static void stopListeningOnHandlerThread(final SurfaceTextureHelper
surfaceTextureHelper)
On 2016/03/02 19:38:53, magjed_webrtc wrote:
> On 2016/03/02 12:08:46, perkj_webrtc wrote:
> > Should this be done in SurfaceTextureHelper.stopListen if we are on a thread
> !=
> > getHandler() ? 
> > It seems a bit strange to be able to call startListen on any thread but
> > stopListen on an internal thread.
> 
> The reason for only allowing stopListening() to be called on the handler
thread
> is to avoid deadlock prone locking and/or blocking while still guaranteeing
that
> onTextureFrameAvailable() won't be called after the function returns. This
> helper function provides the convenience of being able to call it on any
thread,
> but it's also deadlock prone. I have looked at the tests to make sure they
won't
> deadlock from this, so I think it's ok, but this kind of deadlock prone helper
> function is not something I want to provide in the production code unless it's
> needed (and it's not needed (yet)).

fair enough. It is documented and throws an exception. So even if it might look
strange- a client can fix it itself...

[magjed_webrtc, 2016-03-03 09:14:02]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2016-03-03 09:14:13]

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

[commit-bot: I haz the power, 2016-03-03 10:11:46]

Description was changed from

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519
==========

to

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519
==========

[commit-bot: I haz the power, 2016-03-03 10:11:47]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-03-03 10:18:52]

Description was changed from

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519
==========

to

==========
Android SurfaceTextureHelper: Add stopListening() function

This CL replaces the function SurfaceTextureHelper.setListener() that
could only be called once with the functions startListening() and
stopListening() that can be called multiple times. This is necessary
when the SurfaceTextureHelper will be passed to the VideoCapturerAndroid
in startCapture(). startListening() will be called in startCapture() and
stopListening() in stopCapture().

BUG=webrtc:5519

Committed: https://crrev.com/81e8e374ecfedaa3753518f488329b74c5bccf82
Cr-Commit-Position: refs/heads/master@{#11855}
==========

[commit-bot: I haz the power, 2016-03-03 10:18:53]

Patchset 2 (id:??) landed as
https://crrev.com/81e8e374ecfedaa3753518f488329b74c5bccf82
Cr-Commit-Position: refs/heads/master@{#11855}