Read Cues from Serato MP3 Tags #2526

Holzhaus · 2020-02-29T15:35:26Z

This is based on PRs #2499 and #2523.

src/track/serato/tags.cpp

Holzhaus · 2020-03-10T14:44:40Z

src/track/serato/tags.cpp

+        switch (timingShiftCase) {
+#if defined(__MAD__)
+        case EXIT_CODE_CASE_D:
+            timingOffset = -16;


@ywwg Here's an example how to set timing offsets btw.

src/track/track.cpp

uklotzde · 2020-03-20T13:04:03Z

Unrelated warning:

In file included from /tmp/mixxx/src/track/serato/markers.cpp:1:
/tmp/mixxx/src/track/serato/markers.h:94:10: warning: private field 'm_isSet' is not used [-Wunused-private-field]
    bool m_isSet;
         ^
1 warning generated.

uklotzde · 2020-03-21T10:42:26Z

I have some ideas how to explicitly handle the discrepancy between file tags and stream properties of track objects. Stay tuned.

Be-ing · 2020-03-23T04:56:41Z

Has anyone looked into the cue offset issues with this? @ehendrikd maybe you could help? If you don't want to directly work on this, maybe sending @Holzhaus the audio files you tested with Rekordbox and Mixxx would be helpful.

ehendrikd · 2020-03-23T05:17:14Z

Here is a selection of known MP3 offset issue tracks, categorized in to cases A, B, C and D:

https://github.com/pestrela/music/tree/master/traktor/26ms_offsets/examples_tagged

As per ABCD cases defined here:

digital-dj-tools/dj-data-converter#3

And this was the final algorithm I settled on:

#2119 (comment)

mixxx/src/library/rekordbox/rekordboxfeature.cpp

Line 1045 in 78a8640

// The following code accounts for timing offsets required to

uklotzde · 2020-03-23T21:15:44Z

Sorry, but the Xenial issues are blocking me. I will not introduce more workarounds and make unnecessary compromises.

daschuer · 2020-03-25T11:53:27Z

Why is a commented feature a beta blocker?
While the feature is nice, we can introduce it at any time later.

Holzhaus · 2020-03-25T16:39:55Z

Here is a selection of known MP3 offset issue tracks, categorized in to cases A, B, C and D:

https://github.com/pestrela/music/tree/master/traktor/26ms_offsets/examples_tagged

I checked and the offset seems to be -18 ms in all cases for the MAD decoder on linux.

Can someone check the other offsets? I don't have a mac and my windows VM is too slow for audio playback on Serato for some reason.

If you want to test, you need to apply this patch first:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 912c3d2a6f..d99fd3741e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -730,6 +730,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
 set_target_properties(mixxx-lib PROPERTIES AUTOMOC ON AUTOUIC ON)
 target_include_directories(mixxx-lib PUBLIC src "${CMAKE_CURRENT_BINARY_DIR}/src")
 target_compile_definitions(mixxx-lib PRIVATE SETTINGS_FILE="mixxx.cfg")
+target_compile_definitions(mixxx-lib PUBLIC __EXTRA_METADATA__)
 if(UNIX AND NOT APPLE)
   target_compile_definitions(mixxx-lib PRIVATE SETTINGS_PATH=".mixxx/")
 endif()

Be-ing · 2020-03-25T17:06:20Z

@JosepMaJAZ maybe you could test on Windows? @ferranpujolcamins could you test on macOS? Serato DJ Lite does not cost any money and doesn't require any Serato specific hardware.

Can we clarify how exactly to test? Load each file from https://github.com/pestrela/music/tree/master/traktor/26ms_offsets/examples_tagged into Serato, set a cue point on each, then use this branch to import into Mixxx? How are you determining the timing offsets? The deck display in Mixxx is only precise to 10 ms.

Swiftb0y · 2020-03-25T17:08:49Z

And maybe also test it with ffmpeg for mp3 and aac?

Holzhaus · 2020-03-25T17:47:01Z

I set cue points in serato and checked visually if they are at the correct position in Mixxx. I wasn't aware that the waveform display is not accurate. Is there a better way do this?

Be-ing · 2020-03-25T17:49:31Z

I was referring to the time display in the decks:

It only goes to 0.00 seconds or 10 ms precision.

Be-ing · 2020-03-25T17:51:41Z

How did you compare to determine 18 ms is the offset? Did you write code to guess and check until the waveforms looked exactly the same?

Be-ing · 2020-03-25T17:59:29Z

@Holzhaus you can test for the FFMPEG decoder on Linux by editing SoundSourceProviderFFmpeg::getPriorityHint in src/soundsources/soundsourceffmpeg.cpp.

Be-ing · 2020-03-25T18:08:12Z

We use libmad (SoundSourceMp3) for both Linux and Windows, right? So I don't anticipate any difference between Linux and Windows. On macOS we use SoundSourceCoreAudio for MP3 so we do need someone with a Mac to test that. Judging from the offsets for Rekordbox, it is likely CoreAudio will not be the same as libmad.

ywwg · 2020-03-25T19:49:16Z

Why is a commented feature a beta blocker?

it looked like it was designated such from the project page. @Holzhaus would it be ok to take this off the beta-blocking list?

Be-ing · 2020-03-25T19:58:17Z

My understanding is that there is only a little bit left to do here to correct the cue offsets (correct me if I am wrong). The hard work of reverse engineering the file tags is already merged. I think it would be a shame to release this feature in a half-baked state with the Serato library support for playlists without being able to read Serato cues. Releasing it in that state would raise the hopes of lots of people who would quickly be disappointed because it isn't practically all that beneficial. It would also be a shame to disable the Serato library feature to avoid disappointing those users. So I think we should push to finish this last part for the 2.3 beta release.

Holzhaus · 2020-03-25T20:42:25Z

How did you compare to determine 18 ms is the offset? Did you write code to guess and check until the waveforms looked exactly the same?

I set serato to -50% bpm (so that the waveform is zoomed to maximum), then I set hotcues at positions were the waveform suddenly changes (e. g. when the beat starts or something like that). Then I eject the track from the deck to trigger a Metadata write.
I open the file in Mixxx, zoom in the waveform and check if the hotcue position is ahead or behind, then guesstimate an offset and put it in the code, recompile and restart mixxx and reimport Metadata. Repeat until it visually looks the same.

Be-ing · 2020-03-25T21:27:41Z

Shouldn't we get rid of the __EXTRA_METADATA__ guards in this PR now?

src/track/serato/tags.cpp

Be-ing · 2020-04-17T19:38:04Z

Your documentation says AIFF uses the same ID3 tags as MP3. Does this PR also work for AIFF files?

Holzhaus · 2020-04-17T19:41:22Z

I have no idea, I converted the a file into all supported formats, loaded them in Serato and had a quick glance at the tags written. I didn't really try to reverse them.

uklotzde · 2020-04-17T19:53:16Z

Your documentation says AIFF uses the same ID3 tags as MP3. Does this PR also work for AIFF files?

AIFF files could have ID3v2 tags or RIFF tags. For ID3v2 tags it works, for RIFF tags no. I could check if Serato is writing its markers into RIFF INFO tags.

…-integration-cues

Holzhaus · 2020-04-19T20:48:13Z

I guess the lack of Windows/OSX testers is probably due to the buggy waveforms. Until that is fixed, can at least someone check the offsets on Linux? Or shall we postpone this to 2.4? I feel like we're not making progress here.

Be-ing · 2020-04-20T00:21:44Z

@bengl3rt will you have time to find the offsets for macOS soon?

ehendrikd · 2020-04-20T13:33:16Z

I’m also now having a look into this on macOS, think a separate CoreAudio section for each MP3 case as suggested by @Be-ing will be needed. Would it be best to create a PR to @Holzhaus branch once I have completed this with the new CoreAudio offsets?

Be-ing · 2020-04-20T13:37:40Z

Great, thanks for taking this up @ehendrikd. Yes, make a PR for @Holzhaus's fork. Please leave a comment here with a link to your PR so all Mixxx contributors can find it.

ehendrikd · 2020-04-21T13:12:34Z

Think I have the offsets set for CoreAudio. Holzhaus#4

…coreaudio CoreAudio MP3 offsets for Serato cues

Be-ing · 2020-04-21T23:09:06Z

Ready for merge?

Holzhaus · 2020-04-21T23:34:35Z

No windows offsets yet. Also, I'd really like someone to verify my Linux findings. It's strange that all values are the same...

Be-ing · 2020-04-21T23:47:29Z

Mixxx uses libmad (SoundSourceMP3) on both Windows and Linux, so the offsets should be the same on both. Can you confirm that is correct, @uklotzde?

Be-ing · 2020-04-22T00:12:08Z

It's possible that Serato also uses libmad which is why there are no offsets to correct for on Linux and Windows.

From the MAD homepage:

MAD is available under the terms of the GNU General Public License, Version 2, for either permanent use or for evaluation prior to obtaining a commercial license. Please note that under the GPL, there is absolutely no warranty of any kind.
Commercial, non-GPL licensing is also available. Please contact us to discuss possible license terms.

Be-ing · 2020-04-22T00:12:46Z

@Holzhaus maybe you could do a little digging to search for libmad symbols in the Serato binary?

Be-ing · 2020-04-22T00:33:30Z

Ha! What an interesting find. In the "Applications Using MAD" section on the libMAD homepage, it lists Stanton Final Scratch Live, one of the first DVS programs first released in 2004. Serato Scratch Live was also first released in 2004, so it wouldn't surprise me if they also bought a proprietary license for libMAD around the same time.

uklotzde · 2020-04-22T06:27:56Z

Mixxx uses libmad (SoundSourceMP3) on both Windows and Linux, so the offsets should be the same on both. Can you confirm that is correct, @uklotzde?

That's correct.

Holzhaus · 2020-04-22T06:52:39Z

Ok, then lets merge and I'll rebase #2673 on master for the loop import.

Holzhaus added the library label Feb 29, 2020

Holzhaus force-pushed the serato-markers-integration-cues branch 2 times, most recently from 7ed1cb3 to 6287684 Compare March 5, 2020 12:40

Holzhaus commented Mar 6, 2020

View reviewed changes

src/track/serato/tags.cpp Outdated Show resolved Hide resolved

Holzhaus commented Mar 10, 2020

View reviewed changes

Holzhaus commented Mar 12, 2020

View reviewed changes

src/track/track.cpp Outdated Show resolved Hide resolved

Be-ing added this to the 2.3.0 milestone Mar 24, 2020

Be-ing added this to In progress in 2.3 release via automation Mar 24, 2020

Be-ing moved this from In progress to beta blockers in 2.3 release Mar 24, 2020

ywwg added the blocker label Mar 24, 2020

ywwg moved this from beta blockers to In progress in 2.3 release Mar 24, 2020

Be-ing reviewed Mar 25, 2020

View reviewed changes

src/track/serato/tags.cpp Show resolved Hide resolved

This was referenced Apr 17, 2020

track/serato/markers: Fix parsing of position values #2671

Merged

Add qDebug << operators for CueType and CueInfo #2672

Merged

Import saved loops from Serato Tags #2673

Merged

Holzhaus added 3 commits April 17, 2020 23:47

Merge branch 'master' of github.com:mixxxdj/mixxx into serato-markers…

499cd3b

…-integration-cues

track/serato/tags: Improve import logic

04f5bea

track/serato/tags: Apply very slight correction to timing offsets

231c686

Be-ing mentioned this pull request Apr 21, 2020

Fix load/save of most recent loop #2678

Merged

CoreAudio MP3 offsets for Serato cues

60120aa

Merge pull request #4 from ehendrikd/serato-markers-integration-cues-…

6f8da17

…coreaudio CoreAudio MP3 offsets for Serato cues

Holzhaus moved this from In progress to In Review in 2.3 release Apr 22, 2020

Holzhaus marked this pull request as ready for review April 22, 2020 06:48

Be-ing marked this pull request as ready for review April 22, 2020 07:08

Be-ing merged commit 22900e5 into mixxxdj:master Apr 22, 2020

2.3 release automation moved this from In Review to Done Apr 22, 2020

Read Cues from Serato MP3 Tags #2526

Read Cues from Serato MP3 Tags #2526

Conversation

Holzhaus commented Feb 29, 2020

Holzhaus Mar 10, 2020

Choose a reason for hiding this comment

uklotzde commented Mar 20, 2020

uklotzde commented Mar 21, 2020

Be-ing commented Mar 23, 2020

ehendrikd commented Mar 23, 2020 • edited

uklotzde commented Mar 23, 2020

daschuer commented Mar 25, 2020

Holzhaus commented Mar 25, 2020

Be-ing commented Mar 25, 2020 • edited

Swiftb0y commented Mar 25, 2020

Holzhaus commented Mar 25, 2020

Be-ing commented Mar 25, 2020

Be-ing commented Mar 25, 2020

Be-ing commented Mar 25, 2020 • edited

Be-ing commented Mar 25, 2020 • edited

ywwg commented Mar 25, 2020

Be-ing commented Mar 25, 2020

Holzhaus commented Mar 25, 2020 • edited

Be-ing commented Mar 25, 2020

Be-ing commented Apr 17, 2020

Holzhaus commented Apr 17, 2020

uklotzde commented Apr 17, 2020

Holzhaus commented Apr 19, 2020

Be-ing commented Apr 20, 2020

ehendrikd commented Apr 20, 2020 • edited

Be-ing commented Apr 20, 2020

ehendrikd commented Apr 21, 2020

Be-ing commented Apr 21, 2020

Holzhaus commented Apr 21, 2020 • edited

Be-ing commented Apr 21, 2020

Be-ing commented Apr 22, 2020 • edited

Be-ing commented Apr 22, 2020

Be-ing commented Apr 22, 2020

uklotzde commented Apr 22, 2020

Holzhaus commented Apr 22, 2020

ehendrikd commented Mar 23, 2020 •

edited

Be-ing commented Mar 25, 2020 •

edited

Be-ing commented Mar 25, 2020 •

edited

Be-ing commented Mar 25, 2020 •

edited

Holzhaus commented Mar 25, 2020 •

edited

ehendrikd commented Apr 20, 2020 •

edited

Holzhaus commented Apr 21, 2020 •

edited

Be-ing commented Apr 22, 2020 •

edited