Vp9: Enable denoiser by default.

Vp9: Enable denoiser by default.

BUG=webrtc:7412
Review-Url: https://codereview.webrtc.org/2765663002
Cr-Commit-Position: refs/heads/master@{#17395}
Committed: https://chromium.googlesource.com/external/webrtc/+/a5e8aa6e2b9b8cdc6d7d5120fc64c08e4036bf03

[jianj, 2017-03-21 00:02:15]

The CQ bit was checked by jianj@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 00:02:16]

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-21 00:07:00]

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

[commit-bot: I haz the power, 2017-03-21 00:07:01]

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

[jianj, 2017-03-21 00:09:10]

The CQ bit was checked by jianj@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 00:09:20]

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

[marpan, 2017-03-21 00:09:48]

marpan@google.com changed reviewers:
+ brandtr@webrtc.org, mflodman@webrtc.org

[marpan, 2017-03-21 00:10:02]

marpan@google.com changed reviewers:
+ marpan@webrtc.org

[commit-bot: I haz the power, 2017-03-21 00:13:49]

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

[commit-bot: I haz the power, 2017-03-21 00:13:50]

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

[jianj, 2017-03-21 00:14:04]

The CQ bit was checked by jianj@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-21 00:14:09]

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

[jianj, 2017-03-21 00:15:48]

Description was changed from

==========
Vp9: Enable denoiser by default.
==========

to

==========
Vp9: Enable denoiser by default.

BUG=None
==========

[commit-bot: I haz the power, 2017-03-21 00:38:59]

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

[commit-bot: I haz the power, 2017-03-21 00:39:00]

Dry run: Try jobs failed on following builders:
  linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)

[brandtr, 2017-03-21 09:23:09]

On 2017/03/21 00:39:00, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)

Looks like a race between the ViEEncoder task queue and some internal VP9
thread?

We should have better test coverage at the video coding level, and then this
would have been found earlier. I believe that ilnik@ is working on a "quick
flag" for the full stack tests, which would allow those tests to be enabled on
the main waterfall (incl. tsan).

[jianj, 2017-03-21 23:08:28]

On 2017/03/21 09:23:09, brandtr wrote:
> On 2017/03/21 00:39:00, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)
> 
> Looks like a race between the ViEEncoder task queue and some internal VP9
> thread?
> 
> We should have better test coverage at the video coding level, and then this
> would have been found earlier. I believe that ilnik@ is working on a "quick
> flag" for the full stack tests, which would allow those tests to be enabled on
> the main waterfall (incl. tsan).

More unit tests covering that case have been added in libvpx, which exposed the
data race in the denoiser. 
And a fix has also been added.

We need to wait for the libvpx to roll in webrtc and then enable the vp9
denoiser.

[brandtr, 2017-03-22 07:46:30]

On 2017/03/21 23:08:28, jianj wrote:
> On 2017/03/21 09:23:09, brandtr wrote:
> > On 2017/03/21 00:39:00, commit-bot: I haz the power wrote:
> > > Dry run: Try jobs failed on following builders:
> > >   linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)
> > 
> > Looks like a race between the ViEEncoder task queue and some internal VP9
> > thread?
> > 
> > We should have better test coverage at the video coding level, and then this
> > would have been found earlier. I believe that ilnik@ is working on a "quick
> > flag" for the full stack tests, which would allow those tests to be enabled
on
> > the main waterfall (incl. tsan).
> 
> More unit tests covering that case have been added in libvpx, which exposed
the
> data race in the denoiser. 
> And a fix has also been added.
> 
> We need to wait for the libvpx to roll in webrtc and then enable the vp9
> denoiser.

Sounds good. Ping this CL when the roll has been made :)

[marpan, 2017-03-23 17:29:05]

The CQ bit was checked by marpan@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-23 17:29:11]

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-23 18:09:19]

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

[commit-bot: I haz the power, 2017-03-23 18:09:20]

Dry run: Try jobs failed on following builders:
  linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21131)

[jianj, 2017-03-23 18:43:51]

Patchset #4 (id:60001) has been deleted

[jianj, 2017-03-24 19:49:54]

The CQ bit was checked by jianj@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-24 19:50:00]

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

[jianj, 2017-03-24 19:50:28]

The CQ bit was unchecked by jianj@google.com

[jianj, 2017-03-24 20:17:01]

