The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ddf895c72c8c5fe77f1f88698eaeaadf93b880e

commit 5ddf895c72c8c5fe77f1f88698eaeaadf93b880e
Author: emircan <emircan@chromium.org>
Date: Tue Sep 20 18:18:24 2016

Change WebMediaPlayerMS layers transparency dynamically

This CL changes how WebMediaPlayerMS sets its transparency
based on the incoming frame formats.

BUG= 647886 

Review-Url: https://codereview.chromium.org/2348903003
Cr-Commit-Position: refs/heads/master@{#419816}

[modify] https://crrev.com/5ddf895c72c8c5fe77f1f88698eaeaadf93b880e/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/5ddf895c72c8c5fe77f1f88698eaeaadf93b880e/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/5ddf895c72c8c5fe77f1f88698eaeaadf93b880e/content/renderer/media/webmediaplayer_ms_unittest.cc

M53 is already in Stable and bar is VERY high. Also this change is NOT yet baked/verified in Canary/Beta. So it is unlikely we can take this change for next M53 respin if any for Desktop unless it is absolutely critical to take.

This is a fix for a critical issue https://b.corp.google.com/u/0/issues/29285690

cc'ing others who can vouch for it.

Regarding #7, this is mainly needed for ChromeOS at the moment specifically for Chromebox for Meetings boards.
Panther
Guado
Zako
Rikku
Tricky
Stumpy
Buddy

Per comment #11 is mainly needed for ChromeOS, +ketakid@ for M53 merge review/approval.

[Automated comment] Request affecting a post-stable build (M53), manual review required.

Your change meets the bar and is auto-approved for M54 (branch: 2840)

Please confirm whether this change is baked/verified in Canary and safe to merge?If yes, merge your change to M54 (branch: 2840) ASAP so that we could take this for next Beta Release.

This is not in canary yet. https://chromium.googlesource.com/chromium/src/+log/55.0.2862.3..55.0.2866.0?pretty=fuller&n=10000

ligimole@, I have the merge CL ready on https://codereview.chromium.org/2358903002/. However, as harpreet@ pointed out above, it hasn't baked in canary yet.

Also note that this has already been locally patched and thoroughly tested in M53 8530.89.0 / 53.0.2785.128

We can do a local patch on M54 8743.35.0 / 54.0.2840.33 and test it, if that helps.

Thanks for the updates. 

This weeks Beta is already shipped today, next RC cut is scheduled on 09/26, so please verify in tomorrows canary and merge the patch if all looks good.

No new chrome today so the patch is still not in canary for chromeOS.


Though we tested the fix on the latest M54 (8743.37.0 / 54.0.2840.35 patched with Emircan's fix). The numbers look better with the fix. Here are the dropped frame numbers from a 10 minute session with 10 participants:


Devices with the fix:
100.96.48.34 - Dropped frames: 180 Encoded frames: 22862 => 0.0078125
100.96.48.35 - Dropped frames: 456 Encoded frames: 25484 => 0.0175811


Devices without the fix:
100.96.48.105 - Dropped frames: 1384 Encoded frames: 22807 => 0.0572232
100.96.49.156 - Dropped frames: 1476 Encoded frames: 20647 => 0.0667209

I'm ok with the merge from CrOS providing no issues are seen on other platforms (canary)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31fafc9cebde8343c7bee95675dcb2b92b9c8720

commit 31fafc9cebde8343c7bee95675dcb2b92b9c8720
Author: emircan <emircan@chromium.org>
Date: Fri Sep 23 18:53:42 2016

Change WebMediaPlayerMS layers transparency dynamically

This CL changes how WebMediaPlayerMS sets its transparency
based on the incoming frame formats.

BUG= 647886 

Review-Url: https://codereview.chromium.org/2348903003
Cr-Commit-Position: refs/heads/master@{#419816}
(cherry picked from commit 5ddf895c72c8c5fe77f1f88698eaeaadf93b880e)

NOTRY=true
NOPRESUBMIT=true
TBR=mcasas@chromium.org

Review-Url: https://codereview.chromium.org/2358903002
Cr-Commit-Position: refs/branch-heads/2840@{#515}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms_unittest.cc

Sorry, I missed the call in comment 8 to weigh in on the importance of this fix.

My internal motivation is added in:  https://b.corp.google.com/u/0/issues/29285690#comment151

Approving merge to M53 cros.

Thanks. ketakid@, can you point me to the branch for merge?

I merged it to M53(branch:2785) here: https://codereview.chromium.org/2373073002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f3bae8a353c97102ab7737f7ed3baf1e3be1d0e

commit 7f3bae8a353c97102ab7737f7ed3baf1e3be1d0e
Author: emircan <emircan@chromium.org>
Date: Tue Sep 27 18:16:37 2016

[Merge M-53] Change WebMediaPlayerMS layers transparency dynamically

This CL changes how WebMediaPlayerMS sets its transparency
based on the incoming frame formats.

BUG= 647886 

Review-Url: https://codereview.chromium.org/2348903003
Cr-Commit-Position: refs/heads/master@{#419816}
(cherry picked from commit 5ddf895c72c8c5fe77f1f88698eaeaadf93b880e)

NOTRY=true
NOPRESUBMIT=true
TBR=mcasas@chromium.org

Review-Url: https://codereview.chromium.org/2373073002
Cr-Commit-Position: refs/branch-heads/2785@{#934}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/7f3bae8a353c97102ab7737f7ed3baf1e3be1d0e/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/7f3bae8a353c97102ab7737f7ed3baf1e3be1d0e/content/renderer/media/webmediaplayer_ms.h

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=651820 to track work on a test that would have caught this.

FYI - this fix will be in the next M53 stable scheduled for qualification and push tomorrow.

https://chromium.googlesource.com/chromium/src/+log/53.0.2785.144..53.0.2785.154?pretty=fuller&n=10000

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31fafc9cebde8343c7bee95675dcb2b92b9c8720

commit 31fafc9cebde8343c7bee95675dcb2b92b9c8720
Author: emircan <emircan@chromium.org>
Date: Fri Sep 23 18:53:42 2016

Change WebMediaPlayerMS layers transparency dynamically

This CL changes how WebMediaPlayerMS sets its transparency
based on the incoming frame formats.

BUG= 647886 

Review-Url: https://codereview.chromium.org/2348903003
Cr-Commit-Position: refs/heads/master@{#419816}
(cherry picked from commit 5ddf895c72c8c5fe77f1f88698eaeaadf93b880e)

NOTRY=true
NOPRESUBMIT=true
TBR=mcasas@chromium.org

Review-Url: https://codereview.chromium.org/2358903002
Cr-Commit-Position: refs/branch-heads/2840@{#515}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/31fafc9cebde8343c7bee95675dcb2b92b9c8720/content/renderer/media/webmediaplayer_ms_unittest.cc