Merge lp:~mixxxdevelopers/mixxx/features_softtakeover into lp:~mixxxdevelopers/mixxx/trunk
- features_softtakeover
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merge reported by: | Sean M. Pappalardo | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~mixxxdevelopers/mixxx/features_softtakeover | ||||
Merge into: | lp:~mixxxdevelopers/mixxx/trunk | ||||
Diff against target: |
476 lines (+227/-15) (has conflicts) 11 files modified
mixxx/build/depends.py (+2/-0) mixxx/src/midi/mididevice.cpp (+19/-9) mixxx/src/midi/mididevice.h (+2/-0) mixxx/src/midi/midimapping.cpp (+4/-0) mixxx/src/midi/midioptiondelegate.cpp (+10/-3) mixxx/src/midi/midiscriptengine.cpp (+13/-1) mixxx/src/midi/midiscriptengine.h (+4/-0) mixxx/src/mixxxcontrol.cpp (+6/-2) mixxx/src/mixxxcontrol.h (+2/-0) mixxx/src/softtakeover.cpp (+115/-0) mixxx/src/softtakeover.h (+50/-0) Text conflict in mixxx/src/midi/midimapping.cpp |
||||
To merge this branch: | bzr merge lp:~mixxxdevelopers/mixxx/features_softtakeover | ||||
Related bugs: |
|
||||
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
RJ Skerry-Ryan | Approve | ||
Sean M. Pappalardo | Abstain | ||
Albert Santoni | Needs Fixing | ||
Review via email: mp+41027@code.launchpad.net |
Commit message
Description of the change
This branch adds soft-takeover capabilities for MIDI controllers, both for simple XML mappings and MIDI scripting.
I'm requesting that this be included in time for Mixxx v1.9.0.
Note that the main goal was to add this for MIDI scripting. I did it for the XML mappings as well for completeness, but that part isn't totally useful until Mixxx can accept multiple <option>s for simple XML mappings. Adding that capability is beyond the scope of the feature itself since everything is hard-coded for a single <option>, and it would require modification of GUI classes as well, something I don't feel qualified to do, especially not with the hope of getting this merged in time for 1.9.0.
Albert Santoni (gamegod) wrote : | # |
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
Dude, you didn't need to quote the whole diff. :P
> 1) How is this useful in its current incarnation if you can't have
> multiple <options> blocks?
The XML soft-takeover option is independent of the MIDI scripting soft-takeover, so it's 100% useable from script now. And any XML controls that currently use the <normal> option can be used with <soft-takeover> right now.
> 2) Why does this have to go in 1.9? None of the mappings are using it. ??
I guess the urgency was from the discussion of ReplayGain auto-adjusting things (which Vittorio might add as an option anyway since I still want it.) But I'm going to update the SCS.1m mapping to use soft-takeover, and the EKS Otus mappings will use it.
> 3) What are the SCS.3d changes you made?
Sorry, that's just whitespace, tabs-to-spaces. (Soft-takeover would be senseless on most touch-only devices.)
This branch causes the following behavior:
If I turn a knob to one level (say 75%) and then whip it back (maybe to 25%) very fast, the knob stays at 75%. The effect is much more pronounced when I start at either 0% or 100% and whip it up or down. The problem is especially bad with knob controls, but I suppose that it's probably possible to have the same behavior occur for sliders if you move them fast enough.
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
> If I turn a knob to one level (say 75%) and then whip it back (maybe to 25%)
> very fast, the knob stays at 75%. The effect is much more pronounced when I
> start at either 0% or 100% and whip it up or down. The problem is especially
> bad with knob controls, but I suppose that it's probably possible to have the
> same behavior occur for sliders if you move them fast enough.
What's the refresh rate of your controller? That is, if you run Mixxx with --midiDebug and do the whipping, how great is the difference in successive values?
Is the control in question mapped via script? Which ControlObject is it working with that is exhibiting the problem?
(Of note, moving controls very quickly (with a slow controller refresh rate) is indistinguishable from the exact problem soft-takeover is trying to avoid. If you're The Flash, chances are you don't want soft-takeover.)
Albert Santoni (gamegod) wrote : | # |
On Thu, Nov 18, 2010 at 12:57 AM, Sean M. Pappalardo
<email address hidden> wrote:
>
> Dude, you didn't need to quote the whole diff. :P
>
>> 1) How is this useful in its current incarnation if you can't have
>> multiple <options> blocks?
>
> The XML soft-takeover option is independent of the MIDI scripting soft-takeover, so it's 100% useable from script now. And any XML controls that currently use the <normal> option can be used with <soft-takeover> right now.
Then why is the XML soft-takeover code in this branch?
>
>> 2) Why does this have to go in 1.9? None of the mappings are using it. ??
>
> I guess the urgency was from the discussion of ReplayGain auto-adjusting things (which Vittorio might add as an option anyway since I still want it.) But I'm going to update the SCS.1m mapping to use soft-takeover, and the EKS Otus mappings will use it.
>
I hope you're not planning to do those for 1.9...
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
> Then why is the XML soft-takeover code in this branch?
The branch is called softTakeover. I added the XML and MIDI scripting implementations to this branch. If you're uneasy about using it before we have multiple <options> support, just don't tell anyone about the XML option. The version in MIDI script is fine to use now.
I was planning to update those mappings for a 1.9.x release, yes. It doesn't have to be 1.9.0 though.
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
If you don't feel comfortable merging this without the multiple <options> support, please tell me what I'd need to change and where in the GUI classes to support it. (I know what I need to do on the XML side.)
Revision 2559 improves responsiveness with controls mapped in scripts by an enormous amount. I really have to whip the control very hard to cause the behavior again. I think it's very suitable for use now.
Albert Santoni (gamegod) wrote : | # |
Hey Sean,
More comments:
- Can you save the value of QDateTime:
- is it possible to roll the actual soft takeover logic out into a function? Just get it out of setValue() because it makes that function rather hard to read.
- I still don't know what we should do with the MIDI XML part of this. It's not bad, besides the duplication of the soft takeover logic, but I don't think there's a use case for it yet unless we have multiple XML <option> blocks.
- The swath of QMaps in MidiScriptEngine suggests they should maybe be rolled into a class at some point. (I don't expect you to fix this now, just giving you a heads up.)
Thanks,
Albert
Albert Santoni (gamegod) wrote : | # |
Re: your comments on IRC:
[15:16] <Pegasus_RPG_> I'm just going to factor out all the soft-takeover stuff into a class
[15:16] <Pegasus_RPG_> then instantiate that in the MSE
[15:16] <Pegasus_RPG_> and optionally the MIDI device
Yeah, sounds good. :)
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
Ok, all factored out. Applicable code in MidiDevice and the MSE is very clean now.
- Now storing the value of QDateTime:
- For the XML part, go ahead and include it since it works fine and will be useful for controls that otherwise have a <normal/> option tag. (For the future, there is a blueprint for extending the system to accept multiple <option/>s. That limitation is a general one; not a fault or result of soft-takeover specifically, so it shouldn't have any bearing on this merge since this is still useful without it. (It would just be more useful with it.))
RJ Skerry-Ryan (rryan) wrote : | # |
Hey Sean, looking good. Here are my questions/comments:
* In SoftTakeover:
- The string hack isn't pretty but it's fine. QElapsedTimer is also a good solution to what you're doing here because you don't actually care about the time itself, just the delta time. Unfortunately it's only part of 4.7 onwards as well. QElapsedTimer isn't affected by e.g. the user changing the system time while Mixxx is running (QTime::elapsed is also affected by that problem).
* In SoftTakeover:
- Where did the '3' values in threshold calculations come from?
- maxValue, minValue contain invalid data for scaleFactor calculation if cpo is NULL
- scaleFactor calculation is a little strange. Why is it fabs(maxvalue) - fabs(minvalue)? Seems like maxValue - minValue is probably what you want instead?
- Also, a little odd that 128 is factored in here. As I understand it that's mostly only used for MIDI XML mappings. Should MIDI scripts that deal with controls directly also have the 128 factor?
- Check for NULL value of 'temp' (otherwise a MIDI script or mapping can segfault Mixxx!)
- How was 50 determined as the right time delay for ignoring? Please pull it out of the expression and make it a const variable set to 50 w/ a comment about it. Does it work with all latencies?
* In MixxxControl
- the # of MIDI_OPT_
Thanks!
RJ
- 2563. By Sean M. Pappalardo
-
- Prevented calling ComputeValue on Soft-Takeover MIDI option (It's processed differently)
- Checked for NULL COs and set sensible defaults if they do happen to be NULL
- Fixed logic error in scale factor
- Moved subsequent-value-override- time into a const class variable
Sean M. Pappalardo (pegasus-renegadetech) wrote : | # |
> * In SoftTakeover:
> - QElapsedTimer is also a good solution to what you're doing here because you don't actually
> care about the time itself, just the delta time.
True, but then I'd need to create and store pointers to QElapsedTimer objects for each MixxxControl that had soft-takeover enabled instead of just uints as currently. And run-time dynamic memory allocation is a no-no in a RT app, since enable/disable can happen at any time. (Granted QHash also dynamically allocates memory, but not anywhere near as much as another whole object.)
> * In SoftTakeover:
> - Where did the '3' values in threshold calculations come from?
Experimentation. 3/128 units away from the current is enough to catch fast non-sequential moves but not cause an audially noticeable jump.
> - maxValue, minValue contain invalid data for scaleFactor calculation if cpo
> is NULL
I know, but there are no sensible defaults since CO ranges can vary widely. Just hurry up with Control 2.0. ;) For now, I've set max to something really high and min to 0 so that the if(diff>threshold) always fails, effectively disabling soft-takeover for that pass.
> - scaleFactor calculation is a little strange. Why is it fabs(maxvalue) -
> fabs(minvalue)? Seems like maxValue - minValue is probably what you want
> instead?
Hmm, you're right. Logic error I guess. Fixed.
> - Also, a little odd that 128 is factored in here. As I understand it that's
> mostly only used for MIDI XML mappings. Should MIDI scripts that deal with
> controls directly also have the 128 factor?
No, no. I just did that for consistency. See above. We could just as easily do threshold=
> - Check for NULL value of 'temp' (otherwise a MIDI script or mapping can
> segfault Mixxx!)
Done.
> - How was 50 determined as the right time delay for ignoring?
Experimentally. Unless there exists a controller with a really slow control sampling rate, it's just enough to catch wide swings in a short amount of time without defeating the purpose of soft-takeover. Tested and approved by "fast fingers" Joe (JJWC.) :)
> Please pull it
> out of the expression and make it a const variable set to 50 w/ a comment
> about it. Does it work with all latencies?
Done. I would expect that it works with all latencies since MIDI data input speed is not tied to latency. It tests good for me at 1ms and 42ms anyway.
> * In MixxxControl
> - the # of MIDI_OPT_
> SOFT_TAKEOVER 40 and 50? Could they just be 11 and 12, following the options
> coming before it?
Sure they could but I spaced them out since they're conceptually different from the others in that they don't directly modify values the same way rot64, jog, etc. do, leaving room for additional direct-modify options.
Sean M. Pappalardo (pegasus-renegadetech) : | # |
RJ Skerry-Ryan (rryan) wrote : | # |
Looks good, thanks Sean
Minor nitpick in midiscriptengin
if(cot != NULL) {
- cot->slotSet(
+ if (!m_st.
}
You should probably fold the two if statements into one.
Also one line if statements, while they are concise are often problematic 1) because it's easy to overlook that it's even there and 2) because there are no braces to protect against someone adding another statement mistakenly. I have a slight preference to not have one-line or brace-less if's anywhere in Mixxx if possible since they can be a source of bugs.
Go ahead and merge to trunk -- we haven't been strict at all about the feature freeze this time around and this only affects controllers with it explicitly enabled so I don't see a problem.
thanks again,
rj
- 2564. By Sean M. Pappalardo
-
Condensed IF statement per RJ
Preview Diff
1 | === modified file 'mixxx/build/depends.py' |
2 | --- mixxx/build/depends.py 2011-04-26 02:07:24 +0000 |
3 | +++ mixxx/build/depends.py 2011-04-28 21:01:18 +0000 |
4 | @@ -1,3 +1,4 @@ |
5 | +# -*- coding: utf-8 -*- |
6 | |
7 | import os |
8 | import util |
9 | @@ -401,6 +402,7 @@ |
10 | "midi/midioptiondelegate.cpp", |
11 | "midi/midimessage.cpp", |
12 | "midi/midiledhandler.cpp", |
13 | + "softtakeover.cpp", |
14 | |
15 | "main.cpp", |
16 | "controlgroupdelegate.cpp", |
17 | |
18 | === modified file 'mixxx/src/midi/mididevice.cpp' |
19 | --- mixxx/src/midi/mididevice.cpp 2011-03-22 16:24:04 +0000 |
20 | +++ mixxx/src/midi/mididevice.cpp 2011-04-28 21:01:18 +0000 |
21 | @@ -229,11 +229,13 @@ |
22 | //qDebug() << "MidiDevice: " << mixxxControl.getControlObjectGroup() << mixxxControl.getControlObjectValue(); |
23 | |
24 | ConfigKey configKey(mixxxControl.getControlObjectGroup(), mixxxControl.getControlObjectValue()); |
25 | + |
26 | + MidiOption currMidiOption = mixxxControl.getMidiOption(); |
27 | |
28 | #ifdef __MIDISCRIPT__ |
29 | // Custom MixxxScript (QtScript) handler |
30 | |
31 | - if (mixxxControl.getMidiOption() == MIDI_OPT_SCRIPT) { |
32 | + if (currMidiOption == MIDI_OPT_SCRIPT) { |
33 | // qDebug() << "MidiDevice: Calling script function" << configKey.item << "with" |
34 | // << (int)channel << (int)control << (int)value << (int)status; |
35 | |
36 | @@ -253,24 +255,26 @@ |
37 | |
38 | if (p) //Only pass values on to valid ControlObjects. |
39 | { |
40 | - double newValue; |
41 | - |
42 | + double currMixxxControlValue = p->GetMidiValue(); |
43 | + |
44 | + double newValue = value; |
45 | + |
46 | // compute LSB and MSB for pitch bend messages |
47 | if (status == MIDI_STATUS_PITCH_BEND) { |
48 | unsigned int ivalue; |
49 | ivalue = (value << 7) + control; |
50 | |
51 | - newValue = m_pMidiMapping->ComputeValue(mixxxControl.getMidiOption(), p->GetMidiValue(), ivalue); |
52 | + newValue = m_pMidiMapping->ComputeValue(currMidiOption, currMixxxControlValue, ivalue); |
53 | |
54 | // normalize our value to 0-127 |
55 | newValue = (newValue / 0x3FFF) * 0x7F; |
56 | - } else { |
57 | - newValue = m_pMidiMapping->ComputeValue(mixxxControl.getMidiOption(), p->GetMidiValue(), value); |
58 | + } else if (currMidiOption != MIDI_OPT_SOFT_TAKEOVER) { |
59 | + newValue = m_pMidiMapping->ComputeValue(currMidiOption, currMixxxControlValue, value); |
60 | } |
61 | |
62 | // ControlPushButton ControlObjects only accept NOTE_ON, so if the midi |
63 | // mapping is <button> we override the Midi 'status' appropriately. |
64 | - switch (mixxxControl.getMidiOption()) { |
65 | + switch (currMidiOption) { |
66 | case MIDI_OPT_BUTTON: |
67 | case MIDI_OPT_SWITCH: status = MIDI_STATUS_NOTE_ON; break; // Buttons and Switches are |
68 | // treated the same, except |
69 | @@ -278,7 +282,13 @@ |
70 | // computed differently. |
71 | default: break; |
72 | } |
73 | - |
74 | + |
75 | + // Soft-takeover is processed in addition to any other options |
76 | + if (currMidiOption == MIDI_OPT_SOFT_TAKEOVER) { |
77 | + m_st.enable(mixxxControl); // This is the only place to enable it if it isn't already. |
78 | + if (m_st.ignore(mixxxControl,newValue,true)) return; |
79 | + } |
80 | + |
81 | ControlObject::sync(); |
82 | |
83 | //Super dangerous cast here... Should be fine once MidiCategory is replaced with MidiStatusByte permanently. |
84 | @@ -294,7 +304,7 @@ |
85 | QMutexLocker locker(&m_mutex); //Lots of returns in this function. Keeps things simple. |
86 | |
87 | QString message = m_strDeviceName+": ["; |
88 | - for(int i=0; i<length; i++) { |
89 | + for(uint i=0; i<length; i++) { |
90 | message += QString("%1%2") |
91 | .arg(data[i], 2, 16, QChar('0')).toUpper() |
92 | .arg((i<(length-1))?' ':']'); |
93 | |
94 | === modified file 'mixxx/src/midi/mididevice.h' |
95 | --- mixxx/src/midi/mididevice.h 2010-11-29 21:23:11 +0000 |
96 | +++ mixxx/src/midi/mididevice.h 2011-04-28 21:01:18 +0000 |
97 | @@ -33,6 +33,7 @@ |
98 | |
99 | #include <QtCore> |
100 | #include "midimessage.h" |
101 | +#include "softtakeover.h" |
102 | |
103 | //Forward declarations |
104 | class MidiMapping; |
105 | @@ -106,6 +107,7 @@ |
106 | a race condition when a MIDI message is received and looked up in the MidiMapping while |
107 | the MidiMapping is being modified (and is already locked). */ |
108 | bool m_bReceiveInhibit; |
109 | + SoftTakeover m_st; |
110 | }; |
111 | |
112 | #endif |
113 | |
114 | === modified file 'mixxx/src/midi/midimapping.cpp' |
115 | --- mixxx/src/midi/midimapping.cpp 2011-04-10 20:27:59 +0000 |
116 | +++ mixxx/src/midi/midimapping.cpp 2011-04-28 21:01:18 +0000 |
117 | @@ -705,7 +705,11 @@ |
118 | */ |
119 | void MidiMapping::savePreset(QString path) { |
120 | qDebug() << "Writing MIDI preset file" << path; |
121 | +<<<<<<< TREE |
122 | QMutexLocker locker(&m_mappingLock); |
123 | +======= |
124 | + QMutexLocker lock(&m_mappingLock); |
125 | +>>>>>>> MERGE-SOURCE |
126 | QFile output(path); |
127 | if (!output.open(QIODevice::WriteOnly | QIODevice::Truncate)) return; |
128 | QTextStream outputstream(&output); |
129 | |
130 | === modified file 'mixxx/src/midi/midioptiondelegate.cpp' |
131 | --- mixxx/src/midi/midioptiondelegate.cpp 2010-11-15 18:04:41 +0000 |
132 | +++ mixxx/src/midi/midioptiondelegate.cpp 2011-04-28 21:01:18 +0000 |
133 | @@ -21,6 +21,7 @@ |
134 | #define MIDI_OPT_STRING_HERC_JOG "HercJog" // Generic hercules wierd range correction |
135 | #define MIDI_OPT_STRING_SPREAD64 "Spread64" // Accelerated difference from 64 |
136 | #define MIDI_OPT_STRING_SELECTKNOB "SelectKnob" // Relative knob which can be turned forever and outputs a signed value. |
137 | +#define MIDI_OPT_STRING_SOFT_TAKEOVER "SoftTakeover" // Prevents sudden changes when hardware & software MixxxControl value differ |
138 | #define MIDI_OPT_STRING_SCRIPT "Script" // Maps a MIDI control to a custom MixxxScript function |
139 | |
140 | MidiOptionDelegate::MidiOptionDelegate(QObject *parent) |
141 | @@ -50,7 +51,8 @@ |
142 | case MIDI_OPT_HERC_JOG: text = MIDI_OPT_STRING_HERC_JOG; break; |
143 | case MIDI_OPT_SPREAD64: text = MIDI_OPT_STRING_SPREAD64; break; |
144 | case MIDI_OPT_SELECTKNOB: text = MIDI_OPT_STRING_SELECTKNOB; break; |
145 | - case MIDI_OPT_SCRIPT: text = MIDI_OPT_STRING_SCRIPT; break; |
146 | + case MIDI_OPT_SOFT_TAKEOVER: text = MIDI_OPT_STRING_SOFT_TAKEOVER; break; |
147 | + case MIDI_OPT_SCRIPT: text = MIDI_OPT_STRING_SCRIPT; break; |
148 | default: text = QString("%1").arg(midioption); break; |
149 | } |
150 | |
151 | @@ -77,6 +79,7 @@ |
152 | editor->addItem(MIDI_OPT_STRING_HERC_JOG); |
153 | editor->addItem(MIDI_OPT_STRING_SPREAD64); |
154 | editor->addItem(MIDI_OPT_STRING_SELECTKNOB); |
155 | + editor->addItem(MIDI_OPT_STRING_SOFT_TAKEOVER); |
156 | editor->addItem(MIDI_OPT_STRING_SCRIPT); |
157 | |
158 | return editor; |
159 | @@ -102,7 +105,8 @@ |
160 | case MIDI_OPT_HERC_JOG: comboIdx = 8; break; |
161 | case MIDI_OPT_SPREAD64: comboIdx = 9; break; |
162 | case MIDI_OPT_SELECTKNOB: comboIdx = 10; break; |
163 | - case MIDI_OPT_SCRIPT: comboIdx = 11; break; |
164 | + case MIDI_OPT_SOFT_TAKEOVER:comboIdx = 11; break; |
165 | + case MIDI_OPT_SCRIPT: comboIdx = 12; break; |
166 | default: comboIdx = 0; break; |
167 | } |
168 | comboBox->setCurrentIndex(comboIdx); |
169 | @@ -116,6 +120,7 @@ |
170 | //comboBox->interpretText(); |
171 | //Get the text from the combobox and turn it into a MidiMessage integer. |
172 | QString text = comboBox->currentText(); |
173 | + // Sigh, it sucks that we can't switch() on a string... |
174 | if (text == MIDI_OPT_STRING_NORMAL) //These come from the MidiOption enum (mixxxcontrol.h) |
175 | midioption = MIDI_OPT_NORMAL; |
176 | else if (text == MIDI_OPT_STRING_INVERT) |
177 | @@ -138,8 +143,10 @@ |
178 | midioption = MIDI_OPT_SPREAD64; |
179 | else if (text == MIDI_OPT_STRING_SELECTKNOB) |
180 | midioption = MIDI_OPT_SELECTKNOB; |
181 | + else if (text == MIDI_OPT_STRING_SOFT_TAKEOVER) |
182 | + midioption = MIDI_OPT_SOFT_TAKEOVER; |
183 | else if (text == MIDI_OPT_STRING_SCRIPT) |
184 | - midioption = MIDI_OPT_SCRIPT; |
185 | + midioption = MIDI_OPT_SCRIPT; |
186 | model->setData(index, midioption, Qt::EditRole); |
187 | } |
188 | |
189 | |
190 | === modified file 'mixxx/src/midi/midiscriptengine.cpp' |
191 | --- mixxx/src/midi/midiscriptengine.cpp 2011-04-16 20:56:59 +0000 |
192 | +++ mixxx/src/midi/midiscriptengine.cpp 2011-04-28 21:01:18 +0000 |
193 | @@ -717,7 +717,7 @@ |
194 | |
195 | ControlObjectThread *cot = getControlObjectThread(group, name); |
196 | |
197 | - if(cot != NULL) { |
198 | + if(cot != NULL && !m_st.ignore(group,name,newValue)) { |
199 | cot->slotSet(newValue); |
200 | } |
201 | |
202 | @@ -1262,3 +1262,15 @@ |
203 | |
204 | m_ramp[deck] = true; // Activate the ramping in scratchProcess() |
205 | } |
206 | + |
207 | +/* -------- ------------------------------------------------------ |
208 | + Purpose: [En/dis]ables soft-takeover status for a particular MixxxControl |
209 | + Input: MixxxControl group and key values, |
210 | + whether to set the soft-takeover status or not |
211 | + Output: - |
212 | + -------- ------------------------------------------------------ */ |
213 | +void MidiScriptEngine::softTakeover(QString group, QString name, bool set) { |
214 | + MixxxControl mc = MixxxControl(group,name); |
215 | + if (set) m_st.enable(mc); |
216 | + else m_st.disable(mc); |
217 | +} |
218 | \ No newline at end of file |
219 | |
220 | === modified file 'mixxx/src/midi/midiscriptengine.h' |
221 | --- mixxx/src/midi/midiscriptengine.h 2011-03-12 19:43:59 +0000 |
222 | +++ mixxx/src/midi/midiscriptengine.h 2011-04-28 21:01:18 +0000 |
223 | @@ -23,6 +23,8 @@ |
224 | #include "configobject.h" |
225 | #include "midimessage.h" |
226 | #include "pitchfilter.h" |
227 | +#include "softtakeover.h" |
228 | + |
229 | class MidiDevice; |
230 | |
231 | //Forward declaration(s) |
232 | @@ -62,6 +64,7 @@ |
233 | Q_INVOKABLE void scratchEnable(int deck, int intervalsPerRev, float rpm, float alpha, float beta); |
234 | Q_INVOKABLE void scratchTick(int deck, int interval); |
235 | Q_INVOKABLE void scratchDisable(int deck); |
236 | + Q_INVOKABLE void softTakeover(QString group, QString name, bool set); |
237 | |
238 | public slots: |
239 | void slotValueChanged(double value); |
240 | @@ -119,6 +122,7 @@ |
241 | QMutex m_scriptEngineLock; |
242 | QHash<ConfigKey, ControlObjectThread*> m_controlCache; |
243 | QHash<int, QPair<QString, bool> > m_timers; |
244 | + SoftTakeover m_st; |
245 | |
246 | // Scratching functions & variables |
247 | void scratchProcess(int timerId); |
248 | |
249 | === modified file 'mixxx/src/mixxxcontrol.cpp' |
250 | --- mixxx/src/mixxxcontrol.cpp 2010-11-15 18:04:41 +0000 |
251 | +++ mixxx/src/mixxxcontrol.cpp 2011-04-28 21:01:18 +0000 |
252 | @@ -35,7 +35,7 @@ |
253 | strMidiOption = "normal"; |
254 | } |
255 | |
256 | - if (strMidiOption == "normal") |
257 | + if (strMidiOption == "normal") // can't switch() on a string |
258 | m_midiOption = MIDI_OPT_NORMAL; |
259 | else if (strMidiOption == "invert") |
260 | m_midiOption = MIDI_OPT_INVERT; |
261 | @@ -57,6 +57,8 @@ |
262 | m_midiOption = MIDI_OPT_SPREAD64; |
263 | else if (strMidiOption == "selectknob") |
264 | m_midiOption = MIDI_OPT_SELECTKNOB; |
265 | + else if (strMidiOption == "soft-takeover") |
266 | + m_midiOption = MIDI_OPT_SOFT_TAKEOVER; |
267 | else if (strMidiOption == "script-binding") |
268 | m_midiOption = MIDI_OPT_SCRIPT; |
269 | else { |
270 | @@ -114,7 +116,7 @@ |
271 | QDomElement optionsNode = nodeMaker.createElement("options"); |
272 | QString strMidiOption; |
273 | int iMidiOption = this->getMidiOption(); |
274 | - if (iMidiOption == MIDI_OPT_NORMAL) |
275 | + if (iMidiOption == MIDI_OPT_NORMAL) // can't switch() on a string |
276 | strMidiOption = "normal"; |
277 | else if (iMidiOption == MIDI_OPT_INVERT) |
278 | strMidiOption = "invert"; |
279 | @@ -136,6 +138,8 @@ |
280 | strMidiOption = "spread64"; |
281 | else if (iMidiOption == MIDI_OPT_SELECTKNOB) |
282 | strMidiOption = "selectknob"; |
283 | + else if (iMidiOption == MIDI_OPT_SOFT_TAKEOVER) |
284 | + strMidiOption = "soft-takeover"; |
285 | else if (iMidiOption == MIDI_OPT_SCRIPT) |
286 | strMidiOption = "script-binding"; |
287 | else { |
288 | |
289 | === modified file 'mixxx/src/mixxxcontrol.h' |
290 | --- mixxx/src/mixxxcontrol.h 2010-11-15 18:04:41 +0000 |
291 | +++ mixxx/src/mixxxcontrol.h 2011-04-28 21:01:18 +0000 |
292 | @@ -16,6 +16,8 @@ |
293 | MIDI_OPT_HERC_JOG = 8, // Generic hercules wierd range correction |
294 | MIDI_OPT_SPREAD64 = 9, // Accelerated difference from 64 |
295 | MIDI_OPT_SELECTKNOB = 10,// Relative knob which can be turned forever and outputs a signed value. |
296 | + |
297 | + MIDI_OPT_SOFT_TAKEOVER = 40,// Prevents sudden changes when hardware position differs from software value |
298 | |
299 | MIDI_OPT_SCRIPT = 50,// Maps a MIDI control to a custom MixxxScript function |
300 | } MidiOption; |
301 | |
302 | === added file 'mixxx/src/softtakeover.cpp' |
303 | --- mixxx/src/softtakeover.cpp 1970-01-01 00:00:00 +0000 |
304 | +++ mixxx/src/softtakeover.cpp 2011-04-28 21:01:18 +0000 |
305 | @@ -0,0 +1,115 @@ |
306 | +/*************************************************************************** |
307 | + softtakeover.cpp - description |
308 | + ---------------- |
309 | + begin : Sat Mar 26 2011 |
310 | + copyright : (C) 2011 by Sean M. Pappalardo |
311 | + email : spappalardo@mixxx.org |
312 | + ***************************************************************************/ |
313 | + |
314 | +/*************************************************************************** |
315 | + * * |
316 | + * This program is free software; you can redistribute it and/or modify * |
317 | + * it under the terms of the GNU General Public License as published by * |
318 | + * the Free Software Foundation; either version 2 of the License, or * |
319 | + * (at your option) any later version. * |
320 | + * * |
321 | + ***************************************************************************/ |
322 | + |
323 | +#include "softtakeover.h" |
324 | +#include <math.h> // for fabs() |
325 | +#include <qdatetime.h> |
326 | + |
327 | +// HACK: remove this after Control 2.0 is here |
328 | +#include "controlpotmeter.h" |
329 | + |
330 | +// qint64 currentTimeMsecs() { |
331 | +uint SoftTakeover::currentTimeMsecs() { |
332 | + QDateTime currDT = QDateTime::currentDateTime(); |
333 | + |
334 | + // toMSecsSinceEpoch() is preferred since it's only one QDateTime call but |
335 | + // it requires Qt 4.7. Use it instead when something more substantial |
336 | + // elsewhere in Mixxx also requires Qt 4.7. |
337 | + //qint64 t = dt.toMSecsSinceEpoch(); // Requires Qt 4.7 |
338 | + uint t = currDT.toTime_t()*1000+currDT.toString("zzz").toUInt(); |
339 | + return t; |
340 | +} |
341 | + |
342 | +// For legacy Controls |
343 | +void SoftTakeover::enable(QString group, QString name) { |
344 | + MixxxControl mixxxControl = MixxxControl(group,name); |
345 | + enable(mixxxControl); |
346 | +} |
347 | + |
348 | +void SoftTakeover::enable(MixxxControl control) { |
349 | + if (!m_times.contains(control)) m_times.insert(control,currentTimeMsecs()); |
350 | +} |
351 | + |
352 | +// For legacy Controls |
353 | +void SoftTakeover::disable(QString group, QString name) { |
354 | + MixxxControl mixxxControl = MixxxControl(group,name); |
355 | + disable(mixxxControl); |
356 | +} |
357 | + |
358 | +void SoftTakeover::disable(MixxxControl control) { |
359 | + m_times.remove(control); |
360 | +} |
361 | + |
362 | +// For legacy Controls |
363 | +bool SoftTakeover::ignore(QString group, QString name, float newValue) { |
364 | + MixxxControl mixxxControl = MixxxControl(group,name); |
365 | + return ignore(mixxxControl,newValue); |
366 | +} |
367 | + |
368 | +bool SoftTakeover::ignore(MixxxControl mc, float newValue, bool midiVal) { |
369 | + bool ignore = false; |
370 | + QString message; |
371 | + if (m_times.contains(mc)) { |
372 | + // We only want to ignore the MIDI controller when all of the following are true: |
373 | + // - its new value is far away from the MixxxControl |
374 | + // - it's been awhile since the last MIDI message for this control affected it |
375 | + |
376 | + // 3/128 units away from the current is enough to catch fast non-sequential moves |
377 | + // but not cause an audially noticeable jump. |
378 | + float threshold = 3; |
379 | + |
380 | + ControlObject* temp = ControlObject::getControl( |
381 | + ConfigKey(mc.getControlObjectGroup(), mc.getControlObjectValue())); |
382 | + |
383 | + if (temp == NULL) return ignore; |
384 | + |
385 | + if (!midiVal) { |
386 | + // These defaults will effectively disable soft-takeover for this pass |
387 | + // (causing the control to jump to the new value regardless) |
388 | + // if there's a problem with the below CO being NULL |
389 | + double maxValue=10000000; // Anything, just higher than any CO can go |
390 | + double minValue=0; |
391 | + |
392 | + // HACK until we have Control 2.0. It can't come soon enough... |
393 | + ControlPotmeter* cpo = dynamic_cast<ControlPotmeter*>(temp); // for getMax/getMin |
394 | + if (cpo != NULL) { |
395 | + maxValue = cpo->getMax(); |
396 | + minValue = cpo->getMin(); |
397 | + } |
398 | + // End hack |
399 | + |
400 | + double scaleFactor = maxValue-minValue; |
401 | + threshold = scaleFactor*(threshold/128); |
402 | + } |
403 | + |
404 | + double oldValue; |
405 | + if (midiVal) oldValue = temp->GetMidiValue(); |
406 | + else oldValue = temp->get(); |
407 | + double difference = oldValue - newValue; |
408 | + |
409 | + uint currentTime = currentTimeMsecs(); |
410 | + if (fabs(difference)>threshold |
411 | + && (currentTime - m_times.value(mc)) > SUBSEQUENT_VALUE_OVERRIDE_TIME) { |
412 | + ignore = true; |
413 | + } |
414 | + if (!ignore) { |
415 | + // Update the time only if the value is not ignored |
416 | + m_times.insert(mc,currentTime); // Replaces any previous value for this MixxxControl |
417 | + } |
418 | + } |
419 | + return ignore; |
420 | +} |
421 | |
422 | === added file 'mixxx/src/softtakeover.h' |
423 | --- mixxx/src/softtakeover.h 1970-01-01 00:00:00 +0000 |
424 | +++ mixxx/src/softtakeover.h 2011-04-28 21:01:18 +0000 |
425 | @@ -0,0 +1,50 @@ |
426 | +/*************************************************************************** |
427 | + softtakeover.h - description |
428 | + -------------- |
429 | + begin : Thu Mar 17 2011 |
430 | + copyright : (C) 2011 by Sean M. Pappalardo |
431 | + email : spappalardo@mixxx.org |
432 | + ***************************************************************************/ |
433 | + |
434 | +/*************************************************************************** |
435 | + * * |
436 | + * This program is free software; you can redistribute it and/or modify * |
437 | + * it under the terms of the GNU General Public License as published by * |
438 | + * the Free Software Foundation; either version 2 of the License, or * |
439 | + * (at your option) any later version. * |
440 | + * * |
441 | + ***************************************************************************/ |
442 | +#ifndef SOFTTAKEOVER_H |
443 | +#define SOFTTAKEOVER_H |
444 | + |
445 | +#include "mixxxcontrol.h" |
446 | + |
447 | +class SoftTakeover { |
448 | + |
449 | + public: |
450 | + /** Enable soft-takeover for the given Control |
451 | + It's okay to call this on a Control that's already enabled. */ |
452 | + void enable(MixxxControl control); |
453 | + void enable(QString group, QString name); |
454 | + /** Disable soft-takeover for the given Control */ |
455 | + void disable(MixxxControl control); |
456 | + void disable(QString group, QString name); |
457 | + /** Check to see if the new value for the Control should be ignored */ |
458 | + bool ignore(MixxxControl control, float newValue, bool midiVal = false); |
459 | + /** For legacy Controls */ |
460 | + bool ignore(QString group, QString name, float newValue); |
461 | + |
462 | + private: |
463 | + /** If a new value is received within this amount of time, |
464 | + jump to it regardless. This allows quickly whipping controls to work |
465 | + while retaining the benefits of soft-takeover for slower movements. |
466 | + |
467 | + Setting this too high will defeat the purpose of soft-takeover.*/ |
468 | + static const uint SUBSEQUENT_VALUE_OVERRIDE_TIME = 50; // Milliseconds |
469 | + //qint64 currentTimeMsecs(); |
470 | + //QHash<MixxxControl,qint64> m_times; |
471 | + uint currentTimeMsecs(); |
472 | + QHash<MixxxControl,uint> m_times; |
473 | +}; |
474 | + |
475 | +#endif |
476 | \ No newline at end of file |
Hey Sean,
I'm going to give you a hard time because you're a developer. :)
Two questions:
1) How is this useful in its current incarnation if you can't have
multiple <options> blocks?
2) Why does this have to go in 1.9? None of the mappings are using it. ??
3) What are the SCS.3d changes you made? Should we cherry-pick those
changes and put them in trunk before everything else?
Thanks,
Albert
On Tue, Nov 16, 2010 at 4:58 PM, Sean M. Pappalardo /bugs.launchpad .net/bugs/ 555547 /code.launchpad .net/~mixxxdeve lopers/ mixxx/features_ softtakeover/ +merge/ 41027 res/midi/ Stanton- SCS3d-scripts. js' midi/Stanton- SCS3d-scripts. js 2010-06-29 11:58:44 +0000 midi/Stanton- SCS3d-scripts. js 2010-11-17 00:57:57 +0000 triggerS4 = 0xFF; modeSignals = {"fx":[ ["[Flanger]", "lfoDepth", "StantonSCS3d. FXDepthLEDs" ], modeSignals = {"fx":[ ["[Flanger]", "lfoDepth", "StantonSCS3d. FXDepthLEDs" ], FXDelayLEDs" ], FXPeriodLEDs" ], B11LED" ], B11LED" ] ], B11LED" ] ], B11LED" ], BsALED" ], BsBLED" ],
<email address hidden> wrote:
> Sean M. Pappalardo has proposed merging lp:~mixxxdevelopers/mixxx/features_softtakeover into lp:mixxx.
>
> Requested reviews:
> Mixxx Development Team (mixxxdevelopers)
> Related bugs:
> #555547 Add soft-takeover for MIDI controllers (XML and script)
> https:/
>
>
> This branch adds soft-takeover capabilities for MIDI controllers, both for simple XML mappings and MIDI scripting.
>
> I'm requesting that this be included in time for Mixxx v1.9.0.
>
> Note that the main goal was to add this for MIDI scripting. I did it for the XML mappings as well for completeness, but that part isn't totally useful until Mixxx can accept multiple <option>s for simple XML mappings. Adding that capability is beyond the scope of the feature itself since everything is hard-coded for a single <option>, and it would require modification of GUI classes as well, something I don't feel qualified to do, especially not with the hope of getting this merged in time for 1.9.0.
> --
> https:/
> Your team Mixxx Development Team is requested to review the proposed merge of lp:~mixxxdevelopers/mixxx/features_softtakeover into lp:mixxx.
>
> === modified file 'mixxx/
> --- mixxx/res/
> +++ mixxx/res/
> @@ -64,7 +64,7 @@
> StantonSCS3d.
>
> // Signals to (dis)connect by mode: Group, Key, Function name
> -StantonSCS3d.
> +StantonSCS3d.
> ["[Flanger]", "lfoDelay", "StantonSCS3d.
> ["[Flanger]", "lfoPeriod", "StantonSCS3d.
> ["CurrentChannel", "reverse", "StantonSCS3d.
> @@ -80,77 +80,77 @@
> "loop2":[ ["CurrentChannel", "pfl", "StantonSCS3d.
> "loop3":[ ["CurrentChannel", "pfl", "StantonSCS3d.
> "trig":[ ["CurrentChannel", "pfl", "StantonSCS3d.
> - ["CurrentChannel", "hotcue_1_enabled", "StantonSCS3d.
> - ["CurrentChannel", "hotcue_2_enabled", "StantonSCS3d.
> - ["CurrentChannel...