Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(32)

Issue 1406203002: Reland "Prepare MediaCodecVideoEncoder for surface textures."" (Closed)

Created:
3 years, 8 months ago by perkj_webrtc
Modified:
3 years, 7 months ago
Reviewers:
magjed_webrtc, AlexG
CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), Andrew MacDonald, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

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}

Patch Set 1 #

Patch Set 2 : Moved getting inputbuffer to before storing pending frames. #

Total comments: 18

Patch Set 3 : Addressed review comments. #

Patch Set 4 : Rebased #

Total comments: 6

Patch Set 5 : Rebased #

Patch Set 6 : Addressed magjeds comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+254 lines, -105 lines) Patch
A talk/app/webrtc/androidtests/src/org/webrtc/MediaCodecVideoEncoderTest.java View 1 chunk +95 lines, -0 lines 0 comments Download
M talk/app/webrtc/java/jni/androidmediaencoder_jni.cc View 1 2 3 4 5 21 chunks +125 lines, -75 lines 0 comments Download
M talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java View 1 2 3 4 5 10 chunks +34 lines, -30 lines 0 comments Download

Messages

Total messages: 18 (6 generated)
perkj_webrtc
Can you please review?
3 years, 8 months ago (2015-10-16 12:30:31 UTC) #2
AlexG
https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode390 talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:390: RTC_DCHECK(codec_thread_checker_.CalledOnValidThread()); RTC_DCHECK -> RTC_CHECK. We probably should fail on ...
3 years, 8 months ago (2015-10-19 23:38:45 UTC) #3
perkj_webrtc
ptal https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1406203002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode390 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 ...
3 years, 8 months ago (2015-10-21 04:36:29 UTC) #5
AlexG
lgtm
3 years, 8 months ago (2015-10-22 00:04:33 UTC) #6
magjed_webrtc
https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode579 talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:579: ResetCodecOnCodecThread(); Should we return WEBRTC_VIDEO_CODEC_ERROR even if ResetCodecOnCodecThread() is ...
3 years, 8 months ago (2015-10-22 23:24:17 UTC) #7
perkj_webrtc
magjed, please? https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1406203002/diff/80001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode579 talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:579: ResetCodecOnCodecThread(); On 2015/10/22 23:24:17, magjed_webrtc wrote: > ...
3 years, 7 months ago (2015-11-12 13:25:22 UTC) #8
commit-bot: I haz the power
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
3 years, 7 months ago (2015-11-12 13:30:32 UTC) #10
magjed_webrtc
lgtm
3 years, 7 months ago (2015-11-12 13:56:19 UTC) #11
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
3 years, 7 months ago (2015-11-12 14:41:17 UTC) #13
commit-bot: I haz the power
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
3 years, 7 months ago (2015-11-12 14:42:13 UTC) #16
commit-bot: I haz the power
Committed patchset #6 (id:120001)
3 years, 7 months ago (2015-11-12 14:43:20 UTC) #17
commit-bot: I haz the power
3 years, 7 months ago (2015-11-12 14:43:33 UTC) #18
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/9576e548368b34e150c3e6d19e889de9f0f67e96
Cr-Commit-Position: refs/heads/master@{#10622}

Powered by Google App Engine
This is Rietveld 408576698