Add support for Traktor cues #1411

needspeed · 2017-12-12T20:03:11Z

Incomplete: No support for other cues then "AutoGrid" (type 4 => type 2) and Hotcues (type 0 => type 1) (but it should be easy to add those)
Not tested: Conversion of Traktor "START" time (in ms) to mixxx "position" (in samples).
My Traktor library is generated from a Rekordbox library and those two position formats don't match well.
Thus I could not really test if original Traktor cue point times match (there might be an offset of 1152 samples).

daschuer · 2017-12-12T21:12:12Z

Thank you very much for this PR.

How are the Traktor cue points map to Mixxx cue points?
What happens if the Cue points are adjusted in Mixxx or/and Traktor? Who wins?

daschuer · 2017-12-12T21:06:30Z

src/library/traktor/traktorfeature.cpp

+                QXmlStreamAttributes attr = xml.attributes ();
+                int hotcue = attr.value("HOTCUE").toString().toInt();
+                //int position = int(attr.value("START").toString().toFloat()/1000.0 * float(samplerate) * 2.0);
+                int position = int(attr.value("START").toString().toDouble()/1000.0 * 48000.0 * 2.0);


We need here to use the tack sample rate.

We are moving to a floating point transport engine. This means Mixxx can handle double precision cue points soon, and the code here should use double to not loose info.

The Track sample rate is not available at this point.
It is assumed to be 48kHz at first, but the moment the sample rate is available (on track import [traktorfeature.cpp:59 and 79]), the position is changed to the proper value.

Did I misunderstand the second part of the comment?
Because this code uses double already.

I mean: int position -> double position
but maybe I have missed something,

Oh ok.
Is the position not denoting the sample?
What exactly would a double value mean in this case?

Ah ... posMultiplier. Can't we just remove the 48000, in all places and user positionMs until it is converted into Mixxx stereo samples playposition?

Yeah it's probably a prettier solution not to do it the current way.
My main motivation was to keep the traktor_cue table in the same scheme as the cue table.
If changing the position in the traktor_cue table to double, we could insert the raw value of Traktor and later convert it.

What do you mean with "and user positionMs until it is converted into Mixxx stereo samples playposition"?

Exactly what you wrote before. The fancy stereo sample position is called "playposition" in the Mixxx code base. The traktor position needs an other name,for example positionMs.

What exactly would a double value mean in this case?

If you play a track at 50 % speed for instance, you are able to set a cue point between two samples.

I changed the way the traktor cues are saved.
Now I keep the native format for the database.
The conversion is done later and in a single position.
This makes the position conversion more easy and future proof.
(Now you only have to change a single line if the playposition is changed to double)

daschuer · 2017-12-12T21:09:39Z

res/schema.xml