On 2017/03/22 07:46:30, brandtr wrote:
> On 2017/03/21 23:08:28, jianj wrote:
> > On 2017/03/21 09:23:09, brandtr wrote:
> > > On 2017/03/21 00:39:00, commit-bot: I haz the power wrote:
> > > > Dry run: Try jobs failed on following builders:
> > > >   linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)
> > > 
> > > Looks like a race between the ViEEncoder task queue and some internal VP9
> > > thread?
> > > 
> > > We should have better test coverage at the video coding level, and then
this
> > > would have been found earlier. I believe that ilnik@ is working on a
"quick
> > > flag" for the full stack tests, which would allow those tests to be
enabled
> on
> > > the main waterfall (incl. tsan).
> > 
> > More unit tests covering that case have been added in libvpx, which exposed
> the
> > data race in the denoiser. 
> > And a fix has also been added.
> > 
> > We need to wait for the libvpx to roll in webrtc and then enable the vp9
> > denoiser.
> 
> Sounds good. Ping this CL when the roll has been made :)

The roll has been made and all trybots are green now.

[brandtr, 2017-03-27 07:14:35]

On 2017/03/24 20:17:01, jianj wrote:
> On 2017/03/22 07:46:30, brandtr wrote:
> > On 2017/03/21 23:08:28, jianj wrote:
> > > On 2017/03/21 09:23:09, brandtr wrote:
> > > > On 2017/03/21 00:39:00, commit-bot: I haz the power wrote:
> > > > > Dry run: Try jobs failed on following builders:
> > > > >   linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
> > > > >
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/21030)
> > > > 
> > > > Looks like a race between the ViEEncoder task queue and some internal
VP9
> > > > thread?
> > > > 
> > > > We should have better test coverage at the video coding level, and then
> this
> > > > would have been found earlier. I believe that ilnik@ is working on a
> "quick
> > > > flag" for the full stack tests, which would allow those tests to be
> enabled
> > on
> > > > the main waterfall (incl. tsan).
> > > 
> > > More unit tests covering that case have been added in libvpx, which
exposed
> > the
> > > data race in the denoiser. 
> > > And a fix has also been added.
> > > 
> > > We need to wait for the libvpx to roll in webrtc and then enable the vp9
> > > denoiser.
> > 
> > Sounds good. Ping this CL when the roll has been made :)
> 
> The roll has been made and all trybots are green now.

lgtm

[jianj, 2017-03-27 07:16:21]

The CQ bit was checked by jianj@google.com

[commit-bot: I haz the power, 2017-03-27 07:16:31]

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-27 07:21:00]

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

[commit-bot: I haz the power, 2017-03-27 07:21:01]

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

[jianj, 2017-03-27 07:21:15]

The CQ bit was unchecked by jianj@google.com

[jianj, 2017-03-27 07:26:02]

On 2017/03/27 07:21:01, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   presubmit on master.tryserver.webrtc (JOB_FAILED,
> http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/15371)

I think I'll need another lgtm from owners.

[brandtr, 2017-03-27 07:28:12]

On 2017/03/27 07:26:02, jianj wrote:
> On 2017/03/27 07:21:01, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   presubmit on master.tryserver.webrtc (JOB_FAILED,
> >
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/15371)
> 
> I think I'll need another lgtm from owners.

Yep. mflodman has root-level OWNERship, so let's see what he says.

[mflodman, 2017-03-27 12:57:41]

Can you create a tracking bug for this and add for this CL? It would be good to
have so we get this in the release notes.

LGTM with that done.

[jianj, 2017-03-27 16:58:37]

Description was changed from

==========
Vp9: Enable denoiser by default.

BUG=None
==========

to

==========
Vp9: Enable denoiser by default.

BUG=webrtc:7412
==========

[jianj, 2017-03-27 17:06:25]

The CQ bit was checked by jianj@google.com

[commit-bot: I haz the power, 2017-03-27 17:06:29]

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-27 17:09:00]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490634385223480,
"parent_rev": "11173f7ec510ea0622e52655373c1c50599fb5fd", "commit_rev":
"a5e8aa6e2b9b8cdc6d7d5120fc64c08e4036bf03"}

[commit-bot: I haz the power, 2017-03-27 17:09:03]

Description was changed from

==========
Vp9: Enable denoiser by default.

BUG=webrtc:7412
==========

to

==========
Vp9: Enable denoiser by default.

BUG=webrtc:7412

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

[commit-bot: I haz the power, 2017-03-27 17:09:05]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/a5e8aa6e2b9b8cdc6d7d5120f...