Enable setting hotcue color via controlobject #2016
Conversation
…fixed codefactor issues
This reverts commit 3f986a7.
|
What about controller mappings? Shall we adapt the mappings of controllers with rgb pads to reflect the cue colors before releasing 2.3? |
|
Shall I rebase this on 2.3 branch and issue a PR to it? |
Is @dszakallas still willing to work on his launchpad mapping? |
|
I'm not fully grasping this feature. Is this about assigning different colors to different hot cue markers? |
|
Yes exactly. It has been standard in most other DJing Applications and it is IMO essential for most people. There is a reason why almost every modern Controller has RGB Pads (including the latest launchpad generation ;) ) |
This would be a only a nice to have. I hope this will attract attention at the original mapper, and maybe they are willing to do the work. |
We have no 2.3 branch yet. So it is just fine. |
|
This definitely sounds like a cool feature. I'm happy to add it to the my Launchpad mapping. |
daschuer
changed the title
|
LGTM |
|
I'm really happy that we finally finished this PR. Especially, @ferranpujolcamins, thank you for continuing what I've started. Now we just have to think about how to make this usable by exposing it to the Skins. |
ferranpujolcamins
deleted the
Enable-Setting-hotcue-color-via-controlobject
branch
|
I am getting crashes in mixxx and a git bisect identifies commit 29df355 as the culprit. This happens when I just load any track into deck 1. backtrace: #2 0x00005555557c686f in Track::getCuePoints() const (this=0x0) at src/track/track.cpp:799 getCuePoints is called on a nullptr m_pCurrentTrack |
|
I can not reproduce that. I checked out the mentioned commit directly and built it with no extra flags. I can load any track into any Deck and Mixxx stays stable. |
|
It doesn't happen 100% of the time, and it looks like it can only crash on the first try. And I'm guessing this only happens on tracks that have hotcues set. What's going on is m_pCurrentTrack is null at construction time. When a track is loaded, some of the signals are crossed so that onMarkChanged is getting called before slotLoadingTrack is called. Therefore, once any track loads correctly, m_pCurrentTrack will never be null again so the crash is avoided. But also, I suspect onMarkChanged is always getting called for the old track, not the current one. onMarkChanged seems to be called by pmark->connectSamplePositionChanged signal. I'm not sure what event is triggering the sample position changes though. |
|
ok so this only occurs on tracks where a loop has been defined. So here are the reproduce instrux:
The trigger to the crash is basetrackplayer.cpp where it calls m_pLoopInPoint->set(loopStart); -- setting the loop start position triggers the sample position changed signal early, before the track pointer in woverview.cpp has been updated yet. It may just be enough to put a nullptr check around the access of m_pCurrentTrack in woverview.cpp, but I'm trying to figure out if there are any bad side effects to this misordering. |
|
I still can't reproduce the crash... |
|
try building from head, not from that commit. I think that commit just shows where the signal was hooked up. Also try with the latenight skin just in case that makes a difference somehow? |
|
Yes, It only happens with the LateNight skin. All of the other Skins are fine. |
|
I'll file a bug since this is tricky |
|
the video is inaccessible. (private instead of unlisted?) |
|
ah, yes, was private. Looks like you're able to repro now though |
Supersedes #1830 (thanks @Swiftb0y for your work!)
TODO: