Reland of write frame generator capturer to use TaskQueue instead of EventTimeWrapper

Reland of write frame generator capturer to use TaskQueue instead of EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/ )

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of EventTimeWrapper (patchset #2 id:90001 of https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/ )
> > 
> > And enable large full-stack test depending on that change (Reland of https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301, webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33fa4b3538d8160a0f
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301, webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed: https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb30712cefe2639f88

BUG=webrtc:7301, webrtc:7325
Review-Url: https://codereview.webrtc.org/2750473002
Cr-Commit-Position: refs/heads/master@{#17253}
Committed: https://chromium.googlesource.com/external/webrtc/+/2549ad4fef419a055671c5be39fcfa27ba573c3b

[ilnik, 2017-03-13 09:19:22]

Created Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper

[ilnik, 2017-03-13 09:20:44]

Description was changed from

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7301,webrtc:7325
==========

to

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7301,webrtc:7325
==========

[ilnik, 2017-03-13 09:20:44]

ilnik@webrtc.org changed reviewers:
- sprang@webrtc.org, stefan@webrtc.org, tommi@webrtc.org

[ilnik, 2017-03-13 09:31:00]

ilnik@webrtc.org changed reviewers:
+ sprang@webrtc.org, stefan@webrtc.org, tommi@webrtc.org

[ilnik, 2017-03-13 09:35:14]

The CQ bit was checked by ilnik@webrtc.org

[ilnik, 2017-03-13 09:35:18]

The CQ bit was unchecked by ilnik@webrtc.org

[ilnik, 2017-03-13 09:35:27]

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

[commit-bot: I haz the power, 2017-03-13 09:35:32]

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-13 09:35:34]

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

[commit-bot: I haz the power, 2017-03-13 09:35:35]

Dry run: This issue passed the CQ dry run.

[ilnik, 2017-03-13 09:36:39]

Description was changed from

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:7301,webrtc:7325
==========

to

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

BUG=webrtc:7301,webrtc:7325
==========

[ilnik, 2017-03-13 09:36:52]

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

[commit-bot: I haz the power, 2017-03-13 09:37:02]

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-13 10:30:36]

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

[commit-bot: I haz the power, 2017-03-13 10:30:36]

Dry run: This issue passed the CQ dry run.

[tommi, 2017-03-13 10:35:08]

lgtm

[ilnik, 2017-03-13 17:00:24]

On 2017/03/13 10:35:08, tommi (webrtc) wrote:
> lgtm

Tommi, I have added little fix for TaskQueue on windows. Please, take a look.
The issue is: if queue is starved, then delayed tasks may be delayed
indefinitely. Problem is that it's a different behavior from another platforms
and it causes some tests to fail.
More specifically, It may break overuse detection (and
CallPerfTest.ReceivesCpuOveruseAndUnderuse fails) on Windows. Overuse checks are
posted to EncoderQueue as delayed tasks. However, since we are overusing CPU
there always are posted tasks to execute and therefore there always are
WM_RUN_TASK messages in the working thread queue. I still don't get why WM_TIMER
messages are not sent. In the end CheckOveruse task is not executed until frame
generator is stopped at the end of the test then it already failed.

I believe it would be more fair if queue tried to run scheduled tasks first, as
they were posted earlier. It also will make behavior uniform across different
platforms.

[tommi, 2017-03-13 20:03:39]

On 2017/03/13 17:00:24, ilnik wrote:
> On 2017/03/13 10:35:08, tommi (webrtc) wrote:
> > lgtm
> 
> Tommi, I have added little fix for TaskQueue on windows. Please, take a look.
> The issue is: if queue is starved, then delayed tasks may be delayed
> indefinitely. Problem is that it's a different behavior from another platforms
> and it causes some tests to fail.
> More specifically, It may break overuse detection (and
> CallPerfTest.ReceivesCpuOveruseAndUnderuse fails) on Windows. Overuse checks
are
> posted to EncoderQueue as delayed tasks. However, since we are overusing CPU
> there always are posted tasks to execute and therefore there always are
> WM_RUN_TASK messages in the working thread queue. I still don't get why
WM_TIMER
> messages are not sent. In the end CheckOveruse task is not executed until
frame
> generator is stopped at the end of the test then it already failed.
> 
> I believe it would be more fair if queue tried to run scheduled tasks first,
as
> they were posted earlier. It also will make behavior uniform across different
> platforms.

I don't understand exactly what you mean by 'starved'. Can you ping me when you
have time so that we can chat?

[ilnik, 2017-03-15 14:09:28]

On 2017/03/13 20:03:39, tommi (webrtc) wrote:
> On 2017/03/13 17:00:24, ilnik wrote:
> > On 2017/03/13 10:35:08, tommi (webrtc) wrote:
> > > lgtm
> > 
> > Tommi, I have added little fix for TaskQueue on windows. Please, take a
look.
> > The issue is: if queue is starved, then delayed tasks may be delayed
> > indefinitely. Problem is that it's a different behavior from another
platforms
> > and it causes some tests to fail.
> > More specifically, It may break overuse detection (and
> > CallPerfTest.ReceivesCpuOveruseAndUnderuse fails) on Windows. Overuse checks
> are
> > posted to EncoderQueue as delayed tasks. However, since we are overusing CPU
> > there always are posted tasks to execute and therefore there always are
> > WM_RUN_TASK messages in the working thread queue. I still don't get why
> WM_TIMER
> > messages are not sent. In the end CheckOveruse task is not executed until
> frame
> > generator is stopped at the end of the test then it already failed.
> > 
> > I believe it would be more fair if queue tried to run scheduled tasks first,
> as
> > they were posted earlier. It also will make behavior uniform across
different
> > platforms.
> 
> I don't understand exactly what you mean by 'starved'. Can you ping me when
you
> have time so that we can chat?

All changes after LGTM from Tommi@ are reverted. Queue issues are resolved in
https://codereview.webrtc.org/2750853002 .

[ilnik, 2017-03-15 14:09:38]

The CQ bit was checked by ilnik@webrtc.org

[ilnik, 2017-03-15 14:09:39]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2750473002/#ps300001 (title:
"Do not modify QueueTask, as it will be fixed by another CL by Tommi@")

[commit-bot: I haz the power, 2017-03-15 14:09:42]

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-15 14:48:54]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1489586978802480,
"parent_rev": "919dce22d5414761b1ff1a93de3e6ab6078ae3a5", "commit_rev":
"2549ad4fef419a055671c5be39fcfa27ba573c3b"}

[commit-bot: I haz the power, 2017-03-15 14:48:57]

Description was changed from

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

BUG=webrtc:7301,webrtc:7325
==========

to

==========
Reland of write frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2748643002/
)

Reason for revert:
Reland with fixes to the failing perf tests.

Original issue's description:
> Revert of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #2 id:90001 of
https://codereview.webrtc.org/2744003002/ )
> 
> Reason for revert:
> CallPerfTest.ReceivesCpuOveruseAndUnderuse perf test fails due to this CL. It
requires very accurate frame rate, which may not be so accurate now.
> 
> Original issue's description:
> > Reland of rewrite frame generator capturer to use TaskQueue instead of
EventTimeWrapper (patchset #1 id:1 of https://codereview.webrtc.org/2743993002/
)
> > 
> > And enable large full-stack test depending on that change (Reland of
https://codereview.webrtc.org/2741823003/)
> > TBR=stefan@webrtc.org,tommi@webrtc.org
> > BUG=webrtc:7301,webrtc:7325
> > 
> > Review-Url: https://codereview.webrtc.org/2744003002
> > Cr-Commit-Position: refs/heads/master@{#17196}
> > Committed:
https://chromium.googlesource.com/external/webrtc/+/8c0a5896d1cdc7bb81307c33f...
> 
> TBR=stefan@webrtc.org,tommi@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:7301,webrtc:7325
> 
> Review-Url: https://codereview.webrtc.org/2748643002
> Cr-Commit-Position: refs/heads/master@{#17198}
> Committed:
https://chromium.googlesource.com/external/webrtc/+/382a72a0d321f19ac9cdc9bb3...

BUG=webrtc:7301,webrtc:7325

Review-Url: https://codereview.webrtc.org/2750473002
Cr-Commit-Position: refs/heads/master@{#17253}
Committed:
https://chromium.googlesource.com/external/webrtc/+/2549ad4fef419a055671c5be3...
==========

[commit-bot: I haz the power, 2017-03-15 14:48:58]

Committed patchset #11 (id:300001) as
https://chromium.googlesource.com/external/webrtc/+/2549ad4fef419a055671c5be3...

[ilnik, 2017-03-16 15:52:20]

A revert of this CL (patchset #11 id:300001) has been created in
https://codereview.webrtc.org/2751063005/ by ilnik@webrtc.org.

The reason for reverting is: Changes to frame-generator resulted in reduced fps
on android and Mac on all tests..

[ilnik, 2017-03-17 10:47:10]

A revert of this CL (patchset #11 id:300001) has been created in
https://codereview.webrtc.org/2759623002/ by ilnik@webrtc.org.

The reason for reverting is: Reland with fixes to reduced frame-rate on Windows
and Android..