Add support for SeratoMarkers2 GEOB tag (Hotcues/Loops/Flips/etc.)

[Holzhaus]

This is a WIP to add support for the SeratoMarkers2 GEOB tag. This tag contains cuepoints, saved loops, "flips" and other information and is used by the Serato DJ Pro software.

It's nowhere near completion, but due to lack of free time I was unable to invest a lot of time into this. Help is very much appreciated.

I'm unsure how to integrate support properly. I'd like Mixxx to read cues from the tag when a track is loaded into a deck (or its cues are edited via the right click dialog), but also write tags back to the file if anything is changed (this can be made optional).

This is not only useful for former Serato users that have a big library and want to migrate cue points to Mixxx, but also for use cases where multiple computers are used: I usually play around with Mixxx on my desktop PC (testing out transitions, settings cuepoints, etc). Before a gig, I then rsync the tracks I want to play over to my laptop. However, all cuepoints are lost in the process (I could copy over the library, search/replace all paths and remove non-copied tracks but this is cumbersome).
Support for the SeratoMarkers2 tag would allow me to just copy over the MP3 file without losing cuepoints.

Launchpad bug: https://bugs.launchpad.net/mixxx/+bug/741613
Writeup: http://homepage.ruhr-uni-bochum.de/jan.holthuis/posts/reversing-seratos-geob-tags
Documentation: https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers2.md

[Be-ing]

Thank you for publishing this. Hopefully someone else can pick up where you left off.

[uklotzde]

I'll provide a fixed and rebased version soon. Waiting for CI builds.

[Holzhaus]

@uklotzde I added you as a collaborator to my mixxx repository to grant you write permissions (in case you don't want to open a new PR).

[uklotzde]

https://github.com/uklotzde/mixxx/tree/serato-markers2

• Rebased on master
• Code for reading/writing of GEOB tags hidden behind the build flag _EXTRA_METADATA_
• Added workarounds for Qt <5.12, i.e. the branch should build on Xenial (Travis CI)
• Minor fixes and cleanup

@Holzhaus I could do a force push into your repo, but maybe you first make a backup of your original branch before replacing it.

[Holzhaus]

@uklotzde I created a copy of this branch here: https://github.com/Holzhaus/mixxx/tree/serato-markers2-backup

[uklotzde]

We can merge this PR right now to complete and enable this feature later. Importing and parsing of tags are still disabled.

Any thoughts?

[Be-ing]

What is the point of merging code that is not useful yet?

[uklotzde]

@Be-ing To keep it up-to-date and allow someone to pick it up? Letting it rot in a PR that piles up merge conflicts would be a pity. This is definitely something that will be needed eventually.

[Be-ing]

Okay, as long as there are some comments in the code documenting what the current state is and giving some hints on next steps to whoever may pick it up.

[Holzhaus]

@Be-ing I just checked: Serato DJ Pro, Serato DJ Lite and Serato DJ Intro all use this format to store their information.

[uklotzde]

@Be-ing @Holzhaus I would like to merge this soon, because I have some metadata handling improvements ready that would cause merge conflicts and need to be resolved first.

[Be-ing]

That's fine with me if @Holzhaus is okay with merging it at this point.

[Holzhaus]

Yup, this is disabled anyway by default, parse/unparse tests are in place and pass. Feel free to merge.

[uklotzde]

LGTM.

Tests are in place and the extra code only adds a small compile time but no runtime overhead.

[Holzhaus]

By the way, shouldn't we add __EXTRA_METADATA__ to the CI builds? We don't want it in the release builds, but CI should check it anyway to avoid broken code in master.

[Holzhaus]

This only adds support for Serato data stored in for ID3v2 tags. I started to document other file formats here: https://github.com/Holzhaus/serato-tags/blob/master/docs/fileformats.md

[uklotzde]

CI builds: Currently we don't have any build option in master that needs and enables those extra tags. But #2282 is in place and I will rebase it continually on master.

[bungrudi]

I really feel that this work deserve much more attention.

To give some context, I just started into DJ-ing and looking into Serato alternative that will allow me to leverage my programming skill (i.e. into programming MIDI script/automation). I'm a programmer in my day job and naturally feel that I should be able to automate many things (... which currently requires kung-fu skill to pull off in Serato), especially that I have a Novation Launchpad as a secondary controller.

I just found and tried Mixxx yesterday and immediately felt that this (the interoperability with Serato database) is exactly the missing feature that this app need, and what prevented me to use it currently.

My controller came with Serato license and I invested a lot of time and money on the software. I have all my tracks tagged on Serato (hotcue, hot loops, keys, etc). I invested in a tool that will analyze and put automatic cue points as Serato data (Mixed in Key, $58).

VirtualDJ is almost perfect, except that its scripting language is super clunky and the license price is steep at $299.
Having VirtualDJ on trial for a month, I can almost say for certainty that one reason for its success is its ability to work with Serato data format; in both direction.

My hunch tells me that Mixxx will took off once it implements the same feature.

BTW good work @Holzhaus .

@Be-ing To keep it up-to-date and allow someone to pick it up? Letting it rot in a PR that piles up merge conflicts would be a pity. This is definitely something that will be needed eventually.

[Be-ing]

@bungrudi if you want to work on completing this, that would be great! @Holzhaus outlined what is left to do above.

[bungrudi]

@Be-ing I'm a spoilt Java + C# coder.. you don't want me to introduce leaks :D
Can I somehow code it in a sandboxed, ephemeral environment i.e. in the Javascript engine ?

[Be-ing]

No, this would have to be done by editing Mixxx's C++ code. I think if you know Java and C# it would not be so difficult to pick up C++. We have guides on the Mixxx wiki for setting up some IDEs to work on Mixxx. Even if you were an experienced C++ coder, we would review your code before merging it, so don't let making stupid mistakes stop you.

[bungrudi]

I started with C++, but its been 10 years so please understand that I will be very rusty. Also this will be a a weekend activity for me so if I do pick it up you won't see any commit from me until the next 2 weeks probably.
I'll branch and setup my environment later today and see what I can do.

[Be-ing]

That's fine; we're all volunteers here too. Feel free to ask on our Zulip chat if you need help getting started. I think we will be busy working on other features anyway until you have some commits to show.

Fortunately C++ has improved a bit since 10 years ago. You don't need to worry about most fancy new language features that are rarely used, but some common tasks are nicer now (like iterating container objects and having an explicit nullptr value). You will probably need to refer to the Qt documentation when working on Mixxx.

[uklotzde]

@bungrudi Please note that I have some pending metadata updates and fixes queued in my personal branch dev_library_metadata (frequently rebased) that depend on PR #2365.

[Be-ing]

How does that impact the work to be done to complete this feature? Should @bungrudi start from #2365? Or start from your dev_library_metadata branch?

[uklotzde]

Since I am rebasing this branch often I don't recommend to base new work on it now. Just check frequently that no merge conflicts arise.

[Be-ing]

It looks like #2365 will make a big impact on the remaining tasks for this feature. I think it would make sense for @bungrudi to start working from that or wait for that to be merged. I will prioritize reviewing #2365 to not hold up @bungrudi's work.

[bungrudi]

Noted that @uklotzde .
Currently I am basing on master, let me know if i have to rebase to #2365 @Be-ing .

[uklotzde]

@bungrudi You can check for arising merge conflicts by trying to merge the corresponding branch from time to time. Just revert back to your last commit afterwards.

[Holzhaus]

@bungrudi If you don't want to deal with merge conflict problems and reverting commits just do this:

$ git remote add uklotzde https://github.com/uklotzde/mixxx.git  # Only do this once
# Develop your feature in branch "my-feature-branch". When you want to test if your changes cause merge conflicts, do this:
$ git checkout -b my-feature-branch-mergetest  # Branch off from your feature branch
$ git fetch uklotzde && git merge uklotzde/dev_library_metadata  # Merge changes from remote uklotzde
$ git merge --abort  # Only do this if a merge conflict occurs
$ git checkout my-feature-branch  # Switch back to your feature branch
$ git branch -D my-feature-branch-mergetest  # Delete the mergetest branch

[Be-ing]

@bungrudi if you did not see, #2365 was merged to master.

[Holzhaus]

@bungrudi Did you already start working on the integration?

[Holzhaus]

@bungrudi If you're still interested, here's a first quick&dirty hack to read hotcues from metadata: master...Holzhaus:serato-markers2-integration