Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Invalid mpris position report #2225

Closed
3 of 5 tasks
zoriya opened this issue Jul 12, 2024 · 2 comments
Closed
3 of 5 tasks

[Bug]: Invalid mpris position report #2225

zoriya opened this issue Jul 12, 2024 · 2 comments
Labels
bug Something isn't working fix-available A fix to the issue is available in a new version

Comments

@zoriya
Copy link

zoriya commented Jul 12, 2024

Preflight Checklist

  • I use the latest version of YouTube Music (Application).
  • I have searched the issue tracker for a bug report that matches the one I want to file, without success.
  • I understand that th-ch/youtube-music has NO affiliation with Google or YouTube

YouTube Music (Application) Version

3.3.12

Checklists

What operating system are you using?

Other Linux

Operating System Version

Linux fuhen 6.6.36 #1-NixOS SMP PREEMPT_DYNAMIC Thu Jun 27 11:49:15 UTC 2024 x86_64 GNU/Linux

What CPU architecture are you using?

x64

Last Known Working YouTube Music (Application) version

/shrug

Reproduction steps

  1. Enable the MPRIS plugin
  2. Start playing a song, wait a few seconds
  3. Run playerctl position and see something like 0.0025
  4. Seek somewhere on the song (say 1:00`
  5. Run playerctl position and see the position you seeked to (so 60.567 for example)
  6. Wait a few seconds
  7. Run playerctl position and see the same value (or close to) as before (so 60.4567 instead of 65.12 for example)

Expected Behavior

Mpris's position should be reported correctly, even when not seeking manually

Actual Behavior

See reproduction steps.

Enabled plugins

[YTMusic] Plugin "shortcuts::menu" loaded
[YTMusic] Plugin "lyrics-genius::menu" loaded
[YTMusic] Plugin "downloader::menu" loaded
[YTMusic] Plugin "album-color-theme::menu" loaded
[YTMusic] Plugin "shortcuts::menu" loaded
[YTMusic] Plugin "lyrics-genius::menu" loaded
[YTMusic] Plugin "downloader::menu" loaded
[YTMusic] Plugin "album-color-theme::menu" loaded

Additional Information

No response

@JellyBrick JellyBrick added bug Something isn't working fix-available A fix to the issue is available in a new version labels Jul 14, 2024
@bqcrabtree
Copy link

I'm seeing this issue still in v3.4.1

@JellyBrick JellyBrick reopened this Jul 19, 2024
@JellyBrick JellyBrick removed the fix-available A fix to the issue is available in a new version label Jul 19, 2024
@h-banii
Copy link
Contributor

h-banii commented Aug 2, 2024

I've been having this issue for a while, here's more information about it:

  • last known working version: v3.3.6
  • I'm almost sure the first bad commit is eaf9d31
  • workaround: reload page through the menu view > reload

I could not bisect it properly because there were some broken commits for me, but reverting eaf9d31 seems to fix it.

Here's a demonstration video of the issue (and the workaround):

mpris_position.mp4

(In the video you can see the position does not update when playing the song, it stays at around the same timestamp)


Edit: I figured it out

The issue happens because this callback isn't called (reloading the page triggers this event, which runs the callback and fixes the issue)

ipcMain.on('ytmd:player-api-loaded', () => {
win.webContents.send('ytmd:setup-seeked-listener', 'mpris');
win.webContents.send('ytmd:setup-time-changed-listener', 'mpris');
win.webContents.send('ytmd:setup-repeat-changed-listener', 'mpris');
win.webContents.send('ytmd:setup-volume-changed-listener', 'mpris');
win.webContents.send('ytmd:setup-fullscreen-changed-listener', 'mpris');
win.webContents.send('ytmd:setup-autoplay-changed-listener', 'mpris');
requestFullscreenInformation();
requestQueueInformation();
});

That event used to be sent AFTER registerMPRIS(), so the callback runs. However, after commit eaf9d31 that event is sent BEFORE registerMPRIS(), i.e. before the listener is created.

ipcMain.once('ytmd:video-src-changed', (_) => {
registerMPRIS(window);
});


Anyway, here's a possible solution (I'm not opening a PR, because the bad commit was already reverted 7599cc6 )
diff --git a/src/plugins/shortcuts/main.ts b/src/plugins/shortcuts/main.ts
index 84c29899..59f40d13 100644
--- a/src/plugins/shortcuts/main.ts
+++ b/src/plugins/shortcuts/main.ts
@@ -48,9 +48,10 @@ export const onMainLoad = async ({
   _registerLocalShortcut(window, 'CommandOrControl+L', search);
 
   if (is.linux()) {
-    ipcMain.once('ytmd:video-src-changed', (_) => {
-      registerMPRIS(window);
-    });
+    const channels = ['ytmd:player-api-loaded','ytmd:video-src-changed']
+      .map((channel) => new Promise((resolve) => ipcMain.once(channel, resolve)));
+
+    Promise.all(channels).then(() => registerMPRIS(window));
   }
 
   const { global, local } = config;
diff --git a/src/plugins/shortcuts/mpris.ts b/src/plugins/shortcuts/mpris.ts
index cdfc9d4b..59b269b4 100644
--- a/src/plugins/shortcuts/mpris.ts
+++ b/src/plugins/shortcuts/mpris.ts
@@ -118,7 +118,7 @@ function registerMPRIS(win: BrowserWindow) {
       player.setPosition(player.getPosition() + offset);
     };
 
-    ipcMain.on('ytmd:player-api-loaded', () => {
+    const onApiLoaded = () => {
       win.webContents.send('ytmd:setup-seeked-listener', 'mpris');
       win.webContents.send('ytmd:setup-time-changed-listener', 'mpris');
       win.webContents.send('ytmd:setup-repeat-changed-listener', 'mpris');
@@ -127,7 +127,10 @@ function registerMPRIS(win: BrowserWindow) {
       win.webContents.send('ytmd:setup-autoplay-changed-listener', 'mpris');
       requestFullscreenInformation();
       requestQueueInformation();
-    });
+    };
+
+    onApiLoaded();
+    ipcMain.on('ytmd:player-api-loaded', onApiLoaded);
 
     ipcMain.on('ytmd:seeked', (_, t: number) => {
       player.setPosition(secToMicro(t));

@JellyBrick JellyBrick added the fix-available A fix to the issue is available in a new version label Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix-available A fix to the issue is available in a new version
Projects
None yet
Development

No branches or pull requests

4 participants