Lib export with libdjinterop

[mr-smidge]

This PR introduces new functionality to allow a user to export their library to the Engine Library format, as used by Prime-series DJ equipment. It achieves this using an external library libdjinterop (disclaimer: I am the author of libdjinterop).

The functionality contained in this PR is in an early-but-hopefully-functional state, and a few aspects of the setup are not optimal. What is supported:

• Export of whole library or selected crates to Engine Library 1.7.1 format.
  • Schema version 1.7.1 is compatible with SC5000 players with firmware of 1.0.3 and above.
• Exports track files track meta-data, beat grid, cues, waveforms.
• Can be exported directly to disk, including a USB stick, for immediate use with external equipment.

Caveats:

• No support for album art at present (due to lack of support in libdjinterop itself at present).
• The libdjinterop library is not perfect, and should be considered still in beta.
• I have only built this PR on Ubuntu Linux (xenial and bionic). I'll try testing on Windows 10 (libdjinterop builds ok - haven't tried Mixxx with these enhancements yet). I don't have access to a MacOS development environment.

Motivation for submitting this PR in the current state:

• I am not a QT expert, nor a Mixxx expert, so I've had a learning curve to get this PR together. However, I do want to get something in place so that other developers who may be attracted to the functionality can contribute incremental improvements more easily.
• I am incredibly short on spare time: this work is a pure because-I-love-DJing endeavour. Getting something out can potentially give me a bit more time to work on the core libdjinterop library itself.
• I'd love to get feedback and more eyes on it :-).

It is not my expectation by any measure that the next official release of Mixxx contains this library export functionality: but I hope I/we can get it there soon :-).

[Be-ing]

Nice work! Thanks for sharing. I set this pull request as a Draft.

It is not my expectation by any measure that the next official release of Mixxx contains this library export functionality: but I hope I/we can get it there soon :-).

No problem, we'll get there when we get there.

[Holzhaus]

If you messed up the merge commit, please amend the commit and force push to ensure that every commit is still buildable. Otherwise it will cause problems when bisecting issues.

I recommend to use git add -p and setting commit.verbose to true in your git config to avoid committing unintended changes in the future.

[mr-smidge]

If you messed up the merge commit, please amend the commit and force push to ensure that every commit is still buildable. Otherwise it will cause problems when bisecting issues.

I recommend to use git add -p and setting commit.verbose to true in your git config to avoid committing unintended changes in the future.

Thank you for the suggestion @Holzhaus - although I do not think I would have caught the issues with verbose commits turned on, as there were ~600 individual commits in the merge from master. I believe the regression came about through git's auto-merge.

I have just tried rebasing, only to discover that some of those 600 commits don't even rebase cleanly amongst themselves. I'll try to minimise the number of "fixup" commits as best I can.

[Holzhaus]

You can reset your local branch to the state you pushed last (via git reset --hard origin/lib_export_with_libdjinterop) and only go back 4 commits to fix the broken merge with git rebase -i HEAD~4.

WARNING: Doing a hard reset will undo all your local changes in the working tree that you didn't push yet. Better make a backup via git branch -c mybackup.

[uklotzde]

Please don't rebase any commits from master! Otherwise we cannot merge this. Instead rebase your branch on master and ensure that all commits on your branch are still sane, i.e. buildable.

[Be-ing]

There is potential for a lot of confusion with terminology here. We have a loose concept of the "engine" in Mixxx which somewhat corresponds to the audio processing (but not really) and here you are using "Engine" to refer to something entirely different. Can you please use "EnginePrime" in file and class names?

[mr-smidge]

There is potential for a lot of confusion with terminology here. We have a loose concept of the "engine" in Mixxx which somewhat corresponds to the audio processing (but not really) and here you are using "Engine" to refer to something entirely different. Can you please use "EnginePrime" in file and class names?

I'm not precious about the class naming, so I can change this.

Note that the actual database format, technically, is simply called "Engine" (great name 🤦), the library on disk is always called "Engine Library", whereas Engine Prime is the name of an application. However, even within the official ecosystem, "Engine" also refers to similar older software that preceded Engine Prime. To add to the confusion, Engine Prime is starting to be referred to as Engine in some official websites too... ugh.

[mr-smidge]

Can you please use "EnginePrime" in file and class names?

Now renamed throughout.

[mr-smidge]

Travis is showing a test failure that I can't reproduce locally, and which looks like it might be transient:

        Start 182: EngineBufferScaleLinearTest.ScaleConstant
182/617 Test #182: EngineBufferScaleLinearTest.ScaleConstant 
.......................................SIGPIPE***Exception:   0.07 sec

@Be-ing, what do you recommend in this situation? Is there a convenient way to re-run CI builds without faking a commit?

