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

Add Serato library feature #2480

Merged
merged 36 commits into from
Feb 16, 2020
Merged

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Feb 1, 2020

I had this on my disk for a while. I squashed some commits and rebased this on latest master. It works so for me, but I'm not sure that this is ready for being included a release, because I didn't test it on MacOS or Windows yet.

Note that this doesn't include support for Hotcues, which is added in PR #2323 and #2473.

Feel free to review. I bases this on the Rekordbox library feature, but I'm not sure that I did everything right.

@Holzhaus
Copy link
Member Author

Holzhaus commented Feb 1, 2020

One issue I have with this is that the Serato library features some columns that are not supported by Mixxx itself (sample rate, label, etc.). These columns miss a table column header:
2020-02-01-185322_467x195_scrot

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First set of comments

res/schema.xml Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more comments.
Please also run
git clang-format
on your PR

src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
src/library/serato/seratofeature.cpp Outdated Show resolved Hide resolved
@Be-ing
Copy link
Contributor

Be-ing commented Feb 4, 2020

Why have both this and reading Serato's file tags? Is there metadata this imports that is not in the file tags? If they are functionally equivalent, IMO supporting both would be an unnecessary maintenance burden.

@Holzhaus
Copy link
Member Author

Holzhaus commented Feb 4, 2020

Why have both this and reading Serato's file tags? Is there metadata this imports that is not in the file tags? If they are functionally equivalent, IMO supporting both would be an unnecessary maintenance burden.

No, they are not equivalent. This imports the library, i. e. The list of tracks that are shown in the track table in Serato. It also parses Crates that these tracks are stored in. What's still missing here is parsing "Smart Crates" (dynamic playlists that filter by criteria), but I don't know how to map them in Mixxx.

No hotcues/Loop/etc. are stored in the Serato DB. This can be merged indepently from the Metadata PR.

@Be-ing
Copy link
Contributor

Be-ing commented Feb 4, 2020

Thank you for clarifying. Importing playlists from Serato is useful independent of the track metadata.

@Holzhaus
Copy link
Member Author

Holzhaus commented Feb 5, 2020

One issue I have with this is that the Serato library features some columns that are not supported by Mixxx itself (sample rate, label, etc.). These columns miss a table column header:
2020-02-01-185322_467x195_scrot

If we first merge PR #2487, this won't be an issue anymore, because the sample rate column will have a title then. I removed the other two columns (label, Serato DB path) from the view in bba4cae.

@Holzhaus Holzhaus self-assigned this Feb 13, 2020
@Holzhaus
Copy link
Member Author

Anything missing or can this be merged?

@uklotzde uklotzde added this to the 2.3.0 milestone Feb 15, 2020
@daschuer
Copy link
Member

LGTM, Thank you for this nice feature.

@daschuer daschuer merged commit f7f5277 into mixxxdj:master Feb 16, 2020
@daschuer
Copy link
Member

@uklotzde ... ups, missed to wait for your LGTM. Is there anything left?

@uklotzde
Copy link
Contributor

@daschuer Perfectly ok.

@Holzhaus
Copy link
Member Author

@bengl3rt

I checked out and built this branch in the following environment:

ben@Banger-Factory mixxx % sw_vers
ProductName:	Mac OS X
ProductVersion:	10.15.4
BuildVersion:	19E287
ben@Banger-Factory mixxx % cat ~/Code/mixxxenv/2.3-j00008-c18d8fda-osx10.11-x86_64-release/env.sh 
export OSXLIB=/Users/ben/Code/mixxxenv/2.3-j00008-c18d8fda-osx10.11-x86_64-release; 
# Make sure to edit this to match what is actually present in $OSXLIB.
export QTDIR=${OSXLIB}/Qt-5.14.2; 
export PATH=${OSXLIB}/bin:${QTDIR}/bin:$PATH; 
export CXXFLAGS="-isystem ${OSXLIB}/include"; 
export CFLAGS="-isystem ${OSXLIB}/include"
export LDFLAGS="-L${OSXLIB}/lib -F${OSXLIB}/lib -Wl,-rpath,${OSXLIB}/lib"; %                                                                                                                                     ben@Banger-Factory mixxx % clang --version
Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin19.4.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Unfortunately, tracks from the Serato database don't appear to load at all. Please see the attached screen recording. Nothing interesting prints out in the console when attempting to load them either.

Screen Recording 2020-04-12 at 1.55.30 PM.mp4.zip

Hi @bengl3rt, this is the right PR for your issue. Can you navigate to your music directory, enter the _Serato_ subdirectory and attach your database V2 file here? Also, can you give me the absolute path to the database file on your system and the absolute path of a music file that refused to load? I think the Mac code for determining file location might be broken.

@bengl3rt
Copy link

bengl3rt commented Apr 12, 2020

Sure, here are the paths:

ben@Banger-Factory _Serato_ % file database\ V2 
database V2: data
ben@Banger-Factory _Serato_ % pwd
/Users/ben/Music/_Serato_

ben@Banger-Factory mixxtests % ls
case_a__armin_Nehalennia__nothing.mp3				case_b__david_guetta_memories__xing_lame_on_second_frame.mp3	case_c__factorb_luna__crc_zero.mp3
case_b__adele_paul_damixie__xing_lavc.mp3			case_b__diogo_dialeto__xng_lavc.mp3				case_d__pobsky__crc_ok.mp3
case_b__avicci_tim_berg__info_lavc.mp3				case_b__estiva__info_lavf.mp3					wrong_classification__adam_beyer_mind__info_lavf.mp3
ben@Banger-Factory mixxtests % pwd
/Users/ben/Music/mixxtests

database V2.zip

@Holzhaus
Copy link
Member Author

Holzhaus commented Apr 12, 2020

Ok, looks like the paths are relative to the root directory (/):

[...]
otrk (Track, 649 B)
  ttyp (File Type, 6 B): 'mp3'
  pfil (File Path, 172 B): 'Users/ben/Music/mixxtests/case_b__david_guetta_memories__xing_lame_on_second_frame.mp3'
  tsng (Song Title, 112 B): 'case_b__david_guetta_memories__xing_lame_on_second_frame'
  tlen (Length, 16 B): '05:18.67'
  tsiz (Unknown, 10 B): '7.3MB'
  tbit (Bitrate, 18 B): '192.0kbps'
[...]

What happens if you put a file on a USB drive and load it into serato from there? It should create a Serato DB on the USB drive. Can you attach that database file (and the absolute path to the music file on the USB drive) as well?

@bengl3rt
Copy link

Sure, I loaded the following file into Serato using drag and drop:

/Volumes/PILZ3/LDS218/06.DJ_TOOLS_(NEW_EDITS)LDS218/2_chainz_x_tropkillaz-birthday_song(dibs_&_mgm_edit).mp3

Here's the Serato dir it created at the root of the volume:

Serato.zip

@Holzhaus
Copy link
Member Author

Does loading from a USB stick work in Mixxx via the Serato library item?

@bengl3rt
Copy link

It sure does!

@Holzhaus
Copy link
Member Author

In any case, the information you gave should suffice to fix the path issue on OSX. Thank you very much. I'll try to get it working in the next few days.

@Holzhaus
Copy link
Member Author

In any case, the information you gave should suffice to fix the path issue on OSX. Thank you very much. I'll try to get it working in the next few days.

Fixing it was actually way easier than I anticipated.

@bengl3rt Please check out #2659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants