Skip to content

Fix/mpris reporting #4438

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.

Sign up for GitHub

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

Open
mermonia wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix/mpris reporting #4438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d222bb7
services/mpris: bypass incorrect typing for Player static constants
mermonia Apr 26, 2026
bd5697b
services/mpris: fix incorrect reported duration
mermonia Apr 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 26 additions & 17 deletions src/plugins/shortcuts/mpris.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,16 @@ import MprisPlayer, {
type Position,
type Track,
} from '@jellybrick/mpris-service';
import * as MprisMeta from '@jellybrick/mpris-service';

const Mpris = MprisPlayer as typeof MprisPlayer & {
PLAYBACK_STATUS_PLAYING: PlayBackStatus;
PLAYBACK_STATUS_PAUSED: PlayBackStatus;
PLAYBACK_STATUS_STOPPED: PlayBackStatus;

LOOP_STATUS_NONE: LoopStatus;
LOOP_STATUS_TRACK: LoopStatus;
LOOP_STATUS_PLAYLIST: LoopStatus;
};
Comment on lines +11 to +19
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

For the exact version of @jellybrick/mpris-service used in this repository, are PLAYBACK_STATUS_PLAYING / PLAYBACK_STATUS_PAUSED / PLAYBACK_STATUS_STOPPED and LOOP_STATUS_NONE / LOOP_STATUS_TRACK / LOOP_STATUS_PLAYLIST exposed as static properties on the default export, or only as named module exports? Please check the package source or official docs for that version.

💡 Result:

The constants PLAYBACK_STATUS_PLAYING, PLAYBACK_STATUS_PAUSED, PLAYBACK_STATUS_STOPPED, LOOP_STATUS_NONE, LOOP_STATUS_TRACK, and LOOP_STATUS_PLAYLIST are exposed only as named module exports, not as static properties on the default export (Player class). Pear-desktop, a likely candidate repository using this package (given the matching scoped name and JellyBrick's contributions), defines these exact constants in its TypeScript declaration file for @jellybrick/mpris-service (mpris-service.d.ts). They are declared as top-level named exports: export const PLAYBACK_STATUS_PLAYING: 'Playing'; export const PLAYBACK_STATUS_PAUSED: 'Paused'; export const PLAYBACK_STATUS_STOPPED: 'Stopped'; export const LOOP_STATUS_NONE: 'None'; export const LOOP_STATUS_TRACK: 'Track'; export const LOOP_STATUS_PLAYLIST: 'Playlist'; Usage in pear-desktop's mpris.ts confirms named imports: import MprisPlayer, { LOOP_STATUS_NONE, LOOP_STATUS_PLAYLIST, LOOP_STATUS_TRACK, PLAYBACK_STATUS_PAUSED, PLAYBACK_STATUS_PLAYING, PLAYBACK_STATUS_STOPPED, ... } from '@jellybrick/mpris-service'; The package structure includes dist/constants.js (742B), indicating constants are in a separate module file, typical for named exports. The default export is the Player class (new Player({...})), with no evidence of static properties for these values on Player. Properties like player.playbackStatus use the string literals directly (e.g., 'Playing'), but constants are provided as named exports for convenience and type safety.

Citations:

🏁 Script executed:

# First, check the package.json for `@jellybrick/mpris-service` version
cat package.json | grep -A2 -B2 "@jellybrick/mpris-service" || echo "Not found in root"

# Also check for yarn.lock or package-lock.json
if [ -f yarn.lock ]; then
  echo "=== From yarn.lock ===" 
  grep -A5 "@jellybrick/mpris-service" yarn.lock | head -20
elif [ -f package-lock.json ]; then
  echo "=== From package-lock.json ==="
  grep -A5 "@jellybrick/mpris-service" package-lock.json | head -20
fi

Repository: pear-devs/pear-desktop

Length of output: 267

🏁 Script executed:

# Now examine the actual usage in mpris.ts
echo "=== File: src/plugins/shortcuts/mpris.ts (lines 11-50) ==="
sed -n '11,50p' src/plugins/shortcuts/mpris.ts

echo ""
echo "=== Checking all usage of 'Mpris.' pattern ==="
grep -n "Mpris\." src/plugins/shortcuts/mpris.ts | head -30

Repository: pear-devs/pear-desktop

Length of output: 2097

🏁 Script executed:

# Check the declaration file
echo "=== File: src/plugins/shortcuts/mpris-service.d.ts ==="
cat src/plugins/shortcuts/mpris-service.d.ts

Repository: pear-devs/pear-desktop

Length of output: 5777

Replace the type assertion with direct named imports of constants.

The cast at lines 11–19 only satisfies TypeScript; it does not create runtime properties. Since PLAYBACK_STATUS_PLAYING, PLAYBACK_STATUS_PAUSED, PLAYBACK_STATUS_STOPPED, LOOP_STATUS_NONE, LOOP_STATUS_TRACK, and LOOP_STATUS_PLAYLIST are exported only as named constants (not static properties on the Player class) in @jellybrick/mpris-service@2.1.5, every usage of Mpris.PLAYBACK_STATUS_* and Mpris.LOOP_STATUS_* throughout this file will be undefined at runtime, breaking all status comparisons and assignments.

Import the constants directly:

import MprisPlayer, {
  PLAYBACK_STATUS_PLAYING,
  PLAYBACK_STATUS_PAUSED,
  PLAYBACK_STATUS_STOPPED,
  LOOP_STATUS_NONE,
  LOOP_STATUS_TRACK,
  LOOP_STATUS_PLAYLIST,
} from '@jellybrick/mpris-service';

Then remove the type assertion and use the imported constants directly.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/plugins/shortcuts/mpris.ts` around lines 11 - 19, The current type
assertion on MprisPlayer creates no runtime properties and causes
Mpris.PLAYBACK_STATUS_* and Mpris.LOOP_STATUS_* to be undefined; replace the
cast by directly importing MprisPlayer plus the named constants
PLAYBACK_STATUS_PLAYING, PLAYBACK_STATUS_PAUSED, PLAYBACK_STATUS_STOPPED,
LOOP_STATUS_NONE, LOOP_STATUS_TRACK, and LOOP_STATUS_PLAYLIST from
'@jellybrick/mpris-service', remove the type assertion block that defines Mpris
as typeof MprisPlayer & {...}, and update all uses of Mpris.PLAYBACK_STATUS_*
and Mpris.LOOP_STATUS_* to reference the imported constants directly (keeping
MprisPlayer usage for the class where needed).


import {
registerCallback,
Expand Down Expand Up @@ -49,15 +58,15 @@ class YTPlayer extends MprisPlayer {
}

isPlaying(): boolean {
return this.playbackStatus === MprisMeta.PLAYBACK_STATUS_PLAYING;
return this.playbackStatus === Mpris.PLAYBACK_STATUS_PLAYING;
}

isPaused(): boolean {
return this.playbackStatus === MprisMeta.PLAYBACK_STATUS_PAUSED;
return this.playbackStatus === Mpris.PLAYBACK_STATUS_PAUSED;
}

isStopped(): boolean {
return this.playbackStatus === MprisMeta.PLAYBACK_STATUS_STOPPED;
return this.playbackStatus === Mpris.PLAYBACK_STATUS_STOPPED;
}

setPlaybackStatus(status: PlayBackStatus) {
Expand Down Expand Up @@ -142,15 +151,15 @@ export function registerMPRIS(win: BrowserWindow) {
ipcMain.on('peard:repeat-changed', (_, mode: RepeatMode) => {
switch (mode) {
case 'NONE': {
player.setLoopStatus(MprisMeta.LOOP_STATUS_NONE);
player.setLoopStatus(Mpris.LOOP_STATUS_NONE);
break;
}
case 'ONE': {
player.setLoopStatus(MprisMeta.LOOP_STATUS_TRACK);
player.setLoopStatus(Mpris.LOOP_STATUS_TRACK);
break;
}
case 'ALL': {
player.setLoopStatus(MprisMeta.LOOP_STATUS_PLAYLIST);
player.setLoopStatus(Mpris.LOOP_STATUS_PLAYLIST);
// No default
break;
}
Expand Down Expand Up @@ -213,7 +222,7 @@ export function registerMPRIS(win: BrowserWindow) {
let hasNext: boolean;
if (queue.autoPlaying) {
hasNext = true;
} else if (player.loopStatus === MprisMeta.LOOP_STATUS_PLAYLIST) {
} else if (player.loopStatus === Mpris.LOOP_STATUS_PLAYLIST) {
hasNext = true;
} else {
// Example: currentPosition = 0, queue.items.length = 29 -> hasNext = true
Expand All @@ -226,9 +235,9 @@ export function registerMPRIS(win: BrowserWindow) {
player.on('loopStatus', (status: LoopStatus) => {
// SwitchRepeat cycles between states in that order
const switches = [
MprisMeta.LOOP_STATUS_NONE,
MprisMeta.LOOP_STATUS_PLAYLIST,
MprisMeta.LOOP_STATUS_TRACK,
Mpris.LOOP_STATUS_NONE,
Mpris.LOOP_STATUS_PLAYLIST,
Mpris.LOOP_STATUS_TRACK,
];
const currentIndex = switches.indexOf(player.loopStatus);
const targetIndex = switches.indexOf(status);
Expand All @@ -253,21 +262,21 @@ export function registerMPRIS(win: BrowserWindow) {

player.on('play', () => {
if (!player.isPlaying()) {
player.setPlaybackStatus(MprisMeta.PLAYBACK_STATUS_PLAYING);
player.setPlaybackStatus(Mpris.PLAYBACK_STATUS_PLAYING);
playPause();
}
});
player.on('pause', () => {
if (!player.isPaused()) {
player.setPlaybackStatus(MprisMeta.PLAYBACK_STATUS_PAUSED);
player.setPlaybackStatus(Mpris.PLAYBACK_STATUS_PAUSED);
playPause();
}
});
player.on('playpause', () => {
player.setPlaybackStatus(
player.isPlaying()
? MprisMeta.PLAYBACK_STATUS_PAUSED
: MprisMeta.PLAYBACK_STATUS_PLAYING,
? Mpris.PLAYBACK_STATUS_PAUSED
: Mpris.PLAYBACK_STATUS_PLAYING,
);
playPause();
});
Expand Down Expand Up @@ -354,8 +363,8 @@ export function registerMPRIS(win: BrowserWindow) {

player.setPlaybackStatus(
songInfo.isPaused
? MprisMeta.PLAYBACK_STATUS_PAUSED
: MprisMeta.PLAYBACK_STATUS_PLAYING,
? Mpris.PLAYBACK_STATUS_PAUSED
: Mpris.PLAYBACK_STATUS_PLAYING,
);
}
requestQueueInformation();
Expand Down
15 changes: 6 additions & 9 deletions src/providers/song-info-front.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ window.ipcRenderer.on(
// Used because 'loadeddata' or 'loadedmetadata' weren't firing on song start for some users (https://github.com/pear-devs/pear-desktop/issues/473)
const srcChangedEvent = new CustomEvent('peard:src-changed');

const getTime = () => songInfo.elapsedSeconds ?? 0;

export const setupSeekedListener = singleton(() => {
document.querySelector('video')?.addEventListener('seeked', (v) => {
if (v.target instanceof HTMLVideoElement) {
window.ipcRenderer.send('peard:seeked', v.target.currentTime);
}
document.querySelector('video')?.addEventListener('seeked', () => {
window.ipcRenderer.send('peard:seeked', getTime());
});
Comment on lines +30 to 35
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reset the front-end elapsed-time cache when a new track starts.

getTime() now reads songInfo.elapsedSeconds, but that cache is only refreshed by peard:update-song-info and the progress-bar observer. During dataloaded, it still holds the previous track's position, so the first seeked / play-or-paused event for a newly chained track can report the old elapsed time to MPRIS. Reset the local cache on track change, or read the progress bar's current value directly here instead of the cached field.

Also applies to: 229-234

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/providers/song-info-front.ts` around lines 30 - 35, The seeked/play
handlers use getTime() which reads the stale songInfo.elapsedSeconds cache; fix
by having setupSeekedListener read the actual media element time (e.g.
document.querySelector('video')?.currentTime ?? 0) instead of songInfo, and
additionally clear or reset songInfo.elapsedSeconds when a new track is loaded
(handle the dataloaded or 'peard:update-song-info' point to set
songInfo.elapsedSeconds = undefined/0) so the cached value cannot leak into the
first seeked/play-or-paused event; update getTime() or inline the media-read in
setupSeekedListener and apply the same change at the other occurrence referenced
(lines ~229-234).

});

Expand Down Expand Up @@ -227,13 +227,10 @@ export const setupSongInfo = (api: MusicPlayer) => {
});

const playPausedHandler = (e: Event, status: string) => {
if (
e.target instanceof HTMLVideoElement &&
Math.round(e.target.currentTime) > 0
) {
if (e.target instanceof HTMLVideoElement) {
window.ipcRenderer.send('peard:play-or-paused', {
isPaused: status === 'pause',
elapsedSeconds: Math.floor(e.target.currentTime),
elapsedSeconds: getTime(),
});
}
};
Expand Down
Loading