[Be-ing]

Sorry, we've been having issues with spurious test failures for a while. Unfortunately no one has figured out what is going wrong. I manually restarted the Travis build.

[mr-smidge]

Sorry, we've been having issues with spurious test failures for a while. Unfortunately no one has figured out what is going wrong. I manually restarted the Travis build.

@Be-ing, please could you re-run the Travis and AppVeyor builds for this PR? The Travis build has again failed a unit test for an odd reason (qt.qpa.screen: QXcbConnection: Could not connect to display :99.0), and the AppVeyor VS2017 build appears to have timed out downloading the environment. I can't reproduce either of these issues locally.

If all is well, I'll mark this as Ready for Review.

Thanks!

[Holzhaus]

I restarted the CI builds.

[mr-smidge]

@Be-ing / @uklotzde / @Holzhaus - please may I check if you have any outstanding concerns with this PR? Thanks.

[AliceGrey]

Tried this branch today and got a segfault when running mixxx.

Backtrace:

Thread 1 "mixxx" received signal SIGSEGV, Segmentation fault.
0x00007ffff3f2925c in __memmove_avx_unaligned_erms () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff3f2925c in __memmove_avx_unaligned_erms () at /usr/lib/libc.so.6
#1  0x00007ffff7e40042 in sqlite3StrAccumAppend (p=0x7fffffffcf40, z=0x7fffffffceb0 "4.63557153419868e-310", N=21) at ../src/sqlite3.c:26394
#2  0x00007ffff7e3fc38 in sqlite3VXPrintf (pAccum=0x7fffffffcf40, fmt=0x7ffff7ef13f5 "g", ap=0x7fffffffcfb0) at ../src/sqlite3.c:26288
#3  0x00007ffff7e405b1 in sqlite3_vsnprintf
    (n=32, zBuf=0x200010202 <error: Cannot access memory at address 0x200010202>, zFormat=0x7ffff7ef13f0 "%!.15g", ap=0x7fffffffcfb0) at ../src/sqlite3.c:26566
#4  0x00007ffff7e40693 in sqlite3_snprintf (n=32, zBuf=0x200010202 <error: Cannot access memory at address 0x200010202>, zFormat=0x7ffff7ef13f0 "%!.15g")
    at ../src/sqlite3.c:26574
#5  0x00007ffff7e6910c in sqlite3VdbeMemStringify (pMem=0x5555569b90d8, enc=2 '\002', bForce=0 '\000') at ../src/sqlite3.c:70752
#6  0x00007ffff7e6a37f in valueToText (pVal=0x5555569b90d8, enc=2 '\002') at ../src/sqlite3.c:71501
#7  0x00007ffff7e6a41e in sqlite3ValueText (pVal=0x5555569b90d8, enc=2 '\002') at ../src/sqlite3.c:71534
#8  0x00007ffff7e71b2a in sqlite3_value_text16 (pVal=0x5555569b90d8) at ../src/sqlite3.c:77127
#9  0x00007ffff4a20966 in  () at /usr/lib/libsqlite3.so.0
#10 0x00007fffe0027a74 in  () at /usr/lib/qt/plugins/sqldrivers/libqsqlite.so
#11 0x00007fffe002ac2a in  () at /usr/lib/qt/plugins/sqldrivers/libqsqlite.so
#12 0x00007fffe002af92 in  () at /usr/lib/qt/plugins/sqldrivers/libqsqlite.so
#13 0x00007ffff6981f76 in QSqlQuery::exec() () at /usr/lib/libQt5Sql.so.5
#14 0x0000555555ba8632 in SettingsDAO::getValue(QString const&, QString) const ()
#15 0x00005555559d0893 in SchemaManager::SchemaManager(QSqlDatabase const&) ()
#16 0x00005555559cfbcc in MixxxDb::initDatabaseSchema(QSqlDatabase const&, QString const&, int) ()
#17 0x00005555556d4e1d in MixxxMainWindow::initializeDatabase() ()
#18 0x00005555556d9866 in MixxxMainWindow::initialize(QApplication*, CmdlineArgs const&) ()
#19 0x00005555556dc631 in MixxxMainWindow::MixxxMainWindow(QApplication*, CmdlineArgs const&) ()
#20 0x00005555556ac724 in main ()

[Be-ing]

Build succeeded! 🎉 @uklotzde anything left to do before merging?

[uklotzde]

No objections, ready to merge.

@Holzhaus ?

[uklotzde]

@daschuer Please take care of merging 2.3 to main before we merge major new features into main.

[Be-ing]

Thanks for this new feature @mr-smidge! And thank you for your patience with the review. Please continue to open more PRs as you improve the performance.

[mr-smidge]

Thanks to everyone who looked at this PR - it's my first "proper" contribution to Mixxx (and definitely not my last!), and I appreciate it's a lot of effort to review and advise on the right way forward. Really pleased to have got to the end of the marathon!

I am going to keep chipping away at the enhancements for libdjinterop (fingers crossed maybe a few more contributors might be able to help with this now merged into Mixxx!), and will certainly open more PRs to improve the experience and functionality all round.

[Be-ing]

@mr-smidge would you like to write a post for the mixxx.org blog about this new feature? The code for the website is at https://github.com/mixxxdj/website and the blog posts use Markdown.

[mr-smidge]

@mr-smidge would you like to write a post for the mixxx.org blog about this new feature? The code for the website is at https://github.com/mixxxdj/website and the blog posts use Markdown.

Sure, I'll try to put something together in the next week - thanks!

[daschuer]

Unfortunatly this breaks the PPA build:

-- Could NOT find DjInterop (missing: DjInterop_LIBRARY DjInterop_INCLUDE_DIR) 
-- Building libdjinterop sources (with system SQLite) fetched from GitHub
CMake Error at /usr/share/cmake-3.13/Modules/ExternalProject.cmake:2329 (message):
  error: could not find git for clone of libdjinterop
Call Stack (most recent call first):
  /usr/share/cmake-3.13/Modules/ExternalProject.cmake:3105 (_ep_add_download_command)
  CMakeLists.txt:1623 (ExternalProject_Add)

[Be-ing]

Does the referenced hash no longer exist? @mr-smidge did you force push to the libdjinterop repo?

[mr-smidge]

Hmmm, the relevant portion of CMakeLists.txt is:

    ExternalProject_Add(libdjinterop
      GIT_REPOSITORY https://github.com/xsco/libdjinterop.git
      GIT_TAG tags/0.14.6
      ...

The tag definitely exists: https://github.com/xsco/libdjinterop/releases/tag/0.14.6

[uklotzde]

Blog post: Let's publish the post not before main is back in a usable state. Currently there are massive issues with cue objects and control objects that cause debug assertions.

[uklotzde]

...not caused by this PR ;)

[uklotzde]

Integration into the PPA build should work the same way as it does for libkeyfinder.

[mr-smidge]

@daschuer the error message error: could not find git for clone of libdjinterop suggests that git itself can't be found.

How does the PPA build work? Is there anything special to include git?

I notice that keyfinder is a plain old download, rather than a git clone:

    ExternalProject_Add(libkeyfinder
      URL "https://github.com/mixxxdj/libkeyfinder/archive/v2.2.3.zip"
      URL_HASH SHA256=ad43ca006e3bbed0810ff62e170d04522a64f8606c2166bfa5a9b9158b7ebc11
      DOWNLOAD_DIR "${CMAKE_CURRENT_BINARY_DIR}/download/libkeyfinder"
      ...

[uklotzde]

The same applies to the RPM builds. All sources must be available and placed in the download folder.

[mr-smidge]

If the CMakeLists.txt file used a download of https://github.com/xsco/libdjinterop/archive/0.14.6.tar.gz instead of a git clone (and placed it in the download dir), would that resolve issues?

(I have no prior experience building PPAs or RPMs, but obviously happy to fix things I've broken where I can!)

[uklotzde]

The download and packaging of additional sources is part of the DEB or RPM builds and does not belong into CMakeLists.txt. It's the responsibility of the maintainer to decide what gets packaged and which features are enabled.

[uklotzde]

As a quick fix we could add -DENGINEPRIME=OFF to the Debian build.

[Be-ing]

No, let's not procrastinate. Fix the real issue.

[uklotzde]

First fix the build, then modify it: #3595

libKeyFinder is also still disabled (due to licensing issues I guess)

[Be-ing]

libKeyFinder is also still disabled (due to licensing issues I guess)

This is fixed in #3476. We have already been distributing libkeyfinder without updating Mixxx's license.

[poelzi]

@mr-smidge if you want to, you could add the interop option to the new export improvement branch: #3572
I hope we can unify the export in one place that will unify all the possible options and later also add transcoding and other useful features.

[uklotzde]

I suggest to keep the export where it is until #3572 has been merged. Don't needlessly extend the scope of a PR!

[mr-smidge]

@poelzi It sounds like a good idea to consolidate the idea of exporting in one place in the GUI, but I do agree with Uwe that it might be better done as a follow-up to your PR rather than changing it in-flight - the scope could easily expand!

[uklotzde]

Master should soon be usable again after merging #3599.

Yet, we should enable this feature in the DEB/PPA builds before publishing the blog post.