+      See library/traktor/traktorfeature.cpp
+    </description>
+    <sql>
+      CREATE TABLE IF NOT EXISTS traktor_cues (


Please add a comment to the code, how the table will be used.

I am descriptive in the file now.
Don't know if this is enough.

needspeed · 2017-12-12T21:15:59Z

The transport from Traktor cue to mixxx cue is done the moment the track is imported to the library.
So changing cues in Traktor for already imported tracks in mixxx bears no effects.

daschuer · 2017-12-12T21:32:39Z

The transport from Traktor cue to mixxx cue is done the moment the track is imported to the library.
So changing cues in Traktor for already imported tracks in mixxx bears no effects.

That makes sense, because once the user is on Mixxx he will never go back :-P
Yes, the updating issue can be done separate if there is a demand.
Is there anything we can do to enable a future update solution?

daschuer · 2017-12-12T21:33:46Z

src/library/traktor/traktorfeature.cpp

+                QXmlStreamAttributes attr = xml.attributes ();
+                int hotcue = attr.value("HOTCUE").toString().toInt();
+                //int position = int(attr.value("START").toString().toFloat()/1000.0 * float(samplerate) * 2.0);
+                int position = int(attr.value("START").toString().toDouble()/1000.0 * 48000.0 * 2.0);


Is START a floating point in Traktor? Please add a comment to the code.

Yes it is in the unit of milliseconds.

daschuer · 2017-12-12T21:37:03Z

src/library/traktor/traktorfeature.cpp

+                QString label = attr.value("NAME").toString();
+                switch (type) { //TODO
+                    case 0:
+                        type = 1;


named constants would be nice here.

needspeed · 2017-12-12T21:40:03Z

src/library/traktor/traktorfeature.cpp

+    bool track_already_in_library = m_pTrackCollection->getTrackDAO().trackExistsInDatabase(location);
+    TrackPointer pTrack = BaseExternalTrackModel::getTrack(index);
+
+    if (pTrack && !track_already_in_library) {


This is the check that has to be removed (besides other minor changes) to be able to import changed Traktor cues.

daschuer · 2017-12-12T21:41:32Z

In order to merge this once it is ready, we need your permission.
Please sign: https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ

By the way: next time you should create a feature branch in your local repo before starting a new feature.
Here are some additional hints:
https://www.mixxx.org/wiki/doku.php/bugfix_workflow

needspeed · 2017-12-12T21:45:19Z

Signed it.
Sorry about the local branch, missed the note in that article.

daschuer · 2017-12-12T21:49:41Z

Nevermind, it it might be an issue on your side though.

Be-ing · 2017-12-13T02:04:03Z

Thanks for working on this!

daschuer · 2017-12-13T07:10:32Z

@uklotzde: I am unsure if it will be better to have the Traktor Cue table temporary. On one hand the track table is persistent, on the other hand this are temporare data.

Moved the cue code to a seperate file.

Removed unecessary call to cueDAO

Be-ing · 2018-04-18T05:56:53Z

What is the status of this PR? It would be great to have this feature in Mixxx 2.2.

radusuciu · 2018-10-24T19:17:12Z

This would be cool!

pestrela · 2019-01-24T15:18:30Z

Hi,
I've now took the time to document all the options I'm aware to do this conversion, including the "cues shifted in time" issue
https://mixxx.org/wiki/doku.php/traktor_cue_storage_format

massive investigation ongoing on:
digital-dj-tools/dj-data-converter#3

SolarLiner · 2019-04-26T12:33:54Z

Owning Mixed in Key I wanted to use it to redo my library of tracks, and import the exported Traktor collection into Mixxx. I will try this PR and see how well it works.

It looks like everything is in order for merging, except maybe some conflicts from the state of fork repo... What's the status on this PR?

pestrela · 2019-05-04T17:51:14Z

Regarding the traktor time shifts, since January we have made significant efforts to understand this issue in detail.

The findings are very clear: unless mixxx uses exactly the same mp3 decoder as Traktor, this problem will appear over and over. Traktor uses FHG decoder, Rekordbox uses mpg123 decoder.

Please see results and summary in a project about collection conversion from TK to RB:

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

we first found weak correlations to mp3 encoders

we then found strong correlations with the decoders that Traktor and Rekordbox uses

we now found very strong correlations to the Xing and LAME extra headers being interpreted as bogus audio

SolarLiner · 2019-05-04T23:24:29Z

While the decoders are different and differ in the timestamping, the result are shifted timestamps by the same amount, every time. Since we're talking about converting from Traktor to Mixxx, it should simply be a matter of figuring out the delta and correcting when reading (or importing) Traktor cues.

Anyway, I would have loved to test this PR, but I can't get a local build going on my machine and the CI builds are too old and have been removed. Is there a way to trigger a build?

pestrela · 2019-05-05T10:43:40Z

problem definition

the result are shifted timestamps by the same amount, every time.
it should simply be a matter of figuring out the delta and correcting when reading (or importing) Traktor cues.

The value of the delta is very clear: this is a 26ms frame
https://www.google.com/search?q=26ms+mp3+frame

The problem is that no two mp3 are the same; they have a variety of control headers in the data stream that the decoders may or may not understand.
If they don't understand these headers they treat them as a (bogus) audio frame, producing a 26ms silence

old files correction

After months of of investigation we have produced the following correction table for old files.

Case	Signature	TK->RB correction
case A	no extra headers	0 ms
case B	Only Xing headers	26 ms
case C	Xing+Lame, CRC fail	26 ms
case D	Xing+Lame, CRC ok	0 ms

case E	null header + case A	0 ms
case F	null header + any header	26 ms

2019 files correction

Update: We now found that using the latest 2019 FFMPEG produces "case D" files that are false positives.
This means that old case D files were fine, but newer ones are no longer fine. More analysis is required!

pestrela · 2019-05-31T17:02:23Z

I've now made a "give me the numbers" summary of our TK->RB investigation. Please find it in this exact comment
Hopefully it explains clearly what we investigated over several months.

Be-ing · 2019-09-01T01:31:11Z

I just took a brief look at the code and I am doubtful about the approach this is using. Why should Mixxx store Traktors' cues in their own table? Why don't we import them into Mixxx's cue table?

Be-ing · 2019-09-01T02:53:34Z

I see that the other external library integrations take this approach too, so this is a bigger issue. I have started a discussion about this on Zulip.

github-actions · 2020-10-20T00:22:20Z

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

Holzhaus · 2021-05-01T14:20:42Z

is this only one way - in that it can only read usb sticks prepared by rekordbox? or would it be possible to export a mixxx library to a usb that could be read by rekordbox compatible CDJs and related devices?

This is unrelated to this branch, please use the forum instead.

adrienneteicher · 2021-05-01T15:36:36Z

is this only one way - in that it can only read usb sticks prepared by rekordbox? or would it be possible to export a mixxx library to a usb that could be read by rekordbox compatible CDJs and related devices?

This is unrelated to this branch, please use the forum instead.

yes sorry! i had my wires crossed.

needspeed added 2 commits December 12, 2017 20:40

Added Traktor Cue Support

2ec07cc

Merge branch 'master' of https://github.com/needspeed/mixxx

c604809

daschuer reviewed Dec 12, 2017

View reviewed changes

needspeed commented Dec 12, 2017

View reviewed changes

Added (incomplete) TraktorCueType enum

6f22022

needspeed added 2 commits December 14, 2017 00:29

Now Traktor cues are saved in their native format into traktor_cues.

8f03e8f

Moved the cue code to a seperate file.

Added supported for all Traktor cue types

f55b111

Removed unecessary call to cueDAO

Be-ing added this to the 2.2.0 milestone Dec 27, 2017

Be-ing added the library label Dec 27, 2017

Be-ing modified the milestones: 2.2.0, stalled Jun 23, 2018

pestrela mentioned this pull request May 5, 2019

MP3 cues shifted in time going from Traktor to Rekordbox (AKA 26ms problem) digital-dj-tools/dj-data-converter#3

Closed

ehendrikd mentioned this pull request May 24, 2019

Rekordbox removable device library feature #2119

Merged

uklotzde mentioned this pull request May 26, 2019

Universal SoundSource for FFmpeg 4.x #1356

Merged

4 tasks

github-actions bot added the stale Stale issues that haven't been updated for a long time. label Oct 20, 2020

Be-ing changed the base branch from master to main October 23, 2020 23:52

Be-ing mentioned this pull request Jan 23, 2021

1908475 fix detect latest traktor library #3588

Merged

ronso0 marked this pull request as draft February 14, 2021 21:58

Add support for Traktor cues #1411

Are you sure you want to change the base?

Add support for Traktor cues #1411

Conversation

needspeed commented Dec 12, 2017

daschuer commented Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

needspeed commented Dec 12, 2017

daschuer commented Dec 12, 2017

daschuer Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Dec 12, 2017

needspeed commented Dec 12, 2017

daschuer commented Dec 12, 2017

Be-ing commented Dec 13, 2017

daschuer commented Dec 13, 2017

Be-ing commented Apr 18, 2018 • edited Loading

radusuciu commented Oct 24, 2018

pestrela commented Jan 24, 2019 • edited Loading

SolarLiner commented Apr 26, 2019 • edited Loading

pestrela commented May 4, 2019 • edited Loading

SolarLiner commented May 4, 2019

pestrela commented May 5, 2019 • edited Loading

problem definition

old files correction

2019 files correction

pestrela commented May 31, 2019

Be-ing commented Sep 1, 2019

Be-ing commented Sep 1, 2019

github-actions bot commented Oct 20, 2020

Holzhaus commented May 1, 2021

adrienneteicher commented May 1, 2021

daschuer commented Dec 12, 2017 •

edited

Loading

daschuer Dec 12, 2017 •

edited

Loading

Be-ing commented Apr 18, 2018 •

edited

Loading

pestrela commented Jan 24, 2019 •

edited

Loading

SolarLiner commented Apr 26, 2019 •

edited

Loading

pestrela commented May 4, 2019 •

edited

Loading

pestrela commented May 5, 2019 •

edited

Loading