Add interface for Dbus MPRIS and MacOS MediaPlayer - #15754
Conversation
…ner now opens and closes the file
6069e57 to
7254a33
Compare
|
This PR needs testing on macOS and Linux |
|
I'm grateful for this PR, as I've been planning a small set that syncs up some visuals to songs, and thus needs a way to extract the songs & playback positions from Mixxx. On Linux, the GUI hitches at times. Seems like it's related to the cover art. Observed after the short GUI freezes that I personally would also love if this also added the |
|
Thanks for your testing and feedback! I will try to incorporate the code change in this PR ASAP. If you have a MacOS machine or know somebody that does, some feedback there would also be greatly appreciated :D |
|
Sadly I don't have nor know MacOS users 😅 Thought I'd leave an additional comment though since I noticed that it's possible to get the volume meters to red and still not have the song picked up into MPRIS. Found this out with a badly normalized (multistem) audio file (that even the max 30LUFS/-12DB of normalization couldn't save peaking off of the waveform), and compensating for it with setting the deck specific gain very low. I guess this is more of "user error" than a major logic issue though 😛 |
The current logic is to check periodically if there is a track "audible", that is, above a certain threshold. The threshold is defined here and the period there. Could you please check in your fork if adjusting any of these value gives you better result? I guess we could expose these settings in the parameters! |
7254a33 to
5a57663
Compare
Indeed, the coverart file operation was happening in the main thread. This has now been moved in the threadpool instead.
Good shout - should be there now! |
5a2dfc8 to
ced8a45
Compare
- Stub out MacOSMediaPlayerService
- Broadcast now playing info to MPNowPlayingInfoCenter
- Wire up MacOSMediaPlayerService
- Remove unused dependency
- Set now playing playback state
- Define __MACOS_MEDIAPLAYER__ publicly
- Set asset URL and media type for now playing
- Set some dummy values for playback rate and time
- Set MPNowPlayingInfoPropertyIsLiveStream
- Register play command in MPRemoteCommandCenter
- Successfully load cover art asynchronously to macOS media player
- Handle case where broadcasted track is null
- Add moc include
- Use new CoverArtCache API
ced8a45 to
d3285de
Compare
|
@daschuer do you still have interest in reviewing this? I will take the freedom to close the two superseded PRs. |
I already tried adjusting the threshold before to differing values with seemingly no effect, but I'll try more thoroughly tweaking it and the period too when I have the time to rebase and such…
I can still observe similar UI freezing as before sadly. Probably not super common to have this kind of big cover images, ffprobe reports the almost second freezes with as
Yup, thank you! Now with proper URL encoding too which I had forgotten about ( |
Not entirely sure what his happening - there could be an issue somewhere else with DBus. It is worth to say that I have only rebased the existing work that was done, so there could be something else causing this freeze, but since I cannot reproduce it, it is hard to guess... Perhaps other core member will be able to spot it when testing it and help catching it?
From what I can understand from the (quite light) MPRIS spec, this field is expected to be a URL, and thus the encoding as it is currently is expected, and your consumer project should decode the URI. |
|
@mixxxdj/developers this PR is easily catching conflict, so let me know when you have the bandwidth to start reviewing/testing it and I will resolve them. |
|
To be honest, I think it's unlikely that this PR will be merged. Not because of the code itself, but because this PR contains two complex platform-specific features - for two different platforms. |
As you can see in the PR description, I spent a significant amount of effort recovering the exact history of #3483! This means the only change in this PR is related to the last commit, which address the change request in the superseded PR. The only thing to review here is
Now, if you think I should move the last two commits from MacOS support in a subsequent PR, I can do that too, I just would like to stop spending time and effort if nobody is going to help me delivering this long outstanding feature. |
|
I think the best way to split up this PR would be these four chunks:
the way I do this is cherrypicking files from the giant change into a fresh branch. Can you resolve the merge conflicts first, though? |
|
I don't have interest to break down the code I have written in a first place, so except if we can agree on merging 1) and 2), I don't think it is worth spending any further effort here. |
|
why not fix the conflicts first and I'll take a look at it |
| // playerPositionChanged for deck1 after the track is loaded. | ||
| m_eState = ADJ_ENABLE_P1LOADED; | ||
|
|
||
| m_idleState.set(false); |
There was a problem hiding this comment.
assuming m_idleState is supposed to mirror m_eState, it's brittle to make it a separate value because they could get out of sync. Instead, better to make m_eState a controlobject that gets set to the appropriate enum value, and then whatever other part of this needs to know about idle state can just look for that value of m_eState.
There was a problem hiding this comment.
Not entirely sure, these are elements I have taken from the original PR, my only change was to get rid of the pointer, to align with the refactor that was done to remove the fat pointers.


This PR aims to unblock the situation with #3483 and the sheer size of change that could not be reviewed.
This is a rebase of #3483 with two additional commits:
No change was made to the interface, in order to easily bring on the
NowPlaying.txtandListenbrainsin future efforts, with a minimum amount of changes.It also contains a squash-merge of #4809, as well as a commit to integrate this in the same API. Of course, this can easily be moved as a subsequent PR if we think this is still too big.
Screencast.From.2025-12-15.18-01-43.mp4