Merge lp://qastaging/~mixxxdevelopers/mixxx/library_features into lp://qastaging/~mixxxdevelopers/mixxx/trunk
Proposed by
RJ Skerry-Ryan
| Status: | Needs review | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Proposed branch: | lp://qastaging/~mixxxdevelopers/mixxx/library_features | ||||||||||||||||
| Merge into: | lp://qastaging/~mixxxdevelopers/mixxx/trunk | ||||||||||||||||
| Diff against target: |
5250 lines (+1538/-949) 77 files modified
mixxx/build/depends.py (+1/-0) mixxx/res/schema.xml (+14/-0) mixxx/src/dlgautodj.cpp (+5/-2) mixxx/src/dlgautodj.h (+1/-0) mixxx/src/dlgpreferences.cpp (+5/-0) mixxx/src/dlgpreferences.h (+2/-0) mixxx/src/dlgprefplaylist.cpp (+87/-9) mixxx/src/dlgprefplaylist.h (+9/-0) mixxx/src/dlgprefplaylistdlg.ui (+40/-26) mixxx/src/dlgprefrecord.cpp (+1/-4) mixxx/src/library/autodjfeature.cpp (+2/-0) mixxx/src/library/autodjfeature.h (+3/-0) mixxx/src/library/baseexternalplaylistmodel.cpp (+3/-3) mixxx/src/library/baseexternalplaylistmodel.h (+3/-1) mixxx/src/library/baseexternaltrackmodel.cpp (+3/-3) mixxx/src/library/baseexternaltrackmodel.h (+3/-1) mixxx/src/library/baseplaylistfeature.cpp (+5/-4) mixxx/src/library/baseplaylistfeature.h (+0/-1) mixxx/src/library/basesqltablemodel.cpp (+79/-9) mixxx/src/library/basesqltablemodel.h (+60/-48) mixxx/src/library/basetrackcache.cpp (+2/-0) mixxx/src/library/browse/browsefeature.cpp (+23/-2) mixxx/src/library/browse/browsefeature.h (+3/-0) mixxx/src/library/browse/browsetablemodel.cpp (+2/-1) mixxx/src/library/browse/browsetablemodel.h (+1/-1) mixxx/src/library/browse/browsethread.h (+7/-9) mixxx/src/library/browse/foldertreemodel.cpp (+1/-0) mixxx/src/library/cratefeature.cpp (+10/-7) mixxx/src/library/cratefeature.h (+3/-0) mixxx/src/library/cratetablemodel.cpp (+46/-62) mixxx/src/library/cratetablemodel.h (+7/-20) mixxx/src/library/dao/directorydao.cpp (+143/-0) mixxx/src/library/dao/directorydao.h (+31/-0) mixxx/src/library/dao/trackdao.cpp (+307/-76) mixxx/src/library/dao/trackdao.h (+17/-7) mixxx/src/library/hiddentablemodel.cpp (+42/-50) mixxx/src/library/hiddentablemodel.h (+2/-15) mixxx/src/library/itunes/itunesfeature.cpp (+24/-22) mixxx/src/library/itunes/itunesfeature.h (+5/-1) mixxx/src/library/library.cpp (+79/-19) mixxx/src/library/library.h (+16/-1) mixxx/src/library/libraryfeature.h (+4/-3) mixxx/src/library/libraryscanner.cpp (+40/-26) mixxx/src/library/libraryscanner.h (+6/-3) mixxx/src/library/librarytablemodel.cpp (+44/-66) mixxx/src/library/librarytablemodel.h (+2/-21) mixxx/src/library/missingtablemodel.cpp (+0/-125) mixxx/src/library/missingtablemodel.h (+0/-41) mixxx/src/library/mixxxlibraryfeature.cpp (+4/-4) mixxx/src/library/mixxxlibraryfeature.h (+5/-1) mixxx/src/library/playlistfeature.cpp (+6/-5) mixxx/src/library/playlistfeature.h (+5/-1) mixxx/src/library/playlisttablemodel.cpp (+56/-96) mixxx/src/library/playlisttablemodel.h (+14/-30) mixxx/src/library/preparelibrarytablemodel.cpp (+2/-13) mixxx/src/library/preparelibrarytablemodel.h (+2/-7) mixxx/src/library/promotracksfeature.cpp (+1/-1) mixxx/src/library/proxytrackmodel.cpp (+16/-11) mixxx/src/library/proxytrackmodel.h (+3/-4) mixxx/src/library/rhythmbox/rhythmboxfeature.cpp (+6/-4) mixxx/src/library/rhythmbox/rhythmboxfeature.h (+4/-1) mixxx/src/library/setlogfeature.cpp (+6/-5) mixxx/src/library/stardelegate.cpp (+11/-3) mixxx/src/library/stardelegate.h (+2/-0) mixxx/src/library/trackcollection.cpp (+28/-14) mixxx/src/library/trackcollection.h (+12/-1) mixxx/src/library/trackmodel.h (+18/-12) mixxx/src/library/traktor/traktorfeature.cpp (+14/-10) mixxx/src/library/traktor/traktorfeature.h (+8/-3) mixxx/src/library/treeitem.cpp (+6/-0) mixxx/src/mixxx.cpp (+56/-27) mixxx/src/mixxx.h (+2/-1) mixxx/src/playermanager.cpp (+8/-0) mixxx/src/soundsourceproxy.h (+1/-1) mixxx/src/waveform/waveform.h (+1/-1) mixxx/src/widget/wtracktableview.cpp (+44/-4) mixxx/src/widget/wtracktableview.h (+4/-0) |
||||||||||||||||
| To merge this branch: | bzr merge lp://qastaging/~mixxxdevelopers/mixxx/library_features | ||||||||||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Mixxx Development Team | Pending | ||
|
Review via email:
|
|||
Description of the change
Getting the ball rolling on the code review for Max's library_features work.
To post a comment you must log in.
Revision history for this message
| RJ Skerry-Ryan (rryan) wrote | # |
Download full text (3.2 KiB)
Unmerged revisions
- 3288. By Max Linke <email address hidden>
-
merge with trunk
- 3287. By Max Linke <email address hidden>
-
merge with trunk
- 3286. By Max Linke <email address hidden>
-
merge with trunk
- 3285. By Max Linke <email address hidden>
-
merge with trunk
- 3284. By Max Linke <email address hidden>
-
removed fringerprinting feature that is moved into chromaprint branch
- 3283. By Max Linke <email address hidden>
-
next try 2
- 3282. By Max Linke <kain88@640X4.kel.wh.lokal>
-
next try
- 3281. By Max Linke <kain88@640X4.kel.wh.lokal>
-
does launchpad use my email now?
- 3280. By Max Linke <kain88@640X4.kel.wh.lokal>
-
merge with trunk, library scanner does not add songs
- 3279. By RJ Skerry-Ryan
-
Link fftw3 on Windows only.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Hey Max -- I know you aren't ready to merge this yet but I gave it a quick skim while getting chromaprint on the build server and thought I'd shoot you some comments:
* DlgPrefPlaylist
- SQL queries should not be done here -- need to be abstracted away by a DAO.
- Shouldn't rely on the default DB connection existing -- TrackCollection needs to get passed into DlgPreferences. (We could change the creation order in the future and this code would break -- better to have it enforced by code)
* BaseSqlTableMod
- It would be cool if you could handle the use case where you select a whole album and hit relocate. Either maybe auto-magically detecting that the rest of the selected missing files are in the same directory as the first missing one relocated or maybe by letting the user pick a directory if it is clear that all the selected files were in the same directory originally. This would be a handy speedup.
* DirectoryDAO:
* DirectoryDAO:
* DirectoryDAO:
* DirectoryDAO:
* TrackDAO:
* TrackDAO -- some of your queries are using strings that need escaping .. you can only use string construction of the query for when you know the values are safe (i.e. a list of numeric IDs that you join together with commas.) or when you are using table names, column names, and other constants. Strings coming from user input (names, directories, paths, etc.) all need escaping.
* I'm scared of adding delete-
And now, about checksumming:
Checksumming every file when we add it to the library has to come with a huge performance hit to the library scanner. Have you noticed this? Albert and I considered checksums when we first wrote the SQLite library but didn't go with it because of this issue. I thought it would be a good compromise to do the checksums in the analyser queue (because it would be much less noticed there) and then only use the checksum for re-unification if it is present.
And now some comments about the checksum implementation itself:
* We should store version information about the checksum with it in a checksum_version column or with a unique string prefix to put on the front (I prefer a separate column since separators never end up being as unique as you thought they were).
* RE: md5 -- we probably don't need a c...