Reduces the number of onTimeUpdate timers in MediaFoundation playback [chromium/src : main]
Frank Liberato (Gerrit)
Frank Liberato added 1 comment![]( "Open in Gerrit")
(-me) there's plenty of review coverage on this already.
thanks
-fl
Related details
- Dale Curtis
- Jordan Bayles
- Kent Tamura
- Mark Foltz
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 1 comment![]( "Open in Gerrit")
Removing reviewers until Xiaohan and I finish review to avoid noise.
Related details
- Tochukwu Ibe-Ekeocha
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 1 comment![]( "Open in Gerrit")
This unfortunately pollutes too many layers with build flags and knowledge of MediaFoundation. You'll need to find a more generic non-buildflag mechanism to activate this behavior.
E.g., one such mechanism might be to extract the current timer based mechanism from HTMLMediaElement into a helper utility that each WMP instance could either own a copy of or delegate to some underlying subsystem. We can then enable that on all platforms and avoid guarded code.
Related details
- Tochukwu Ibe-Ekeocha
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dale Curtis (Gerrit)
Dale Curtis added 1 comment![]( "Open in Gerrit")
an overall wake reduction per second of 8.This seems like a pretty minor reduction in timers. What does this mean in terms of power usage saved?
Xiaohan Wang (Gerrit)
Xiaohan Wang added 2 comments![]( "Open in Gerrit")
In addition to the theoretical analysis, we also performed a WPA trace which collects data on CPU Wakes. The traces were performed using Edge, as well as a vanilla chromium build with HWDRM support through PlayReady enabled. On average the traces showed less timer invocations when the feature flag was enabled.Thanks for the CL, and the detailed explanation in CL description!
I'll put all my comments here for easier discussion.
#### Real World Impact
What's the real world impact in terms of power savings? For example, for normal user scenarios (play, pause, etc).
#### Timer Frequency
According the the HTML5 video element spec:
If the time was reached through the usual monotonic increase of the current playback position during normal playback, and if the user agent has not fired a timeupdate event at the element in the past 15 to 250ms and is not still running event handlers for such an event, then the user agent must queue a task to fire a simple event named timeupdate at the element...
So during playback, how often will MediaFoundation MediaEngine file OnTimeUpdate()? For example, if it fires every 125ms, then we won't have much savings?
One thing I noticed is that we don't stop the timer in MojoRendererService when the playback rate is 0 (pause). Maybe that's something we can improve.
#### Code Consistency
I agree with Dale that we should not expose too much Windows / MediaFoundation-specifics in the media pipeline. A lot of files touched in this CL are generic, that they support playback on all platforms.
So if we do feel a push (event-driven) model is necessary, we should have a generic way to support it, and each Renderer can choose which model it wants to use.
base::FEATURE_DISABLED_BY_DEFAULT);If you rebase this CL, you can use the new 2-param BASE_FEATURE macro 😊
```
BASE_FEATURE(kMediaFoundationEventDrivenTimeUpdates,
base::FEATURE_DISABLED_BY_DEFAULT);
```
Tochukwu Ibe-Ekeocha (Gerrit)
Tochukwu Ibe-Ekeocha abandoned this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |