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

[proposal#1] Library plugins #2

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

acolombier
Copy link
Member

@acolombier acolombier commented Mar 24, 2024

Discussion on Zulip

Links to:

@daschuer
Copy link
Member

daschuer commented Mar 24, 2024

Thank your for the proposal. I like the idea to easily import and export playlists from various local and cloud storage. An export feature would be also more than welcome.

However I have some doubts that the proposed mode is a good idea. Here my points.

1.) The proposed API is Mixxx proprietary and there fore all work will be done with the help of the Mixxx team and will associated with Mixxx even though it is kept in a private repository. Possible workaround:
a) Do the development with in the Mixxx repository and care for external licenses. That does not mean required using C++ or being less generic. It is a good Idea to set a common ground for integration any external library.
b) Adopt a third party generic interface for sharing playlists (Mopidy, Tomahawk, Clementine)

2.) A generic API will always limit any particular service until it is is truly generic. We have already a few external libraries integrated which have all some special hard-coded features. So we may consider to add the Spotify library the same way.
(I have just skimmed over there terms and this seems not to violate any terms)
Refactoring of the external library integration is anyway overdue and a welcome addition.

3.) The generic streaming audio integration should be no part of this proposal, to avoid future trouble.

4.) This proposal does not cover metadata extraction for exiting tracks in the Mixxx library. I think we need at least design a stub for it, that matches external with local tracks.

@daschuer
Copy link
Member

Just had a quick look to the linked POC code and it looks like a good foundation for my 1a.) above. I have no objections to merge the prerequisites as single PRs step by step. The big issue Mixxx always suffers from is the missing SQLite thread. Maybe we can develop ideas to overcome that as well.

@acolombier
Copy link
Member Author

1.) The proposed API is Mixxx proprietary and there fore all work will be done with the help of the Mixxx team and will associated with Mixxx even though it is kept in a private repository. Possible workaround: a) Do the development with in the Mixxx repository and care for external licenses. That does not mean required using C++ or being less generic. It is a good Idea to set a common ground for integration any external library.

I'm quite clueless about licenses so I could get that wrong, but could publish the spec under another license defined in the spec file itself like in the gRPC repo? Since this is something part of new work, I would imagine it does need any agreement with past contributor, or is that off the table? This way, Mixxx could implement that spec, under GPLv2 like the rest of the codebase, but the spec itself referenced in other plugin project like in the example one can be safe to be distributed under other licenses, in order to accommodate plugin developers or dependencies they may use?

2.) A generic API will always limit any particular service until it is is truly generic.

Agreed. I am 100% sure the current spec I am providing is missing usecase that will arise in the future. However, I want to hope that gRPC is a good fit for a rapidly involving spec, which ensuring backward compatibility with plugin developed on past version. Perhaps the term "generic" isn't very well chosen; What I meant by that is something unopinionated that can work with any services, and isn't "just" tailored for a specific service or third party.

We have already a few external libraries integrated which have all some special hard-coded features. So we may consider to add the Spotify library the same way. (I have just skimmed over there terms and this seems not to violate any terms)

I have considered this, but IMHO this would require quite some ground work to enable easy interfacing with non C/C++ libraries. Not impossible of course, but potentially will add complexity to compilation for a feature that not everyone wants to use (I think we've seen a fair amount of dislike toward Spotify from the community on Zulip, which is fair). It could go under a feature flag obviously, but then the distribution to non-technical user become an other challenge.

3.) The generic streaming audio integration should be no part of this proposal, to avoid future trouble.

Roger that - are you saying it should be considered as a next proposal/iteration or are you saying this is a clear no go? I fill like there is usecase for streaming file integration in Mixxx (potentially not only audio, but also meta file like covers)

4.) This proposal does not cover metadata extraction for exiting tracks in the Mixxx library. I think we need at least design a stub for it, that .

I'm not sure what you mean by that? In the current demo/poc, there is metadata communication, which also gets persisted in the database? Note that some of those screenshots show empty line in the tracklist / empty fields, result of crash which I didn't clean up

matches external with local tracks

Currently, the setup for it was to delegate that to the plugin (which could return path to a file on the file system, matched later one by Mixxx using the DAO getTrackByLocation (or something like that). I feel like making that matching within Mixxx maybe a source of trouble as metadata may often be incomplete or non matching between steaming service and local filesystem (current external integration also rely on track, I guess for a similar reason)

@daschuer
Copy link
Member

Licenses issue

This Wikipeda https://en.wikipedia.org/wiki/License_compatibility link is useful.

We cannot distribute Mixxx with a library that is not compatible to our GPL 2.0 and feature set. This is for example a GPL 3.0. or a library that puts other restrictions to Mixxx like forbidding DJ-ing. The user can hover combine any libraries without distributing this combination.

If we publish a Plug-In stub with the Mixxx license, the glue code between that and an external library needs to be also open source. If we use for example the MIT license for the Plug-In stub, a third party is able to develop a closed source plug-In. That gives a lot of freedom to potentially commercial partners but limits the rights our users.

The Apache 2.0 license of gRPC is not GPL 2.0 compatible due to the patent clauses. There is however a pending discussion to change to GPL 3.0 for Mixxx.

So we may consider to add the Spotify library the same way.

I have considered this, but IMHO this would require quite some ground work to enable easy interfacing with non C/C++ libraries.

Oh sorry, I was not clear enough. Using gRPC makes perfectly sense in this case. I just meant to integrate it directly into the Mixxx repository without aiming to develop a generic solution at the same time.

I fill like there is usecase for streaming file integration in Mixxx.

I fully agree. However a solution needs to be fully generic, via Aux routing or via a third party eco system. I don't want to see a Plugin in connection with Mixxx that violates specific cloud service rules.

This proposal does not cover metadata extraction ...

I'm not sure what you mean by that?

I mean adding the metadata gathered from Spotify to tracks you already have in you Mixxx library. This requires a to recognize tracks by more than just the Artist and Title. Sotify may have a different remix/recording than local or Spotify may have different spelling for the same remix/recording. This is also helpful for importing and exporting playlists, or marking Spotify track suggestions as "available".

Currently, the setup for it was to delegate that to the plugin

That would expose user data to the plug-In. Probably not desired if we also allow closed source plugins.

current external integration also rely on track

They rely on the path on the HDD, which is a unique identifier for all local libraries. This is no longer true for cloud libraries.

@Swiftb0y
Copy link
Member

@acolombier would you mind rebasing on the current state?

@acolombier acolombier force-pushed the library-plugins branch 2 times, most recently from d0fe882 to 12c6fa6 Compare May 23, 2024 21:20
@acolombier acolombier force-pushed the library-plugins branch 2 times, most recently from b7ee83a to 577b0a1 Compare June 4, 2024 14:48
@acolombier acolombier marked this pull request as draft July 15, 2024 12:54
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