mixxx icon indicating copy to clipboard operation
mixxx copied to clipboard

expose current track metadata to external APIs

Open daschuer opened this issue 4 years ago • 41 comments

This is a rebase of @davidhm Livemetadata #1675 PR on current main.

It fetaures:

  • MPRIS D-Bus interface for Linux system tray remotes
  • Scrobbling to Listenbrains
  • Writing a NowPlaying.txt

daschuer avatar Dec 27 '20 00:12 daschuer

All green now. @poelzi Please give this a try.

daschuer avatar Dec 27 '20 23:12 daschuer

@daschuer thanks for picking this one up, it would have taken quite some time for me. apart from smaller suggestions and one crash, this branch works nice.

poelzi avatar Dec 28 '20 06:12 poelzi

Now the crash is fixed. Mpirs player should now work at any Auto-DJ state, fixing the reported issue. I have also used [Master],gain for mpris volume, because it creates weird situations when messing with the deck volume.

daschuer avatar Dec 29 '20 23:12 daschuer

Please add comments in the header of every new class explaining its purpose.

Be-ing avatar Dec 30 '20 00:12 Be-ing

https://extensions.gnome.org/extension/1379/mpris-indicator-button/ works with this branch. Cool.

Be-ing avatar Dec 30 '20 01:12 Be-ing

I did a lengthly review and when the points are fixed, I'm fine with merging.

poelzi avatar Feb 18 '21 00:02 poelzi

I don't know how others feel about this, but I would like the mpris interface to be disabled when recording is running (aka, I'm actually mixing). I had the idea of a live mode already with different playcounter and last_played values, but since this is not yet there. I'm just imagine some integration doing unwanted stuff to my set.... but maybe I'm paranoid.

I get your point, but we shouldn't disable it IMHO. For example, the MPRIS interface is very convenient for polling the current track and then updating metadata on your website when streaming.

The the actual issue is that Mixxx shouldn't be able to be controlled during mixing. At first, I thought we could just set CanControl to false when starting a stream, but this does not work:

CanControl — b

Read only

The org.freedesktop.DBus.Properties.PropertiesChanged signal is not emitted when this property changes.

Whether the media player may be controlled over this interface.

This property is not expected to change, as it describes an intrinsic capability of the implementation.

So we can't change it at runtime.

I'd suggest that we just remove support for controlling Mixxx via MPRIS entirely. I consider it a gimmick and not actually that useful, unless you abuse Mixxx as a generic media player instead of a DJ application.

If you think this feature is useful, we could keep support for it and add a preferences option to disable support for controlling Mixxx via MPRIS (changing this will require to restart Mixxx).

Another alternative would be to silently disable support for controlling Mixxx while broadcasting/recording without changing the CanControl property, but IMHO this is hacky, might lead to bug reports ("Your MPRIS implementation is broken!") and also doesn't work for use cases where you use Mixxx to DJ live on a PA without broadcasting/recording your set.

Holzhaus avatar Feb 18 '21 13:02 Holzhaus

@Holzhaus I'm absolutely against removing the control possibilities, because I use mixxx as my day to day music player. I want this so I can integrate mixxx into my home automation and pause the playback when I leave the room and later add a mixxx specific interface to call COs etc.

poelzi avatar Feb 18 '21 13:02 poelzi

@poelzi Okay. What about this proposal?

If you think this feature is useful, we could keep support for it and add a preferences option to disable support for controlling Mixxx via MPRIS (changing this will require to restart Mixxx).

Holzhaus avatar Feb 18 '21 14:02 Holzhaus

@Holzhaus yes, that would be fine. I will implement the live mode sooner or later, then we can make this dependent on that. I strongly disagree about calling home use an abuse. Of course, it is not the most efficient player, but it has unbeatable advantages compared to any other player. It makes no sense to use any other player to be honest if you want to dj well. I doubt that someone listens to music on player x and when he likes some part, switches to his dj software just to prepare the track. If the track is already loaded in place, setting a hotcue is easy. Only on the road I use a MPD which I fill from mixxx.

poelzi avatar Feb 18 '21 15:02 poelzi

I don't understand the point of an option to disable MPRIS. If you don't want to use it, then simply don't use it. How does it interfere with anything?

Be-ing avatar Feb 18 '21 15:02 Be-ing

@Be-ing for example, I will see mixxx on my android lock screen, which track is playing 👍🏽 and have a pause play button next to it. This is wonderful in daytoday use, but when I'm doing a live stream, I don't want my fat fingers to accidentally pause or activate the autodj.

poelzi avatar Feb 18 '21 15:02 poelzi

I think most DEs also map multimedia buttons on the keyboard to it.

Holzhaus avatar Feb 18 '21 16:02 Holzhaus

There are merge conflicts now.

Holzhaus avatar Apr 18 '21 14:04 Holzhaus

FWIW, I've been using a re-based version of this PR on top of main pretty regularly (https://github.com/aw-was-here/mixxx/tree/mpris2) with only one big change: adding xesam:url to the MPRIS2 output. I do have the occasional crash after many hours of runtime, but I haven't had a chance to track down where the crash is coming from and since it is main... . What needs to happen to push this PR forward?

Thanks!

aw-was-here avatar Jun 05 '21 18:06 aw-was-here

FWIW, I've been using a re-based version of this PR on top of main pretty regularly (https://github.com/aw-was-here/mixxx/tree/mpris2) with only one big change: adding xesam:url to the MPRIS2 output. I do have the occasional crash after many hours of runtime, but I haven't had a chance to track down where the crash is coming from and since it is main... . What needs to happen to push this PR forward?

Thanks!

I consider this branch too big to review. Someone just needs to take responsibility and split the change set into bite-sized, reviewable chunks. Each PR must contain an isolated, quantifiable improvement or extension with a description of the purpose. After the PR has been merged the remaining changes need to be rebased and so forth. Rinse and repeat, requires perseverance and patience. But otherwise the changes will be lost at some point.

uklotzde avatar Jun 05 '21 19:06 uklotzde

@uklotzde I spent a half day to review this PR and test it in every aspect. I found some bugs and some apis did at least not work with my mpris client. Otherwise, I review this and if my comments are fixed, I consider this mergeable.

poelzi avatar Jun 07 '21 12:06 poelzi

I am interested in the features you are trying to implement, but the external APIs you are sending data to are very specific (MPRIS , ListenBrainz).

My goal is to synchronise lighting with Mixxx : I need to know which song is playing and the precise elapsed time of the song Wouldn't it be a good idea to do this with a TCP socket instead ? A TCP socket offers great flexibility and allows to build pretty much anything to work with Mixxx.

Would such an API be a great idea given the current architecture of Mixxx ?

I'd like to implement it myself but it'll take a very long time because of my lack of my limited knowledge of cpp/qt.

I'm sorry if it's not the place to post this message, I'll move it if not.

storca avatar Jul 16 '21 12:07 storca

Yes sure you can easily hack something suitable together. However in a long run that leads to our own property API facing our own property problems.

This is not bad if there is nothing suitable out there, but if there is something, it is much better to use that. Because of available support in third party softwares and debugging tools and so on.

Which lighting application do you use? Does it offer an interface to DJ applications already?

What are you requirements regarding payload and timing?

daschuer avatar Jul 16 '21 14:07 daschuer

I don't understand the API property problems that could happen, the only problem I see is that we'd have to create our own protocol for communication within the socket? If it's not that, could you send me a link which explains it ?

A standard protocol for lighting that I know over TCP/IP is ArtNet, which is meant to work with DMX.

However, in my case, I'd like to synchronise a stroboscope with the music, by reading a file which contains the instant of every beat. So it's a homemade software.

~~The timing requirements are tight, DMX was not a solution because the refresh rate is 44Hz (22ms) and you would still be able to perceive a difference in beat / stroboscope synchronisation.~~ The other solution was GDB, which I got working eventually on linux, but it was not precise enough with the beats for me unfortunatly. I don't have a precise estimate on the timing requirements, I'll have to try for myself to see at what time delta the difference is gross. Edit : 35ms seems like a reasonable overall maximum, ArtNet + DMX could suit this problem

My idea was to provide an API to Mixxx via a static library (like MixxxConnect.dll on windows), which then connects to the TCP socket on the Mixxx app. What's great about this is that people could, in the end, have access to every aspect of Mixxx in more languages (Python) and could wrap pretty much anything around Mixxx.

Again, this is just a suggestion/question, maybe there are a ton of problems that I don't see.

storca avatar Jul 16 '21 15:07 storca

Using ArtNet would be incredibly hacky. A much better choice would be to provide an OSC server within Mixxx. There are a bunch of PRs for that, though all of them have stalled unfortunately.

Swiftb0y avatar Jul 16 '21 16:07 Swiftb0y

This is the 3rd or 4th time this has been asked in the past few weeks. It is up to someone who is interested in the feature to do the work of implementing it, but so far nobody has followed through on that. Please continue the discussion on Zulip; it's not directly related to this PR.

Be-ing avatar Jul 16 '21 17:07 Be-ing

This PR is marked as stale because it has been open 90 days with no activity.

github-actions[bot] avatar Oct 15 '21 00:10 github-actions[bot]

Can you rebase on / merge 2.3 so we get up-to-date CI?

Swiftb0y avatar Jul 30 '22 10:07 Swiftb0y

This is now on top of main, I have also taken care that most commits are still compiling. I plan to extract the ListemBrainz changes that @fatihemreyildiz is able to make use of it for his suggestion feature.

daschuer avatar Jul 31 '22 11:07 daschuer

So when is this getting merged?

Zipdox2 avatar Jan 28 '23 19:01 Zipdox2

When this has received a good amount of test and code review. Do you have interest to test help here? I also consider it to split the features in separate branches. Which feature do you need the most?

daschuer avatar Jan 30 '23 09:01 daschuer

Do you have interest to test help here?

You mean help test? I'm not sure what that entails.

I also consider it to split the features in separate branches. Which feature do you need the most?

D-Bus MPRIS

Zipdox2 avatar Jan 30 '23 11:01 Zipdox2

On which OS are you? The test may include building this branch and check if it meets your expectation or of there is anything bad happen.

daschuer avatar Jan 30 '23 12:01 daschuer

Debian sid. I already cloned the branch and built it. It seems to work fine, though I haven't run any tests.

Zipdox2 avatar Jan 30 '23 14:01 Zipdox2