Preliminary change for new rtc rendering algorithm

Preliminary change for new rtc rendering algorithm:

1. Add trace flags for smoothness testing

2. Propagate render_time calculated in Jitter Buffer to media::VideoFrame for new rendering algorithm to use.

3. Move invalidation trigger to compositor thread.


This is a recommit of CL 1265433003, the original CL was reverted because it breaks Blink Layout tests.

BUG=514873
TBR=sunnyps@chromium.org,mcasas@chromium.org,dalecurtis@chromium.org,miu@chromium.org,piman@chromium.org

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel;tryserver.blink:win_blink_rel

Committed: https://crrev.com/c1e6f1defc961774afa95af08c5fb457816e8492
Cr-Commit-Position: refs/heads/master@{#345200}

[qiangchen, 2015-08-21 19:07:35]

qiangchen@chromium.org changed reviewers:
+ pfeldman@chromium.org

[qiangchen, 2015-08-21 19:07:55]

The CQ bit was checked by qiangchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-21 19:09:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306203002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306203002/20001

[commit-bot: I haz the power, 2015-08-21 19:09:33]

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

[commit-bot: I haz the power, 2015-08-21 19:09:34]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[qiangchen, 2015-08-21 20:26:05]

qiangchen@chromium.org changed reviewers:
+ emircan@chromium.org

[qiangchen, 2015-08-24 16:26:06]

qiangchen@chromium.org changed reviewers:
- emircan@chromium.org

[pfeldman, 2015-08-24 17:47:07]

not lgtm

[qiangchen, 2015-08-24 20:02:38]

On 2015/08/24 17:47:07, pfeldman wrote:
> not lgtm

Can you take a look at the new patch again.

1. I think the failure of win_blink_oilpan_rel is not my fault, as the trunk
code also fails the same test.

2. I'm confused: for patch 4, the first time when I run mac_blink_rel, it fails;
but after I re-run the trybot, it passes the test.

3. For linux_chromium_asan_rel_ng, it may pass or fail randomly. If it fails,
the reason is for browser_tests, "some shards did not complete: K", where K is a
number not fixed, sometimes 0, sometimes 4 or 5.

Do you have an idea on the sometimes pass sometimes fail issue?

[pfeldman, 2015-08-24 20:16:49]

Ah, I misunderstood that Trunk try. Anyhow, you want someone from media to
review this.

[qiangchen, 2015-08-24 20:31:46]

On 2015/08/24 20:16:49, pfeldman wrote:
> Ah, I misunderstood that Trunk try. Anyhow, you want someone from media to
> review this.

The only file I changed in this CL is render_frame_impl.cc. So it makes sense
for the owner of that file to take a look again.

However,  piman@chromium.org the reviewer of my original CL and the owner of
render_frame_impl.cc is OOO till 8/31. Should we pass it to another content
owner?

[qiangchen, 2015-08-24 21:22:15]

qiangchen@chromium.org changed reviewers:
+ sievers@chromium.org

[qiangchen, 2015-08-24 21:25:43]

Daniel, can you take a look at the changes to render_frame_impl.cc. What we
essentially want to do is to pass the compositor task runner to
WebMediaPlayerMS. In the case that no compositor thread is used, we use the main
thread task runner as the compositor thread task runner.

Changes to other files have already been reviewed by dale curtis.

[qiangchen, 2015-08-24 21:45:23]

qiangchen@chromium.org changed reviewers:
+ mcasas@chromium.org

[mcasas, 2015-08-24 21:46:36]

lgtm

[qiangchen, 2015-08-24 21:47:03]

The CQ bit was checked by qiangchen@chromium.org

[commit-bot: I haz the power, 2015-08-24 21:47:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306203002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306203002/60001

[commit-bot: I haz the power, 2015-08-24 21:47:20]

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

[commit-bot: I haz the power, 2015-08-24 21:47:21]

A disapproval has been posted by following reviewers: pfeldman@chromium.org.
Please make sure to get positive review before another CQ attempt.

[qiangchen, 2015-08-24 21:48:33]

On 2015/08/24 17:47:07, pfeldman wrote:
> not lgtm

Can you revert this not lgtm?

[pfeldman, 2015-08-24 21:58:42]

lgtm

[qiangchen, 2015-08-24 22:01:27]

The CQ bit was checked by qiangchen@chromium.org

[commit-bot: I haz the power, 2015-08-24 22:01:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306203002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306203002/60001

[commit-bot: I haz the power, 2015-08-24 22:01:56]

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

[commit-bot: I haz the power, 2015-08-24 22:01:57]

Transient error: Too many
"tryserver.blink:linux_blink_rel,tryserver.blink:win_blink_rel" delimiters in
":".
Correct syntax is "tryserver:bot1,bot2;tryserver2:bot3,bot4;".

[qiangchen, 2015-08-24 22:04:07]

The CQ bit was checked by qiangchen@chromium.org

[commit-bot: I haz the power, 2015-08-24 22:04:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1306203002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1306203002/60001

[no sievers, 2015-08-24 22:27:44]

On 2015/08/24 21:25:43, qiangchenC wrote:
> Daniel, can you take a look at the changes to render_frame_impl.cc. What we
> essentially want to do is to pass the compositor task runner to
> WebMediaPlayerMS. In the case that no compositor thread is used, we use the
main
> thread task runner as the compositor thread task runner.
> 

lgtm

[commit-bot: I haz the power, 2015-08-24 23:09:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-08-24 23:10:15]

Patchset 4 (id:??) landed as
https://crrev.com/c1e6f1defc961774afa95af08c5fb457816e8492
Cr-Commit-Position: refs/heads/master@{#345200}

[Lei Zhang, 2015-08-25 02:33:14]

Please note, there's a bunch of procedural issues with this CL:

1) Even though you wrote "Preliminary change for new rtc rendering algorithm" as
the subject line, the description is what actually goes in the commit message,
and that's just "Recommit of CL 1265433003" which is slightly annoying because
it doesn't directly describe what this CL does.

2) This also doesn't include a BUG= field, which is relevant.

3) Not all the people you TBR'd are in the reviewers list or CC'd on the bug.
How will they know to review this without getting a notification?


Consider writing the description as:

Preliminary change for new rtc rendering algorithm

This is a recommit of CL 1265433003. The previous CL was reverted because ____.
This CL fixes ____ by ____.

BUG=514873

(and CC the folks you are TBRing, which you should still do)

[chromium-reviews, 2015-08-25 16:04:47]

Thanks, I've updated the description. Is there anything else for me to do
to make the description enter commit message?

On Mon, Aug 24, 2015 at 7:33 PM, <thestig@chromium.org> wrote:

> Please note, there's a bunch of procedural issues with this CL:
>
> 1) Even though you wrote "Preliminary change for new rtc rendering
> algorithm" as
> the subject line, the description is what actually goes in the commit
> message,
> and that's just "Recommit of CL 1265433003" which is slightly annoying
> because
> it doesn't directly describe what this CL does.
>
> 2) This also doesn't include a BUG= field, which is relevant.
>
> 3) Not all the people you TBR'd are in the reviewers list or CC'd on the
> bug.
> How will they know to review this without getting a notification?
>
>
> Consider writing the description as:
>
> Preliminary change for new rtc rendering algorithm
>
> This is a recommit of CL 1265433003. The previous CL was reverted because
> ____.
> This CL fixes ____ by ____.
>
> BUG=514873
>
> (and CC the folks you are TBRing, which you should still do)
>
> https://codereview.chromium.org/1306203002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[qiangchen, 2015-08-28 00:50:31]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1322463003/ by qiangchen@chromium.org.

The reason for reverting is: Crash for HW WebRTC.

[phoglund_chromium, 2015-08-28 07:11:35]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1321753002/ by phoglund@chromium.org.

The reason for reverting is: Speculative revert: WebRTC video seems to have
gotten choppier. Reverting to see if this patch is the culprit. See
http://crbug.com/525264..

[phoglund_chromium, 2015-08-28 07:14:04]

On 2015/08/28 07:11:35, phoglund_chrome wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/1321753002/ by mailto:phoglund@chromium.org.
> 
> The reason for reverting is: Speculative revert: WebRTC video seems to have
> gotten choppier. Reverting to see if this patch is the culprit. See
> http://crbug.com/525264..

Note: there was a revert by qiangchenC earlier, but it never got committed.