Add field trial to update quality scaler QP thresholds for Android HW encoder.

Add field trial to update quality scaler QP thresholds for Android HW encoder.

BUG=b/36034878

Review-Url: https://codereview.webrtc.org/2764143002
Cr-Commit-Position: refs/heads/master@{#17367}
Committed: https://chromium.googlesource.com/external/webrtc/+/d1c44356c5d6d572ed660b91af683ed0b136c0cd

[AlexG, 2017-03-21 21:30:23]

glaznev@webrtc.org changed reviewers:
+ kthelgason@webrtc.org

[AlexG, 2017-03-21 21:30:24]

PTAL

[AlexG, 2017-03-21 21:31:28]

glaznev@webrtc.org changed reviewers:
+ stefan@webrtc.org

[kthelgason, 2017-03-22 07:38:59]

lgtm

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
webrtc/modules/video_coding/utility/quality_scaler.cc:150: << " has been high ,
asking for lower resolution.";
There is a bug here (https://bugs.chromium.org/p/webrtc/issues/detail?id=7373)
about reducing the amount of logging done by the quality scaler. Some have
thought it's confusing that it will log even though resolution is at max/min.
Feel free to weigh in on the bug if you feel these log lines are useful.

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
File webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc:184: VideoCodecType
GetCodecType() const;
Thanks for cleaning this up!

[kthelgason, 2017-03-22 07:39:05]

The CQ bit was checked by kthelgason@webrtc.org

[commit-bot: I haz the power, 2017-03-22 07:39:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[kthelgason, 2017-03-22 07:39:16]

The CQ bit was unchecked by kthelgason@webrtc.org

[kthelgason, 2017-03-22 07:39:33]

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

[commit-bot: I haz the power, 2017-03-22 07:39:43]

Dry run: 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, 2017-03-22 08:05:12]

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

[commit-bot: I haz the power, 2017-03-22 08:05:13]

Dry run: This issue passed the CQ dry run.

[AlexG, 2017-03-22 19:53:45]

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
webrtc/modules/video_coding/utility/quality_scaler.cc:150: << " has been high ,
asking for lower resolution.";
On 2017/03/22 07:38:58, kthelgason wrote:
> There is a bug here (https://bugs.chromium.org/p/webrtc/issues/detail?id=7373)
> about reducing the amount of logging done by the quality scaler. Some have
> thought it's confusing that it will log even though resolution is at max/min.
> Feel free to weigh in on the bug if you feel these log lines are useful.

I am not sure regarding the proper logging here. Probably worth to address in a
separate CL

[AlexG, 2017-03-22 19:53:53]

The CQ bit was checked by glaznev@webrtc.org

[commit-bot: I haz the power, 2017-03-22 19:54:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[AlexG, 2017-03-22 19:57:04]

Stefan, can you please review quality_scaler.cc - I need owner of
modules/video_coding approve

[commit-bot: I haz the power, 2017-03-22 19:59:10]

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

[commit-bot: I haz the power, 2017-03-22 19:59:11]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/15252)

[stefan-webrtc, 2017-03-23 17:58:04]

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
webrtc/modules/video_coding/utility/quality_scaler.cc:150: << " has been high ,
asking for lower resolution.";
On 2017/03/22 19:53:45, AlexG wrote:
> On 2017/03/22 07:38:58, kthelgason wrote:
> > There is a bug here
(https://bugs.chromium.org/p/webrtc/issues/detail?id=7373)
> > about reducing the amount of logging done by the quality scaler. Some have
> > thought it's confusing that it will log even though resolution is at
max/min.
> > Feel free to weigh in on the bug if you feel these log lines are useful.
> 
> I am not sure regarding the proper logging here. Probably worth to address in
a
> separate CL

I think the current logging is confusing, so I'd probably leave it out if you
don't think it's really necessary. I It will periodically log this (and below)
throughout the call if it can't get a higher/lower resolution

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
File webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc:1197: ALOGD << "QP custom
thresholds: " << experiment_string << " for codec "
LOG(LS_INFO) or similar?

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc:1202: int
highH264QpThreshold;
No camelCase

[AlexG, 2017-03-23 19:25:09]

PTAL

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
File webrtc/modules/video_coding/utility/quality_scaler.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/modules/video_coding/u...
webrtc/modules/video_coding/utility/quality_scaler.cc:150: << " has been high ,
asking for lower resolution.";
On 2017/03/23 17:58:04, stefan-webrtc wrote:
> On 2017/03/22 19:53:45, AlexG wrote:
> > On 2017/03/22 07:38:58, kthelgason wrote:
> > > There is a bug here
> (https://bugs.chromium.org/p/webrtc/issues/detail?id=7373)
> > > about reducing the amount of logging done by the quality scaler. Some have
> > > thought it's confusing that it will log even though resolution is at
> max/min.
> > > Feel free to weigh in on the bug if you feel these log lines are useful.
> > 
> > I am not sure regarding the proper logging here. Probably worth to address
in
> a
> > separate CL
> 
> I think the current logging is confusing, so I'd probably leave it out if you
> don't think it's really necessary. I It will periodically log this (and below)
> throughout the call if it can't get a higher/lower resolution

Done. I removed "asking for lower/higher resolution logs". May still worth to
keep QP log to monitor average QP?

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
File webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc (right):

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc:1197: ALOGD << "QP custom
thresholds: " << experiment_string << " for codec "
On 2017/03/23 17:58:04, stefan-webrtc wrote:
> LOG(LS_INFO) or similar?

ALOGD is used now for all logging in this file - this was done to use custom TAG
for both Java and JNI code for easier filtering in logcat. May be moving to
LOG(LS_INFO) will make sense, but then we need to change all logging on this
file. This is probably out of scope for this CL.

https://codereview.webrtc.org/2764143002/diff/1/webrtc/sdk/android/src/jni/an...
webrtc/sdk/android/src/jni/androidmediaencoder_jni.cc:1202: int
highH264QpThreshold;
On 2017/03/23 17:58:04, stefan-webrtc wrote:
> No camelCase

Done.

[stefan-webrtc, 2017-03-23 20:08:15]

lgtm

[AlexG, 2017-03-23 21:05:41]

The CQ bit was checked by glaznev@webrtc.org

[AlexG, 2017-03-23 21:05:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
kthelgason@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2764143002/#ps20001 (title:
"Address comments")

[commit-bot: I haz the power, 2017-03-23 21:05:47]

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, 2017-03-23 21:40:08]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490303141451930,
"parent_rev": "f1dbf70ffcd3b0d2d959e7183d3a774678c44893", "commit_rev":
"d1c44356c5d6d572ed660b91af683ed0b136c0cd"}

[commit-bot: I haz the power, 2017-03-23 21:40:12]

Description was changed from

==========
Add field trial to update quality scaler QP thresholds for Android HW encoder.

BUG=b/36034878
==========

to

==========
Add field trial to update quality scaler QP thresholds for Android HW encoder.

BUG=b/36034878

Review-Url: https://codereview.webrtc.org/2764143002
Cr-Commit-Position: refs/heads/master@{#17367}
Committed:
https://chromium.googlesource.com/external/webrtc/+/d1c44356c5d6d572ed660b91a...
==========

[commit-bot: I haz the power, 2017-03-23 21:40:13]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/d1c44356c5d6d572ed660b91a...