Closed Bug 1689538 Opened 4 years ago Closed 7 months ago

Provide timeline information of media to Windows 10's SMTC

Categories

(Core :: Audio/Video: Playback, enhancement, P4)

Firefox 85
enhancement

Tracking

()

RESOLVED FIXED
125 Branch
Tracking Status
firefox125 --- fixed

People

(Reporter: Poopooracoocoo, Assigned: nerixdev)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

  1. Played https://googlechrome.github.io/samples/media-session/video.html
  2. Pressed pause on my keyboard. Note that I am running Modern Flyouts on Windows 10.

Also see: https://github.com/ModernFlyouts-Community/ModernFlyouts/blob/main/docs/GSMTC-Support-And-Popular-Apps.md

Actual results:

No timeline information was displayed.

Expected results:

Timeline information should have been displayed.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core

Just as an FYI, Google said that they recently implemented this for macOS, and may do so for Windows.

Severity: -- → S3
Priority: -- → P3

has there been any progress on this bug btw?

I don't have much experience with C++, but I looked into this.

The main problem is that with the current setup, Firefox can't update the timeline.
As I explained in 1698283 (comment), Firefox (and on this note Chromium as well) uses the ISystemMediaTransportControls interface which can't update the timeline (as it's too old?).

To update the timeline, Firefox would need to get the SystemMediaTransportControls class from C++/WinRT. This could introduce breaking changes.

  1. Compatibility with older version of Windows. In the WindowsSMTCProvider, there's a check for Windows 8.1 or later. The "new" SMTC would require at least Windows 10 (Build 10240, the first one).
  2. From what I found, Firefox is not using C++/WinRT so this would need to be added.
  3. The "new" SMTC don't provide a way of getting the class/interface for a specific HWND, instead one just calls the static method. Currently, Firefox creates an invisible window to bind the SMTC to.

There also seems to exist ISystemMediaTransportControls2 (in Windows.Media.h) which I just found out about.

It's not documented anywhere but it's the missing interface. It doesn't need C++/WinRT, the ComPtr<ISystemMediaTransportControls> (in WindowsSMTCProvider) could be casted to a ComPtr<ISystemMediaTransportControls2> where the timeline could be updated.

I'm not too sure about compatibility with older Windows versions.

In order to do this, we might want to have a cross-platform mechanism to update the time to the platform specific MediaControlKeySource first. We have a method to set position if websites set the position state via MediaSession API, but I suppose that that API won't be called frequently or never be called. So we might need a way to calulate that by ourselve in order to update a rough position information and use above method to notify the key source to call platform API.

Also, if multiple media are playing in the same tab, we would also need to determine what media should be shown for the timeline update. (or don't show timeline in this situation)


In addition, as the default SMTC visual control interface on Windows 10 doesn't have timeline display, this bug is an enhancement for an external plugin (Modern Flyouts), so change its priority to P4.

Severity: S3 → --
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: P3 → P4

The PositionState is already saved as an optional value inside the MediaSession.
This makes the PositionState optional in all events emitted as it can be
explicitly reset by passing an empty object.

Assignee: nobody → nerixdev
Status: NEW → ASSIGNED

When setting the position state for a media session, it might not be the
active. When activating a session it should be sent.

Depends on D198847

Sorry for my late response. I just came back from vacation. I just want to let you know that I will start reviewing your patches next week, thank you for your patience!

Attachment #9373334 - Attachment description: Bug 1689538 - Provide timeline information to Windows' SMTC. r=alwu → Bug 1689538 - Provide timeline information and handle SeekTo on Windows. r=alwu
Blocks: 1880153

Thank you so much for your help on this issue. After getting r+ on your patches, you could consider to grant the try server permission and then pushing patches to the try server, or I can help you push them to the try server to see if they break any tests or not.

Let me know what path you would like to choose. Thank you!

Flags: needinfo?(nerixdev)

I updated the patches, and I'll try to get permission and push to the try server after getting r+.

For integration tests of SMTC, GSMTC could be used, but that might be flaky. I know that in some (unknown) cases, it doesn't capture running apps, but I don't know how rare that bug is. Windows itself uses the NowPlayingSessionManager, which is undocumented.

Flags: needinfo?(nerixdev)

(In reply to Nerixyz from comment #16)

I updated the patches, and I'll try to get permission and push to the try server after getting r+.

I already r+ all you patches and the win-reviewers for D198850 is optional, so you're good to go now! I'm happy to vouch you for the level 1 access, you just need to follow this instruction and NI me on the bug.

For integration tests of SMTC, GSMTC could be used, but that might be flaky. I know that in some (unknown) cases, it doesn't capture running apps, but I don't know how rare that bug is. Windows itself uses the NowPlayingSessionManager, which is undocumented.

Yes unfortunately currently we're lacking of the platform level integration test for this feature, we mostly rely on the mediacontroller level integration tests. We could definitely do it better if there is a good and robust way to do so.

I pushed the patches to try. The 974 failures don't seem to be related to the patches - at least they're not in the mediacontrol tests.

You can use ./mach try fuzzy --preset media-full to test our pre-defined media tests.

Ah, thank you! This one got a few more failures, but the only one from mediacontrol was browser_audio_focus_management.js.

Hmm usually we shouldn't get that many errors, I would suggest that you can push two times, one with your patches and one without, to see the difference. Also, it would be good to push changes based on the latest mozilla-central to see if there is any conflict.

I pushed your changes to the try server, and the result looks good. I will land it later after the tasks are all completed and no broken tests on the CI.

Thank you! Seems like we both pushed :)

Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f94f409282ad
Emit PositionState as an optional value. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3b2c031481cb
Save PositionState for media sessions in MediaStatusManager. r=alwu
https://hg.mozilla.org/integration/autoland/rev/0ab5cd414ce9
Implement current playback position algorithm for PositionState. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f81cf9bd7b9f
Provide timeline information and handle SeekTo on Windows. r=alwu

Backed out for causing build bustage on TestPositionState.cpp

Backout link

Push with failures

Failure log

Flags: needinfo?(nerixdev)

Moved the test data out of the anonymous namespace (see D198849).

Flags: needinfo?(nerixdev)
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/72bc9a4787bc
Emit PositionState as an optional value. r=alwu
https://hg.mozilla.org/integration/autoland/rev/9e54467e852a
Save PositionState for media sessions in MediaStatusManager. r=alwu
https://hg.mozilla.org/integration/autoland/rev/41fd655c5c32
Implement current playback position algorithm for PositionState. r=alwu
https://hg.mozilla.org/integration/autoland/rev/21d2a36f8f07
Provide timeline information and handle SeekTo on Windows. r=alwu
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: