Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(299)

Issue 1451953002: Preliminary support of VP9 HW encoder on Android. (Closed)

Created:
5 years, 1 month ago by AlexG
Modified:
5 years, 1 month ago
Reviewers:
magjed_webrtc
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@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Preliminary support of VP9 HW encoder on Android. Not fully tested yet. Verified in test loopback application with fake VP9 codec factory. Assume that encoder generates bitstream in non flexible mode with one temporal and one spatial layers. R=magjed@webrtc.org Committed: https://crrev.com/ad948c42a1fd29bf22205ded2a175a967748abe4 Cr-Commit-Position: refs/heads/master@{#10695}

Patch Set 1 #

Patch Set 2 : Use warnings in log #

Total comments: 7

Patch Set 3 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -8 lines) Patch
M talk/app/webrtc/java/jni/androidmediadecoder_jni.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M talk/app/webrtc/java/jni/androidmediaencoder_jni.cc View 1 2 9 chunks +51 lines, -5 lines 0 comments Download
M talk/app/webrtc/java/src/org/webrtc/MediaCodecVideoEncoder.java View 3 chunks +12 lines, -1 line 0 comments Download

Messages

Total messages: 16 (6 generated)
AlexG
PTAL
5 years, 1 month ago (2015-11-16 22:30:17 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1451953002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1451953002/20001
5 years, 1 month ago (2015-11-16 22:30:26 UTC) #4
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 1 month ago (2015-11-16 23:20:26 UTC) #6
magjed_webrtc
lgtm https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode320 talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:320: scale_ = webrtc::field_trial::FindFullName( I would prefer scale_ = ...
5 years, 1 month ago (2015-11-18 13:28:24 UTC) #7
AlexG
https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right): https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/androidmediaencoder_jni.cc#newcode320 talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:320: scale_ = webrtc::field_trial::FindFullName( On 2015/11/18 13:28:24, magjed_webrtc wrote: > ...
5 years, 1 month ago (2015-11-18 19:03:51 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1451953002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1451953002/40001
5 years, 1 month ago (2015-11-18 19:05:17 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
5 years, 1 month ago (2015-11-18 21:05:29 UTC) #13
AlexG
Committed patchset #3 (id:40001) manually as ad948c42a1fd29bf22205ded2a175a967748abe4 (presubmit successful).
5 years, 1 month ago (2015-11-18 21:06:57 UTC) #14
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/ad948c42a1fd29bf22205ded2a175a967748abe4 Cr-Commit-Position: refs/heads/master@{#10695}
5 years, 1 month ago (2015-11-18 21:06:59 UTC) #15
magjed_webrtc
5 years, 1 month ago (2015-11-19 08:10:03 UTC) #16
Message was sent while issue was closed.
https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/...
File talk/app/webrtc/java/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/1451953002/diff/20001/talk/app/webrtc/java/jni/...
talk/app/webrtc/java/jni/androidmediaencoder_jni.cc:888: if (codecType_ ==
kVideoCodecVP8 && scale_) {
On 2015/11/18 19:03:51, AlexG wrote:
> On 2015/11/18 13:28:24, magjed_webrtc wrote:
> > Revert this change, and only check |scale_|.
> 
> I think it is a little cleaner to keep this check - when we call vp8 specific
QP
> parsing we should also check that VP8 is selected. This part of code should
not
> assume that scale_ is always false for VP9

I see, I didn't realize it was vp8 specific scaling.

Powered by Google App Engine
This is Rietveld 408576698