New issue
Advanced search Search tips

Issue 865159 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Mac
Pri: 2
Type: Bug

Blocking:
issue 793301



Sign in to add a comment

Enabling of Enable VideoSurfaceLayer and PictureInPicture flags causes video playback smoothness regression

Project Member Reported by crouleau@chromium.org, Jul 18

Issue description

See b/111593092 (Googler-only). A bisection of the issue zeroed in on the commit crrev.com/572793. Note that this is impacting both Mac and Windows.

Explanation of smoothness score is at https://sites.google.com/a/google.com/av-analysis-service/user-docs/metrics#TOC-Smoothness

 
Description: Show this description
Labels: ReleaseBlock-Stable M-69
Some of the bugs are fixed, but these are still broken according to recent AV Analysis results: 

b/111592105 smoothness_score 69.0.3481.0 -> 69.0.3488.0 surfacepro_corem3_6y30_intelhd515_win10  

b/111592375 smoothness_score 69.0.3482.0 -> 69.0.3489.0 macbookpro_i5_5257u_inteliris6100

Strange that some were fixed but not all.
It seems that 30fps videos have an issue. liberato@ is looking into this.
Great. I added more bugs into the blocking list of https://b.corp.google.com/issues/111593092 . Because of how the alerting system is set up, not all the alerts come in at the same time.
You're the owner of a Pri-1 M-69 chrome media issue. M-69 is now in beta and will ship to stable in coming weeks. See go/chromeschedule. Please work on resolving your issue ASAP if it needs fixing for the M-69 branch.

Pri-1 means the work is required for the branch. Alternatively, update the milestone to M-70 or remove the milestone and drop the priority to P-3.
Friendly ping to get an update as stable release is coming soon and this bug is marked as RBS for M69.
Thanks..!

Cc: fbeaufort@chromium.org
Gentle ping! Could you please provide any update on this issue as it has been marked as a stable blocker.

Thank You!
We are still investigating the root cause of the issue. The problem isn't significant enough to block Beta so we are running with 50/50 experiment in Beta. It may give us more interesting data. Stable will either have a 99% enabled or 1% enabled depending on whether we decide it's safe.

liberato@ was able to fix part of the issue but some smoothness regression may come from the new compositing pipeline. The team said they would have a look this week.
M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
What is the status now?
It seems still regressed. See this result for example: https://av-analysis.corp.google.com/#get-testpass-details/2/VideoStack/781386 (internal only link, sorry!): for that result, the smoothness is 48.

However, note that there have been some recent issues in the lab, so I don't have many recent (last 5 days) results. I will fix the issues and rerun the tests now.
Looks still regressed everywhere I see. For example: 
https://b.corp.google.com/issues/111299021#comment4
https://b.corp.google.com/issues/111431729#comment5

Note that it seems like there may be some kind of finch config messing with the results. For the same version number, I see one obviously good result on https://b.corp.google.com/issues/111299021#comment4 and two bad ones. It's unfortunate that we cannot find some reproductible testing method in Chrome.

M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.


I believe this is disabled by default and only enabled via finch experiment for beta+ at the moment, so we can probably remove the RB-S label. There's talk of a 1% stable experiment, but otherwise no plans to ship this in M69 due to this bug and others. Mounir, want to update the labels?
Labels: -ReleaseBlock-Stable
Dropping RBS. If by any chance we end up finding a solution that is cherry-pickable, we can re-evaluate but as dalecurtis@ said it is fairly unlikely we will go with 69 at the moment.
Blocking: 793301
Labels: -M-69
Is it still reasonable to have VideoSurfaceLayer be default on in Canary? It means that we aren't catching regression and bugs in the non-VideoSurfaceLayer path.

You could at least add a test case for the normal behavior in https://cs.corp.google.com/piper///depot/google3/chrome/videostack/testing/av_analysis/desktop_test_suite.py
Cc: cliffordcheng@chromium.org
+Clifford to take this over from me to help on an ongoing basis.
FWIW, I did a triage of the open AVA issues today and we went from 30 bugs to 5. Sounds like things are improving. We may want to confirm that the 5 remaining issues are actually related to VSL:
 - https://b.corp.google.com/issues/113661567
 - https://b.corp.google.com/issues/112029667
 - https://b.corp.google.com/issues/111722171
 - https://b.corp.google.com/issues/111494891
 - https://b.corp.google.com/issues/111177001
Labels: -Pri-1 Pri-2
The perf regressions are mostly resolved. We should keep an eye on this as a couple of bots still have 2-3 points regressions.
Nice, can you link the ones that are still active for posterity?
From the list above, all bugs are Fixed or WontFix except this one:
https://b.corp.google.com/issues/113661567
That one is fixed. I'm not sure about b/111494891 there's still a 6 point drop with surface layer enabled. Might be exacerbated by the fullscreen Android issue frank is looking at.

https://av-analysis.corp.google.com/#/get-testpasslist/2/VideoStack?device=nexus

Do we not run windowed tests with SurfaceLayer enabled? I don't see any results for them.
Is there any windowed test with SurfaceLayer enabled running before?
(Sorry, that I just took over)

I searched the code and there is only test with windowed setting to False.
Looks like b/111494891 finally completed a bisect and did point to surface layer. Will see if recent results are still bad. This shouldn't be affected by my background rendering fix since the tab should always be in the foreground.
Status: Fixed (was: Assigned)
I think this is fixed now, N7 scores are in high 90s:
https://data.corp.google.com/sites/v6hts2xtvxa6/android/

Sign in to add a comment