Update to libdjinterop 0.19 #11720
Update to libdjinterop 0.19 #11720
Conversation
mr-smidge
force-pushed
the
enh/djinterop-0.19
branch
from
7ccbcb6 to
8be5301
Compare
|
The |
|
@toszlanyi @whanake-music Could you please check that this PR works with your EngineOS devices. |
| @@ -18,7 +18,7 @@ | |||
| #include "library/trackset/crate/cratestorage.h" | |||
| #include "moc_dlglibraryexport.cpp" | |||
|
|
|||
| namespace el = djinterop::enginelibrary; | |||
| namespace el = djinterop::engine; | |||
There was a problem hiding this comment.
el isn't self explaining anymore, if you remove the word library
There was a problem hiding this comment.
Good spot - renamed to e.
I successfully built the PR this morning and gonna check exporting and playing for the Twitch stream tonight. Report back tomorrow. Cheers! |
All went smooth on my SC6000M, except one track (out of ca. 150) that wouldn't show the overview waveform on the player. It's available in Mixxx though. I gonna do more testing and discuss in zulip chat for more details. All in all a nice experience for my 2h set yesterday. Cheers |
|
@toszlanyi Is the waveform issue new, or did it occur already with djinterop 0.16.1 which is currently included in Mixxx? |
I did test a couple of scenarios and unfortunately need to admit it is a new issue. I gonna give a full report on zulip chat |
| @@ -233,7 +233,7 @@ void DlgLibraryExport::checkExistingDatabase() { | |||
| m_pVersionCombo->clear(); | |||
| m_pVersionCombo->setEnabled(true); | |||
| for (int versionIndex = 0; | |||
| versionIndex < (int)e::all_versions.size(); | |||
| versionIndex < static_cast<int>(e::all_versions.size()); | |||
There was a problem hiding this comment.
Why not use std::size_t in the first place and remove the nerrowing cast here?
There was a problem hiding this comment.
because we add -1 further down... Though the solution would be to just start at 1 in this loop instead of 0...
https://github.com/mixxxdj/mixxx/pull/11720/files#diff-d9d73ff88def8fe08f4f69d92acf7c11db20ab58ba2712c92fb0addf6036932bR258-R259
|
@mr-smidge We have merge conflicts in CMakeLists.txt now - due to merge of #11729 into 2.4. Please resolve the conflicts. |
mr-smidge
force-pushed
the
enh/djinterop-0.19
branch
from
2901aca to
5183ecf
Compare
@JoergAtGithub Done! |
|
@mr-smidge Should we merge this PR as it is, to have a bigger testing group - or do you plan fixes for the missing waveform issue reported above first? |
Realistically, at this stage I don't know what form the waveform fix will take - my debugging will essentially be to change things methodically until I find the thing that makes it work! It seems from testing that the waveform issue only affects a minority of tracks at present, and there is still a workaround: to export as Engine version 1.6.1 (which does not display the bug) and let the Denon players upgrade the database to 3.x. I would be comfortable merging this in its current state, but I appreciate the decision is not up to me 😄. |
|
Ok, than let's merge it! Thank you for this big improvement, we know it was a huge amount of work! |
This PR updates Mixxx to use version 0.19 of libdjinterop, which supports Denon firmware up to version 3.0.0.
There are no functional changes as part of this PR - the changes are limited to those necessary to adhere to the evolving public API of libdjinterop whilst it is in its early 0.x state.