Skip to content

Upgrade the MPris2 definitions#2029

Draft
Vinzzzze wants to merge 2 commits intostrawberrymusicplayer:masterfrom
Vinzzzze:mpris_upgrade
Draft

Upgrade the MPris2 definitions#2029
Vinzzzze wants to merge 2 commits intostrawberrymusicplayer:masterfrom
Vinzzzze:mpris_upgrade

Conversation

@Vinzzzze
Copy link
Copy Markdown
Contributor

@Vinzzzze Vinzzzze commented Mar 4, 2026

Properties / methods updated :

  • Tracks : return all the tracks in the current playlist
  • GetTracksMetadata : return the specified tracklist metadata using Song::ToXesam method
  • AddTracks : add the tracks to the current playlist
  • RemoveTracks : remove the tracks from the current playlist
  • GoTo : go to the specified track in the current playlist
  • GetPlaylists : use the order parameter
  • PlaylistItemsAdded : To know when a track is really added to a playlist
  • PlaylistItemsRemoved : To know when a track is really removed from a playlist

@jonaski
Copy link
Copy Markdown
Member

jonaski commented Mar 4, 2026

Create some new MPris properties / methods, my main worry is about the Shuffle property that was a boolean property and I migrated it to a string property. May be I should have created a new property. Properties / methods updated :

Shuffle needs to be bool: https://specifications.freedesktop.org/mpris/latest/Player_Interface.html#Property:Shuffle
And I'm not sure we should add methods that are not part of the official specifications either.

@Vinzzzze
Copy link
Copy Markdown
Contributor Author

Vinzzzze commented Mar 5, 2026

ok, I will change my PR to match these requirements : no new method and Shuffle will be bool

@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 2 times, most recently from f2e595d to 1ffa3ff Compare March 7, 2026 07:53
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/playlist/playlist.h Outdated
@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 2 times, most recently from be5291c to a903658 Compare March 11, 2026 21:42
@jonaski jonaski requested a review from Copilot March 11, 2026 22:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the MPRIS2 implementation to expose a fuller TrackList/Playlists feature set (track enumeration, metadata lookup, add/remove/goto operations, and playlist ordering), and adds support for an “Album” loop mode.

Changes:

  • Expose playlist playback order indices via Playlist::virtual_items() for MPRIS TrackList integration.
  • Implement Tracks, GetTracksMetadata, AddTrack, RemoveTrack, and GoTo in Mpris2.
  • Add basic sorting support in GetPlaylists(order, reverse_order) and extend LoopStatus handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
src/playlist/playlist.h Adds a getter to expose virtual_items_ (playback order indices) to other components (MPRIS).
src/mpris2/mpris2.cpp Implements/updates MPRIS2 TrackList and Playlists behaviors (loop status, tracks enumeration, metadata, add/remove/goto, playlist ordering).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 2 times, most recently from 42692f1 to d99254c Compare March 12, 2026 18:48
@Vinzzzze Vinzzzze requested a review from jonaski March 12, 2026 18:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 11 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
@jonaski jonaski force-pushed the mpris_upgrade branch 3 times, most recently from 4494040 to 7ca94f8 Compare March 14, 2026 00:25
@jonaski jonaski requested a review from Copilot March 14, 2026 00:30
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp
Comment thread src/mpris2/mpris2.cpp
@jonaski jonaski force-pushed the mpris_upgrade branch 2 times, most recently from 7994a84 to 4ac6add Compare March 14, 2026 01:50
@jonaski jonaski requested a review from Copilot March 14, 2026 01:51
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Comment thread src/mpris2/mpris2.cpp Outdated
Copy link
Copy Markdown
Member

@jonaski jonaski left a comment

Choose a reason for hiding this comment

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

According to the specifications https://mpris2.readthedocs.io/en/latest/mpris2.tracklist.html, the track ID's need to be unique and not change, when reshuffling, what songs virtual_items_ point to will change so we can't use that, so I removed use of that so it's using the playlist item indexes, but I realize we can't use that either since it changes when inserting, removing or moving songs in the playlist.
We can't use song_id, because only collection songs have that, so I think we need to introduce a unique identifier to the playlist items.
Also, according to the documentation, TrackList should not return the entire playlist, but only the items in the queue + some of the previously played songs.
And AddTrack / RemoveTrack need to emit TrackAdded / TrackRemoved, but the signals needs to be emitted after the songs have been inserted in the playlist and a unique ID has been allocated.
So implementing this is much more complicated than I originally thought.

@Vinzzzze
Copy link
Copy Markdown
Contributor Author

According to the specifications https://mpris2.readthedocs.io/en/latest/mpris2.tracklist.html, the track ID's need to be unique and not change, when reshuffling, what songs virtual_items_ point to will change so we can't use that, so I removed use of that so it's using the playlist item indexes, but I realize we can't use that either since it changes when inserting, removing or moving songs in the playlist. We can't use song_id, because only collection songs have that, so I think we need to introduce a unique identifier to the playlist items. Also, according to the documentation, TrackList should not return the entire playlist, but only the items in the queue + some of the previously played songs. And AddTrack / RemoveTrack need to emit TrackAdded / TrackRemoved, but the signals needs to be emitted after the songs have been inserted in the playlist and a unique ID has been allocated. So implementing this is much more complicated than I originally thought.

Ok, got it. If you want, we can close this PR until having the right idea to solve theses issues. I am very interested by being able to control the player from the command line.
May be I should rather implement some new options in that purpose, or I can have a look to the MPris requirement and exchange with you on the solution I want to put in place before submitting a PR.

@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 2 times, most recently from e413fc7 to 2f7b807 Compare March 26, 2026 18:38
@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 2 times, most recently from 80c4618 to 5f4b1c9 Compare April 4, 2026 18:35
@jonaski
Copy link
Copy Markdown
Member

jonaski commented Apr 6, 2026

Let's just leave this open, then add the required changes separately, merge those first, then rebase this after it's implented.
We can implement a unique ID when new playlist items are inserted, which is saved to the playlist_items so it doesn't change when playlists are re-opened, I think QUuid: https://doc.qt.io/qt-6/quuid.html would be sufficient.
Then implement a function to Playlist which returns the previous 20 and next 20 ID's for use with MPRIS2 taking played indexes and next virtual items and queued items into consideration.
There also need to be a signal when items are inserted / removed back to MPRIS2 but only for the changes through MPRIS2 AddTrack / RemoveTrack, a bit unsure how difficult that would be to implement.

@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 3 times, most recently from 2a8518f to 3f8c391 Compare April 10, 2026 14:28
@Vinzzzze
Copy link
Copy Markdown
Contributor Author

Hello,
I created the PR #2059 to add the unique identifier to items, this PR had to be validated first.
This PR is also included in this PR (that's why there are two commits, I will maintain them up-to-date) to show you the solution I imagine.
I think that now the code fulfill the mpris specifications.

@Vinzzzze Vinzzzze marked this pull request as draft April 10, 2026 14:42
@Vinzzzze Vinzzzze force-pushed the mpris_upgrade branch 3 times, most recently from 5c6f801 to e6c21a8 Compare April 10, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants