Reland "Prepare MediaCodecVideoEncoder for surface textures.""

Reland "Prepare MediaCodecVideoEncoder for surface textures.""

This reverts commit 12f680214e28dc5f0a13ac8afc0d1445f89e67e6.
Original cl in https://codereview.webrtc.org/1396073003/
Prepare MediaCodecVideoEncoder for surface textures.
This refactors MediaVideoEncoder to prepare for adding support to encode from textures. The C++ layer does not have any functional changes.
- Moves ResetEncoder to always work on the codec thread
- Adds use of ThreadChecker.
- Change Java MediaEncoder.Init to return true or false and introduce method getInputBuffers.
- Add simple unit test for Java MediaCodecVideoEncoder.

The pure revert of the revert is in patchset 1.
Patchset 2, moves getting the input buffer to before storing pending timestamps etc to fix b/24984012.

BUG=webrtc:4993
b/24984012

Committed: https://crrev.com/9576e548368b34e150c3e6d19e889de9f0f67e96
Cr-Commit-Position: refs/heads/master@{#10622}

[perkj_webrtc, 2015-10-16 12:21:31]

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

[perkj_webrtc, 2015-10-16 12:30:31]

Can you please review?

[AlexG, 2015-10-19 23:38:45]

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:390:
RTC_DCHECK(codec_thread_checker_.CalledOnValidThread());
RTC_DCHECK -> RTC_CHECK. We probably should fail on release build as well here.

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:410:
RTC_DCHECK(codec_thread_checker_.CalledOnValidThread());
ditto: RTC_DCHECK -> RTC_CHECK and below

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:415: // TODO(fischman):
wouldn't it be nice if there was a way to gracefully
May be it's a good time now to make Reset to return boolean as well and if it
fails return WEBRTC_VIDEO_CODEC_ERROR

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:533: ALOGD << "Encoder drop
frame - failed callback.";
ALOGW?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:571: ALOGD <<"Encoder drop
frame - no input buffers available";
nit: whitespace after "<<" + ALOGD -> ALOGW?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:575: return true;  //
TODO(fischman): see webrtc bug 2887.
Why true? This function should return WEBRTC_VIDEO_CODEC_* ?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:577: if
(j_input_buffer_index == -2) {
 ResetCodecOnCodecThread() ?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:578: return false;
return WEBRTC_VIDEO_CODEC_ERROR?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/src/...
File talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java (right):

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java:247: colorFormat
= properties.colorFormat;
Print color format in the log for debug purposes?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java:267:
outputBuffers = mediaCodec.getOutputBuffers();
Print outputBuffers.length for debug purposes?

[perkj_webrtc, 2015-10-21 04:35:43]

Patchset #3 (id:40001) has been deleted

[perkj_webrtc, 2015-10-21 04:36:29]

ptal

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:390:
RTC_DCHECK(codec_thread_checker_.CalledOnValidThread());
On 2015/10/19 23:38:44, AlexG wrote:
> RTC_DCHECK -> RTC_CHECK. We probably should fail on release build as well
here.

The thread_checker doesnt do anything in release, same as
chrome.https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc/base/thread_checker.h&l=26

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:410:
RTC_DCHECK(codec_thread_checker_.CalledOnValidThread());
On 2015/10/19 23:38:45, AlexG wrote:
> ditto: RTC_DCHECK -> RTC_CHECK and below

dito

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:415: // TODO(fischman):
wouldn't it be nice if there was a way to gracefully
On 2015/10/19 23:38:44, AlexG wrote:
> May be it's a good time now to make Reset to return boolean as well and if it
> fails return WEBRTC_VIDEO_CODEC_ERROR

Ok. But the whole thing is a bit weird. It seems like Encode( errors are ignored
in the upper layers. 
Also, DelverOutPutPendingBuffers call this method, but still
ResetCodecOnCodecThread is called in Encode( if Deliver fails. I guess the
reason is the other path where DeliverPendingBuffers is called from
OnProcess....

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:533: ALOGD << "Encoder drop
frame - failed callback.";
On 2015/10/19 23:38:44, AlexG wrote:
> ALOGW?

Done.

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:571: ALOGD <<"Encoder drop
frame - no input buffers available";
On 2015/10/19 23:38:44, AlexG wrote:
> nit: whitespace after "<<" + ALOGD -> ALOGW?

Done.

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:575: return true;  //
TODO(fischman): see webrtc bug 2887.
On 2015/10/19 23:38:44, AlexG wrote:
> Why true? This function should return WEBRTC_VIDEO_CODEC_* ?

darn. Yes... 
Is there an easy way to trigger this and test?

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:577: if
(j_input_buffer_index == -2) {
On 2015/10/19 23:38:44, AlexG wrote:
>  ResetCodecOnCodecThread() ?

Done.

https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:578: return false;
On 2015/10/19 23:38:44, AlexG wrote:
> return WEBRTC_VIDEO_CODEC_ERROR?

Done.

[AlexG, 2015-10-22 00:04:33]

lgtm

[magjed_webrtc, 2015-10-22 23:24:17]

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:579:
ResetCodecOnCodecThread();
Should we return WEBRTC_VIDEO_CODEC_ERROR even if ResetCodecOnCodecThread() is
successful?

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:597: current_timestamp_us_
+= 1000000 / last_set_fps_;
s/1000000/rtc::kNumMicrosecsPerSec/g

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/src/...
File talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java (right):

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java:108:
MediaCodecVideoEncoder() {
Can't we just remove this constructor?

[perkj_webrtc, 2015-11-12 13:25:22]

magjed, please?

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:579:
ResetCodecOnCodecThread();
On 2015/10/22 23:24:17, magjed_webrtc wrote:
> Should we return WEBRTC_VIDEO_CODEC_ERROR even if ResetCodecOnCodecThread() is
> successful?

Yes, it will give the upper layer the chance to fallback to a software encoder.
This is not a change in this cl.

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:597: current_timestamp_us_
+= 1000000 / last_set_fps_;
On 2015/10/22 23:24:17, magjed_webrtc wrote:
> s/1000000/rtc::kNumMicrosecsPerSec/g

Done.

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/src/...
File talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java (right):

https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/src/...
talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java:108:
MediaCodecVideoEncoder() {
On 2015/10/22 23:24:17, magjed_webrtc wrote:
> Can't we just remove this constructor?

Done.

[perkj_webrtc, 2015-11-12 13:30:26]

The CQ bit was checked by perkj@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 13:30:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1406203002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1406203002/120001

[magjed_webrtc, 2015-11-12 13:56:19]

lgtm

[commit-bot: I haz the power, 2015-11-12 14:41:16]

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

[commit-bot: I haz the power, 2015-11-12 14:41:17]

Dry run: This issue passed the CQ dry run.

[perkj_webrtc, 2015-11-12 14:42:02]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2015-11-12 14:42:03]

The patchset sent to the CQ was uploaded after l-g-t-m from glaznev@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1406203002/#ps120001 (title:
"Addressed magjeds comments.")

[commit-bot: I haz the power, 2015-11-12 14:42:13]

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

[commit-bot: I haz the power, 2015-11-12 14:43:20]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-11-12 14:43:33]

Patchset 6 (id:??) landed as
https://crrev.com/9576e548368b34e150c3e6d19e889de9f0f67e96
Cr-Commit-Position: refs/heads/master@{#10622}