Merge lp:~mixxxdevelopers/mixxx/features_scriptTimers into lp:~mixxxdevelopers/mixxx/trunk

Proposed by Sean M. Pappalardo
Status: Merged
Merged at revision: not available
Proposed branch: lp:~mixxxdevelopers/mixxx/features_scriptTimers
Merge into: lp:~mixxxdevelopers/mixxx/trunk
Diff against target: 893 lines (+406/-132)
10 files modified
mixxx/res/midi/midi-mappings-scripts.js (+1/-1)
mixxx/src/main.cpp (+4/-0)
mixxx/src/midi/mididevice.cpp (+4/-5)
mixxx/src/midi/mididevice.h (+2/-0)
mixxx/src/midi/midimapping.cpp (+43/-45)
mixxx/src/midi/midimapping.h (+7/-2)
mixxx/src/midi/midimessage.cpp (+1/-0)
mixxx/src/midi/midiscriptengine.cpp (+325/-63)
mixxx/src/midi/midiscriptengine.h (+19/-11)
mixxx/src/mixxx.cpp (+0/-5)
To merge this branch: bzr merge lp:~mixxxdevelopers/mixxx/features_scriptTimers
Reviewer Review Type Date Requested Status
RJ Skerry-Ryan Approve
Albert Santoni Needs Fixing
Sean M. Pappalardo Needs Resubmitting
Review via email: mp+17850@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

MIDI script timers have been successfully added to the MIDI script engine, and tested on an SCS.3d using a light show script. (The venerable light show script first seen on YouTube has been updated to no longer run an endless loop! :) The light show can now be paused, restarted, and run without noticeably impacting Mixxx's performance.)

I respectfully request consideration of merging this in time for the 1.8 release for the following reasons:

1) The timer functionality has been added on to the MIDI script engine (as opposed to changing anything,) so the core functionality has not been altered
2) Good scratching performance requires the use of fixed-interval control sampling which is not possible without timers. This is one area where Mixxx is lagging its competitors and having it ready for 1.8 would help close the gap. (I plan to modify the common script scratching functions to make use of the timers (which won't take long) after this is merged, allowing all scripts that currently use the scratch.* common functions to take advantage of it immediately. (Some minor tweaks may be needed.))
3) Timers also allow script programmers to flash lights on MIDI controllers (such as when nearing the end of a track, setting a cue point (like CDJs,) or changing modes) without busy-wait loops which are inaccurate and have an adverse effect on Mixxx's playback.

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Hey Sean,

Thanks for working on this. It looks like a feature that will provide needed flexibility to script writers. I've looked it over and I have a few comments.

On the primer timer:
- Why did you pick 20ms? Will a slower timer serve the same purpose of making the other timers work? (e.g. can we set the primer timer to something like an hour? a day?)
- Did you ever discover why startTimer() stops working unless the primer timer is created? It seems like this is either a bug in Qt or a bug in our code.

Honestly this is the most concerning part of this branch and I think it needs fixing. This means that a high resolution timer will always be executing for every MIDI device you plug in (one per script engine) even if your scripts aren't using timers (high-or-low resolution) at all.

First off, the approach is very clean (other than the primer timer) -- which I do like. I am concerned that a script could bring down Mixxx by creating 100 high resolution timers. The script engine thread could strangle the engine. (Also, MidiScriptEngine should set its thread priority lower.) We already have bad thread contention on single-CPU machines so this makes me worry. When you were implementing this design, did you put any thought into the alternate (less clean) approach we discussed where you would create one Qt script timer and run the script timers on that main timer? You would just keep track of the last time each timer got triggered and trigger the timer if the current time is greater than the previous time plus the timer's interval. What made you go with this approach rather than that one?

- Do you have any profiling output to support that this approach is not taxing?
- Have you experimented with malicious (or dumb) script writing to figure out just how many timers your machine can support before Mixxx performance is degraded? I'm very interested to see data like this for single-CPU low-power machines.

I also have some minor code comments:

~MidiScriptEngine()
- When calling stopTimer(), use a QMutableHashIterator and remove each key before calling stopTimer. You do hold the script engine lock, but it can't hurt to do it safer.

beginTimer()
- No bounds checking on the 'interval' variable (should at least be non-zero). Have you given any thought to preventing scripts from making high-resolution timers? (e.g. sub-10ms) I don't think that a script should be able to affect Mixxx's performance.

stopTimer()
- No validity checking on timerId variable. (Should check that the hash contains the timerId key first before calling stopTimer -- Qt doesn't specify its behavior if the timer id you pass is invalid, so you can only assume it crashes or asserts).

timerEvent()
- Please check that event.timerId() is in m_timers before indexing into the hash (would cause an assert and kill Mixxx). Suppose killTimer has a delayed effect or something and a timerEvent for a timer that was just killTimer'd can occur (Qt doesn't make any promises like this). You could run into a situation where the timer id is not in the hash. Better safe than sorry.

review: Needs Information
Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :
Download full text (4.9 KiB)

> On the primer timer:
> - Why did you pick 20ms? Will a slower timer serve the same purpose of making
> the other timers work? (e.g. can we set the primer timer to something like an
> hour? a day?)

No, since timers set up before exec() apparently block any other timers, forcing them to fire only up to as often as the shortest pre-exec() one. Also 20ms is the highest resolution that most platforms support. (Linux apparently supports higher, but I noticed a ~10% CPU usage increase at idle when I had it fire as often as possible (interval 0) on this 2.53GHz Celeron D.)

> - Did you ever discover why startTimer() stops working unless the primer timer
> is created? It seems like this is either a bug in Qt or a bug in our code.

Our code is straightforward. Unless the event loop/thread is somehow being set up incorrectly, I'm thinking it's Qt.

> Honestly this is the most concerning part of this branch and I think it needs
> fixing. This means that a high resolution timer will always be executing for
> every MIDI device you plug in (one per script engine) even if your scripts
> aren't using timers (high-or-low resolution) at all.

I agree. It bothers me a lot.

> First off, the approach is very clean (other than the primer timer) -- which I
> do like. I am concerned that a script could bring down Mixxx by creating 100
> high resolution timers. The script engine thread could strangle the engine.

That's possible, though Qt will refuse to set up a timer if the OS has reached its limit. But how do we decide what an acceptable upper bound is without limiting future expandability?

> (Also, MidiScriptEngine should set its thread priority lower.)

Done, set to QThread::LowPriority, though I worry a bit about responsiveness and scratching performance (until we implement that in the engine.)

> When you
> were implementing this design, did you put any thought into the alternate
> (less clean) approach we discussed where you would create one Qt script timer
> and run the script timers on that main timer?

Not really. I just wanted to see if the simplest design would work acceptably, and it does! :) Though I don't see how doing what you suggest would help much since you'd still need a high resolution timer running all the time.

> You would just keep track of the
> last time each timer got triggered and trigger the timer if the current time
> is greater than the previous time plus the timer's interval. What made you go
> with this approach rather than that one?

Well, the OS already handles that, (and probably better than I would) so why do the work again? And like I said above, I wanted to try the simplest method first (not knowing about the need for this primer timer.) That and QBasicTimer/QTimer operate using the same event loop mechanism. They just make timer management more convenient, at the cost of some overhead.

> - Do you have any profiling output to support that this approach is not
> taxing?

It IS taxing if the primer timer runs any more often. (I saw a 10% CPU increase on this 2.53GHz Celeron-D when I tried the primer at interval 0 (run every time there are no more events in the queue.)) That said, running it at 20ms caused only a slight aver...

Read more...

review: Needs Resubmitting
Revision history for this message
Adam Davison (adamdavison) wrote :

This is mostly not my area of expertise but Sean I have one question. You mention that this is required for good scratching. Can you explain why this is the case? Surely ultimately for high performance scratching you want up to ~1ms precision which as you mention, operating system timers cannot provide. And in fact you aren't sampling the "velocity" of the midi wheels, you're getting discrete delta position numbers from the midi controller. Surely the best approach is to process these scratch deltas as they arrive in some way, not as part of a timer loop which necessarily introduces additional latency?

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

> This is mostly not my area of expertise but Sean I have one question. You
> mention that this is required for good scratching. Can you explain why this is
> the case?

I mean it's required in order to continue to do it in script. Ultimately we'll be doing it in the Engine, but that's a ways off and I'd like to get some improvement on this into 1.8.

> And in fact you aren't sampling the "velocity" of the midi wheels, you're
> getting discrete delta position numbers from the midi controller. Surely the
> best approach is to process these scratch deltas as they arrive in some way,

That's exactly my plan, using the alpha-beta filter code already in scratch.* script functions. But an alpha-beta filter is a time-sensitive one and without interval-based sampling, there is no way to tell it the wheel has stopped, so it's starved of data in that case instead of realizing that it needs to rapidly decelerate. (That's why in 1.7 it acts so screwy when you just hold your finger in one spot on the SCS.3d's red vinyl mode.)

Revision history for this message
Adam Davison (adamdavison) wrote :

> > And in fact you aren't sampling the "velocity" of the midi wheels, you're
> > getting discrete delta position numbers from the midi controller. Surely the
> > best approach is to process these scratch deltas as they arrive in some way,
>
> That's exactly my plan, using the alpha-beta filter code already in scratch.*
> script functions. But an alpha-beta filter is a time-sensitive one and without
> interval-based sampling, there is no way to tell it the wheel has stopped, so
> it's starved of data in that case instead of realizing that it needs to
> rapidly decelerate. (That's why in 1.7 it acts so screwy when you just hold
> your finger in one spot on the SCS.3d's red vinyl mode.)

Ok, so the timer basically just provides some kind of nothing-happened-in-this-tick signal to the filter. That all sounds reasonable.

Thanks,

Adam

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

Resubmitting for review.

I discovered that the primer timer was a hack and the real problem is that the MidiDevice thread was executing all of the script functions, including timer creation! (Timers must be created in the thread in which they're to fire, i.e. the MidiScriptEngine thread.)

I've untangled all of that and now the MidiScriptEngine takes care of loading, initializing, and shutting down all MIDI scripts, as it should. It and the MidiMapping communicate via signals so things happen in the correct order (the MidiMapping tells the MidiScriptEngine to do one of those tasks then waits on its signal to continue.) Timers now work perfectly as originally expected on Linux and Windows with no appreciable CPU increase (unless you run 3000 timers at once...which I tried, and Mixxx is still responsive up but you notice it going slower. :) )

(This should also help resolve any strange script engine crashes we'd seen in bug reports.)

review: Needs Resubmitting
Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Looking good Sean. Glad to see the primer timer is gone. I have a few comments / fixes.

MidiScriptEngine::MidiScriptEngine(...)

- Passing references to m_scriptFileNames and m_scriptFunctionPrefixes is not
  good. They are QList's, which are not thread safe. Two threads accessing a
  list at the same time will segfault. You should make these two lists arguments
  to the signals that deal with them and pass by value, not reference.

- On the diversity of callMidiScriptFunction signals and execute methods with
  multiple parameters: In the end we just make a QScriptValueList and we don't
  care about the individual types of the parameters. I say we just accept one
  signal for executing a script function: callMidiScriptFunction(QString name,
  QList<QVariant> arguments) and consolidate all the execute(...) methods into
  one execute(QString function, QList<QVariant> arguments); method.

MidiScriptEngine::gracefulShutdown()

- Need to hold the engine lock before calling m_connectedControls.clear()

MidiScriptEngine::loadScriptFile()

- As I mentioned, convert this to take m_rScriptFileNames as an argument instead
  of sharing it between the classes. Is there any problem I'm not seeing with doing it this way?

MidiScriptEngine::stopAllTimers()

- Please add comment to the top that the caller should hold the script lock.

MidiScriptEngine::timerEvent()

- Need to hold the lock to user m_timers. I know that this is technically safe,
  we're just going for overkill here :).

Overall

- qWarnings and qCritical - I really don't think these should anywhere in the
  script engine. Maybe it's ok for a script function that only happens on
  initialization -- but in the end no DJ cares about a syntax error more than
  s/he cares about Mixxx not going down in the middle of their set.

review: Needs Fixing
2273. By Sean M. Pappalardo

Addressed RJ's code review concerns:
- got rid of the reference passing between threads and replaced with value passing on signals (it's actually neater this way too)
- moved the lock in gracefulShutdown() to include clearing m_connectedControls
- added lock assert to stopAllTimers() too
- moved the lock in timerEvent to include m_timers[] access

2274. By Sean M. Pappalardo

- Made the midiDebug flag global in the MidiScriptEngine instead of asking the MidiDevice for it each time
- Replaced obsolete canEvaluate() with checkSyntax()
- Mixxx now shows friendly warning dialogs and no longer aborts on script errors (unless run with --midiDebug)

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

Another resubmit.

> - Passing references to m_scriptFileNames and m_scriptFunctionPrefixes is not
> good.

Fixed.

> I say we just accept one
> signal for executing a script function: callMidiScriptFunction(QString name,
> QList<QVariant> arguments) and consolidate all the execute(...) methods into
> one execute(QString function, QList<QVariant> arguments); method.

I agree, though right now the only thing that signals the MidiScriptEngine is MidiMapping and the existing two signals are all it needs. So I'm thinking we should defer this consolidation unless/until it ever becomes an issue. (Also, can you even send null QLists for situations where we want to execute a parameter-less function or a non-function piece of script code?)

> MidiScriptEngine::gracefulShutdown()
> - Need to hold the engine lock before calling m_connectedControls.clear()

Fixed.

> MidiScriptEngine::loadScriptFile()
> - As I mentioned, convert this to take m_rScriptFileNames as an argument
> instead of sharing it between the classes.

Done, works fine.

> MidiScriptEngine::stopAllTimers()
> - Please add comment to the top that the caller should hold the script lock.

Did you one better and asserted that the lock is held (kind of redundant since stopTimers() also checks, but whatever.)

> MidiScriptEngine::timerEvent()
> - Need to hold the lock to user m_timers.

Fixed.

> - qWarnings and qCritical - I really don't think these should anywhere in the
> script engine. Maybe it's ok for a script function that only happens on
> initialization -- but in the end no DJ cares about a syntax error more than
> s/he cares about Mixxx not going down in the middle of their set.

Agreed. I examined the various things that were qCriticals and discovered that QScriptEngine::canEvaluate() has been obsoleted. So those qCriticals have been removed. I added the checkSyntax() system in their place and I have it pop up qCriticals with the error details only if --midiDebug is enabled, otherwise it qDebugs the details and pops up a friendly qWarning which the user can acknowledge and continue to mix (though with the controller in a degraded state.) As for loops, only the first warning dialog comes up; all others are supressed until the first is acknowledged. (So the user will see one each time he does something that calls an offending function.) However, if the syntax error is discovered on file load, that warning dialog shows up only once (each time the script file is (re)loaded) and informs the user that the offending script will be disabled.

One thing to note about allowing the user to continue on a script error: the offending function aborts on the statement that caused the exception, so the rest of the function doesn't execute and could leave the controller and script global variables in an indeterminate state (for example a mode change could have happened but the LEDs didn't get updated to show it.)

review: Needs Resubmitting
2275. By Sean M. Pappalardo

Merging from trunk

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

Resubmitting after trunk merge (seeing if it realizes I resolved the SConscript "conflict.")

Also, confirmed that the fixes in this branch eliminate the segfault-on-close 90% of the time in Windows resulting from a qWarning trying to appear to tell us of a race condition "[Main]: QWaitCondition: Destroyed while threads are still waiting." This must've been related to MidiScriptEngine threads since it doesn't happen in this branch.

review: Needs Resubmitting
Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :

One tester confirmed that this appears to indeed fix a scriptEngine crash from 1.7, though I'd have to back-port to be sure. I'm not sure that's worth the time though. https://bugs.launchpad.net/mixxx/+bug/493793/comments/30

2276. By Sean M. Pappalardo

- Removed _exit() hack for Windows
- Added "shutdown complete" message for closure

2277. By Sean M. Pappalardo

Updated from trunk

Revision history for this message
Sean M. Pappalardo (pegasus-renegadetech) wrote :
2278. By Sean M. Pappalardo

Merging from trunk

2279. By Sean M. Pappalardo

Merging from trunk

2280. By Sean M. Pappalardo

Mo' comments

2281. By Sean M. Pappalardo

Merging from trunk again

Revision history for this message
Albert Santoni (gamegod) wrote :
Download full text (3.2 KiB)

> Another resubmit.
>
> > - Passing references to m_scriptFileNames and m_scriptFunctionPrefixes is
> not
> > good.
>
> Fixed.
>
> > I say we just accept one
> > signal for executing a script function: callMidiScriptFunction(QString
> name,
> > QList<QVariant> arguments) and consolidate all the execute(...) methods
> into
> > one execute(QString function, QList<QVariant> arguments); method.
>
> I agree, though right now the only thing that signals the MidiScriptEngine is
> MidiMapping and the existing two signals are all it needs. So I'm thinking we
> should defer this consolidation unless/until it ever becomes an issue. (Also,
> can you even send null QLists for situations where we want to execute a
> parameter-less function or a non-function piece of script code?)
>
> > MidiScriptEngine::gracefulShutdown()
> > - Need to hold the engine lock before calling m_connectedControls.clear()
>
> Fixed.
>
> > MidiScriptEngine::loadScriptFile()
> > - As I mentioned, convert this to take m_rScriptFileNames as an argument
> > instead of sharing it between the classes.
>
> Done, works fine.
>
> > MidiScriptEngine::stopAllTimers()
> > - Please add comment to the top that the caller should hold the script lock.
>
> Did you one better and asserted that the lock is held (kind of redundant since
> stopTimers() also checks, but whatever.)
>
> > MidiScriptEngine::timerEvent()
> > - Need to hold the lock to user m_timers.
>
> Fixed.
>
> > - qWarnings and qCritical - I really don't think these should anywhere in
> the
> > script engine. Maybe it's ok for a script function that only happens on
> > initialization -- but in the end no DJ cares about a syntax error more
> than
> > s/he cares about Mixxx not going down in the middle of their set.
>
> Agreed. I examined the various things that were qCriticals and discovered that
> QScriptEngine::canEvaluate() has been obsoleted. So those qCriticals have been
> removed. I added the checkSyntax() system in their place and I have it pop up
> qCriticals with the error details only if --midiDebug is enabled, otherwise it
> qDebugs the details and pops up a friendly qWarning which the user can
> acknowledge and continue to mix (though with the controller in a degraded
> state.) As for loops, only the first warning dialog comes up; all others are
> supressed until the first is acknowledged. (So the user will see one each time
> he does something that calls an offending function.) However, if the syntax
> error is discovered on file load, that warning dialog shows up only once (each
> time the script file is (re)loaded) and informs the user that the offending
> script will be disabled.
>
> One thing to note about allowing the user to continue on a script error: the
> offending function aborts on the statement that caused the exception, so the
> rest of the function doesn't execute and could leave the controller and script
> global variables in an indeterminate state (for example a mode change could
> have happened but the LEDs didn't get updated to show it.)

Sounds like you should give an option to reload the script when there's an exception. I'd rather have my controller lose whatever mode it's...

Read more...

Revision history for this message
Albert Santoni (gamegod) wrote :

How should I be testing this code? The scratching on the SCS.3d seems asymmetric to me in the branch, as in stabbing forwards and backwards go at different rates.

There's a lot of great code here, I think you've done a great job with the script engine overall and you should be very proud of it. :)

The only inconsistency in the code I've found (so far) is in MidiScriptEngine::timerEvent(QTimerEvent *event). In the first "if" statement, you access m_timers without holding the script engine lock first. In every other case where m_timers is accessed in the midiscriptengine.cpp, the lock is always held first. So I would move the m_scriptEngineLock.lock() call that's already in the function right to the start, and then make sure you unlock it before you return in that if statement.

Otherwise, looking good to me...

Thanks!
Albert

review: Needs Fixing
Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

I agree with Albert's timerEvent comment. I committed a fix and I re-reviewed the code. It all look good to me, so I'm marking approved. I'll let you do the honors Sean :).

review: Approve
2282. By RJ Skerry-Ryan

Move lock in MidiScriptEngine::timerEvent to resolve thread-safety issue.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'mixxx/res/midi/midi-mappings-scripts.js'
2--- mixxx/res/midi/midi-mappings-scripts.js 2010-03-06 21:32:16 +0000
3+++ mixxx/res/midi/midi-mappings-scripts.js 2010-03-24 04:40:48 +0000
4@@ -239,7 +239,7 @@
5
6 // The actual alpha-beta filter
7 scratch.filter = function (currentDeck, controlValue, revtime, alpha, beta) {
8- // ------------- Thanks to Radiomark (of Xwax) for the info for below ------------------------
9+ // ------------- Thanks to Mark Hills of Xwax (http://www.xwax.co.uk) for the info for below ------------------------
10
11 // ideal position = (initial_p + (y - x) / 128 * 1.8)
12 var ideal_p = scratch.variables["initialTrackPos"] + (controlValue - scratch.variables["initialControlValue"]) / 128 * revtime;
13
14=== modified file 'mixxx/src/main.cpp'
15--- mixxx/src/main.cpp 2010-03-11 12:14:05 +0000
16+++ mixxx/src/main.cpp 2010-03-24 04:40:48 +0000
17@@ -351,6 +351,10 @@
18
19 delete mixxx;
20
21+ qDebug() << "Mixxx shutdown complete.";
22+
23+ // Don't make any more output after this
24+ // or mixxx.log will get clobbered!
25 if(Logfile.isOpen())
26 Logfile.close();
27
28
29=== modified file 'mixxx/src/midi/mididevice.cpp'
30--- mixxx/src/midi/mididevice.cpp 2009-11-26 06:15:01 +0000
31+++ mixxx/src/midi/mididevice.cpp 2010-03-24 04:40:48 +0000
32@@ -210,11 +210,10 @@
33 //to the device. (sendShortMessage() would try to lock m_mutex...)
34 locker.unlock();
35
36- if (!m_pMidiMapping->getMidiScriptEngine()->execute(configKey.item, channel,
37- control, value, status,
38- mixxxControl.getControlObjectGroup())) {
39- qDebug() << "MidiDevice: Invalid script function" << configKey.item;
40- }
41+ // This needs to be a signal because the MIDI Script Engine thread must execute
42+ // script functions, not this MidiDevice one
43+ emit(callMidiScriptFunction(configKey.item, channel, control, value, status,
44+ mixxxControl.getControlObjectGroup()));
45 return;
46 }
47 #endif
48
49=== modified file 'mixxx/src/midi/mididevice.h'
50--- mixxx/src/midi/mididevice.h 2009-11-26 06:15:01 +0000
51+++ mixxx/src/midi/mididevice.h 2010-03-24 04:40:48 +0000
52@@ -71,6 +71,8 @@
53
54 signals:
55 void midiEvent(MidiMessage message);
56+ void callMidiScriptFunction(QString function, char channel, char control, char value, MidiStatusByte status, QString group);
57+
58 protected:
59 /** Verbose device name, in format "[index]. [device name]". Suitable for display in GUI. */
60 QString m_strDeviceName;
61
62=== modified file 'mixxx/src/midi/midimapping.cpp'
63--- mixxx/src/midi/midimapping.cpp 2010-01-05 03:05:02 +0000
64+++ mixxx/src/midi/midimapping.cpp 2010-03-24 04:40:48 +0000
65@@ -47,6 +47,8 @@
66 qDebug() << "Creating new MIDI presets directory" << BINDINGS_PATH;
67 QDir().mkpath(BINDINGS_PATH);
68 }
69+ // So we can signal the MidiScriptEngine and pass a QList
70+ qRegisterMetaType<QList<QString> >("QList<QString>");
71
72 //Q_ASSERT(outputMidiDevice);
73
74@@ -85,6 +87,7 @@
75
76 m_pScriptEngine->moveToThread(m_pScriptEngine);
77
78+ // Let the script engine tell us when it's done with each task
79 connect(m_pScriptEngine, SIGNAL(initialized()),
80 this, SLOT(slotScriptEngineReady()),
81 Qt::DirectConnection);
82@@ -94,58 +97,53 @@
83 m_scriptEngineInitializedCondition.wait(&m_scriptEngineInitializedMutex);
84 m_scriptEngineInitializedMutex.unlock();
85
86+ // Allow this object to signal the MidiScriptEngine to load the list of script files
87+ connect(this, SIGNAL(loadMidiScriptFiles(QList<QString>)), m_pScriptEngine, SLOT(loadScriptFiles(QList<QString>)));
88+ // Allow this object to signal the MidiScriptEngine to run the initialization
89+ // functions in the loaded scripts
90+ connect(this, SIGNAL(initMidiScripts(QList<QString>)), m_pScriptEngine, SLOT(initializeScripts(QList<QString>)));
91+ // Allow this object to signal the MidiScriptEngine to run its shutdown routines
92+ connect(this, SIGNAL(shutdownMidiScriptEngine(QList<QString>)), m_pScriptEngine, SLOT(gracefulShutdown(QList<QString>)));
93+
94+ // Allow this object to signal the MidiScriptEngine to call functions
95+ connect(this, SIGNAL(callMidiScriptFunction(QString)),
96+ m_pScriptEngine, SLOT(execute(QString)));
97+ connect(this, SIGNAL(callMidiScriptFunction(QString, QString)),
98+ m_pScriptEngine, SLOT(execute(QString, QString)));
99 }
100
101 void MidiMapping::loadScriptCode() {
102 QMutexLocker Locker(&m_mappingLock);
103-
104- ConfigObject<ConfigValue> *config = new ConfigObject<ConfigValue>(QDir::homePath().append("/").append(SETTINGS_PATH).append(SETTINGS_FILE));
105-
106- qDebug() << "MidiMapping: Loading & evaluating all MIDI script code";
107-
108- QListIterator<QString> it(m_pScriptFileNames);
109- while (it.hasNext()) {
110- QString curScriptFileName = it.next();
111- m_pScriptEngine->evaluate(config->getConfigPath().append("midi/").append(curScriptFileName));
112-
113- if(m_pScriptEngine->hasErrors(curScriptFileName)) {
114- qDebug() << "Errors occured while loading " << curScriptFileName;
115- }
116+ if(m_pScriptEngine) {
117+ m_scriptEngineInitializedMutex.lock();
118+ // Tell the script engine to run the init function in all loaded scripts
119+ emit(loadMidiScriptFiles(m_scriptFileNames));
120+ // Wait until it's done
121+ m_scriptEngineInitializedCondition.wait(&m_scriptEngineInitializedMutex);
122+ m_scriptEngineInitializedMutex.unlock();
123 }
124 }
125
126 void MidiMapping::initializeScripts() {
127 QMutexLocker Locker(&m_mappingLock);
128 if(m_pScriptEngine) {
129- // Call each script's init function if it exists
130- QListIterator<QString> prefixIt(m_pScriptFunctionPrefixes);
131- while (prefixIt.hasNext()) {
132- QString initName = prefixIt.next();
133- if (initName!="") {
134- initName.append(".init");
135- qDebug() << "MidiMapping: Executing" << initName;
136- if (!m_pScriptEngine->execute(initName, m_deviceName))
137- qWarning() << "MidiMapping: No" << initName << "function in script";
138- }
139- }
140- }
141+ m_scriptEngineInitializedMutex.lock();
142+ // Tell the script engine to run the init function in all loaded scripts
143+ emit(initMidiScripts(m_scriptFunctionPrefixes));
144+ // Wait until it's done
145+ m_scriptEngineInitializedCondition.wait(&m_scriptEngineInitializedMutex);
146+ m_scriptEngineInitializedMutex.unlock();
147+ }
148 }
149
150 void MidiMapping::shutdownScriptEngine() {
151 QMutexLocker Locker(&m_mappingLock);
152 if(m_pScriptEngine) {
153- // Call each script's shutdown function if it exists
154- QListIterator<QString> prefixIt(m_pScriptFunctionPrefixes);
155- while (prefixIt.hasNext()) {
156- QString shutName = prefixIt.next();
157- if (shutName!="") {
158- shutName.append(".shutdown");
159- qDebug() << "MidiMapping: Executing" << shutName;
160- if (!m_pScriptEngine->execute(shutName))
161- qWarning() << "MidiMapping: No" << shutName << "function in script";
162- }
163- }
164- qDebug() << "MidiMapping: Deleting MIDI script engine...";
165+ // Tell the script engine to do its shutdown sequence
166+ emit(shutdownMidiScriptEngine(m_scriptFunctionPrefixes));
167+ // ...and wait for it to finish
168+ m_pScriptEngine->wait();
169+
170 MidiScriptEngine *engine = m_pScriptEngine;
171 m_pScriptEngine = NULL;
172 delete engine;
173@@ -484,8 +482,8 @@
174 -------- ------------------------------------------------------ */
175 void MidiMapping::addScriptFile(QString filename, QString functionprefix) {
176 QMutexLocker Locker(&m_mappingLock);
177- m_pScriptFileNames.append(filename);
178- m_pScriptFunctionPrefixes.append(functionprefix);
179+ m_scriptFileNames.append(filename);
180+ m_scriptFunctionPrefixes.append(functionprefix);
181 }
182 #endif
183
184@@ -539,8 +537,8 @@
185 m_mappingLock.lock();
186
187 #ifdef __MIDISCRIPT__
188- m_pScriptFileNames.clear();
189- m_pScriptFunctionPrefixes.clear();
190+ m_scriptFileNames.clear();
191+ m_scriptFunctionPrefixes.clear();
192 #endif
193
194 // For each controller in the DOM
195@@ -753,15 +751,15 @@
196 controller.appendChild(scriptFiles);
197
198
199- for (int i = 0; i < m_pScriptFileNames.count(); i++) {
200-// qDebug() << "MidiMapping: writing script block for" << m_pScriptFileNames[i];
201- QString filename = m_pScriptFileNames[i];
202+ for (int i = 0; i < m_scriptFileNames.count(); i++) {
203+// qDebug() << "MidiMapping: writing script block for" << m_scriptFileNames[i];
204+ QString filename = m_scriptFileNames[i];
205
206
207 //Don't need to write anything for the required mapping file.
208 if (filename != REQUIRED_SCRIPT_FILE) {
209 qDebug() << "MidiMapping: writing script block for" << filename;
210- QString functionPrefix = m_pScriptFunctionPrefixes[i];
211+ QString functionPrefix = m_scriptFunctionPrefixes[i];
212 QDomElement scriptFile = doc.createElement("file");
213
214
215
216=== modified file 'mixxx/src/midi/midimapping.h'
217--- mixxx/src/midi/midimapping.h 2009-11-25 02:31:11 +0000
218+++ mixxx/src/midi/midimapping.h 2010-03-24 04:40:48 +0000
219@@ -111,6 +111,11 @@
220 void midiLearningStarted();
221 void midiLearningFinished(MidiMessage);
222 void midiLearningFinished();
223+ void callMidiScriptFunction(QString);
224+ void callMidiScriptFunction(QString, QString);
225+ void loadMidiScriptFiles(QList<QString>);
226+ void initMidiScripts(QList<QString>);
227+ void shutdownMidiScriptEngine(QList<QString>);
228
229 private:
230 int internalNumInputMidiMessages();
231@@ -142,8 +147,8 @@
232 /** Actually loads script code from the files in the list */
233 void loadScriptCode();
234
235- QList<QString> m_pScriptFileNames;
236- QList<QString> m_pScriptFunctionPrefixes;
237+ QList<QString> m_scriptFileNames;
238+ QList<QString> m_scriptFunctionPrefixes;
239 MidiScriptEngine *m_pScriptEngine;
240
241 QMutex m_scriptEngineInitializedMutex;
242
243=== modified file 'mixxx/src/midi/midimessage.cpp'
244--- mixxx/src/midi/midimessage.cpp 2009-09-01 15:33:21 +0000
245+++ mixxx/src/midi/midimessage.cpp 2010-03-24 04:40:48 +0000
246@@ -7,6 +7,7 @@
247 {
248 //Register this class with QT so we can use this bad boy in signals/slots.
249 qRegisterMetaType<MidiMessage>("MidiMessage");
250+ qRegisterMetaType<MidiStatusByte>("MidiStatusByte");
251
252 m_midiStatusByte = status;
253 m_midiNo = midino;
254
255=== modified file 'mixxx/src/midi/midiscriptengine.cpp'
256--- mixxx/src/midi/midiscriptengine.cpp 2010-02-22 23:13:52 +0000
257+++ mixxx/src/midi/midiscriptengine.cpp 2010-03-24 04:40:48 +0000
258@@ -20,7 +20,7 @@
259 #include "controlobjectthread.h"
260 #include "mididevice.h"
261 #include "midiscriptengine.h"
262-
263+// #include <QScriptSyntaxCheckResult>
264
265 #ifdef _MSC_VER
266 #include <float.h> // for _isnan() on VC++
267@@ -38,16 +38,6 @@
268
269 MidiScriptEngine::~MidiScriptEngine() {
270
271- // Stop processing the event loop and terminate the thread.
272- quit();
273-
274- // Clear the m_connectedControls hash so we stop responding
275- // to signals.
276- m_connectedControls.clear();
277-
278- // Wait for the thread to terminate
279- wait();
280-
281 // Delete the script engine, first clearing the pointer so that
282 // other threads will not get the dead pointer after we delete it.
283 if(m_pEngine != NULL) {
284@@ -56,6 +46,41 @@
285 engine->deleteLater();
286 }
287
288+}
289+
290+/* -------- ------------------------------------------------------
291+Purpose: Shuts down MIDI scripts in an orderly fashion
292+ (stops timers then executes shutdown functions)
293+Input: -
294+Output: -
295+-------- ------------------------------------------------------ */
296+void MidiScriptEngine::gracefulShutdown(QList<QString> scriptFunctionPrefixes) {
297+ qDebug() << "MidiScriptEngine shutting down...";
298+ m_scriptEngineLock.lock();
299+ // Clear the m_connectedControls hash so we stop responding
300+ // to signals.
301+ m_connectedControls.clear();
302+
303+ // Disconnect the function call signal
304+ disconnect(m_pMidiDevice, SIGNAL(callMidiScriptFunction(QString, char, char,
305+ char, MidiStatusByte, QString)),
306+ this, SLOT(execute(QString, char, char, char, MidiStatusByte, QString)));
307+
308+ // Stop all timers
309+ stopAllTimers();
310+
311+ // Call each script's shutdown function if it exists
312+ QListIterator<QString> prefixIt(scriptFunctionPrefixes);
313+ while (prefixIt.hasNext()) {
314+ QString shutName = prefixIt.next();
315+ if (shutName!="") {
316+ shutName.append(".shutdown");
317+ if (m_midiDebug) qDebug() << "MidiScriptEngine: Executing" << shutName;
318+ if (!internalExecute(shutName))
319+ qWarning() << "MidiScriptEngine: No" << shutName << "function in script";
320+ }
321+ }
322+
323 // Free all the control object threads
324 QList<ConfigKey> keys = m_controlCache.keys();
325 QList<ConfigKey>::iterator it = keys.begin();
326@@ -63,12 +88,14 @@
327 while(it != end) {
328 ConfigKey key = *it;
329 ControlObjectThread *cot = m_controlCache.take(key);
330- if(cot != NULL) {
331- delete cot;
332- }
333+ delete cot;
334 it++;
335 }
336
337+ m_scriptEngineLock.unlock();
338+
339+ // Stop processing the event loop and terminate the thread.
340+ quit();
341 }
342
343 bool MidiScriptEngine::isReady() {
344@@ -95,11 +122,73 @@
345 // 'engine'.
346 QScriptValue engineGlobalObject = m_pEngine->globalObject();
347 engineGlobalObject.setProperty("engine", m_pEngine->newQObject(this));
348+
349+ // Make the MidiDevice instance available to scripts as
350+ // 'midi'.
351 if (m_pMidiDevice)
352 engineGlobalObject.setProperty("midi", m_pEngine->newQObject(m_pMidiDevice));
353
354-}
355-
356+ // Allow the MidiDevice to signal script function calls
357+ connect(m_pMidiDevice, SIGNAL(callMidiScriptFunction(QString, char, char,
358+ char, MidiStatusByte, QString)),
359+ this, SLOT(execute(QString, char, char, char, MidiStatusByte, QString)));
360+}
361+
362+/* -------- ------------------------------------------------------
363+ Purpose: Load all script files given in the shared list
364+ Input: -
365+ Output: -
366+ -------- ------------------------------------------------------ */
367+void MidiScriptEngine::loadScriptFiles(QList<QString> scriptFileNames) {
368+
369+ // Set the Midi Debug flag
370+ if (m_pMidiDevice)
371+ m_midiDebug = m_pMidiDevice->midiDebugging();
372+
373+ qDebug() << "MidiScriptEngine: Loading & evaluating all MIDI script code";
374+
375+ ConfigObject<ConfigValue> *config = new ConfigObject<ConfigValue>(QDir::homePath().append("/").append(SETTINGS_PATH).append(SETTINGS_FILE));
376+
377+ QString scriptPath = config->getConfigPath().append("midi/");
378+ delete config;
379+
380+ QListIterator<QString> it(scriptFileNames);
381+ m_scriptEngineLock.lock();
382+ while (it.hasNext()) {
383+ QString curScriptFileName = it.next();
384+ safeEvaluate(scriptPath+curScriptFileName);
385+
386+ if(m_scriptErrors.contains(curScriptFileName)) {
387+ qDebug() << "Errors occured while loading " << curScriptFileName;
388+ }
389+ }
390+
391+ m_scriptEngineLock.unlock();
392+ emit(initialized());
393+}
394+
395+/* -------- ------------------------------------------------------
396+ Purpose: Run the initialization function for each loaded script
397+ if it exists
398+ Input: -
399+ Output: -
400+ -------- ------------------------------------------------------ */
401+void MidiScriptEngine::initializeScripts(QList<QString> scriptFunctionPrefixes) {
402+ m_scriptEngineLock.lock();
403+
404+ QListIterator<QString> prefixIt(scriptFunctionPrefixes);
405+ while (prefixIt.hasNext()) {
406+ QString initName = prefixIt.next();
407+ if (initName!="") {
408+ initName.append(".init");
409+ if (m_midiDebug) qDebug() << "MidiScriptEngine: Executing" << initName;
410+ if (!safeExecute(initName, m_pMidiDevice->getName()))
411+ qWarning() << "MidiScriptEngine: No" << initName << "function in script";
412+ }
413+ }
414+ m_scriptEngineLock.unlock();
415+ emit(initialized());
416+}
417
418 /* -------- ------------------------------------------------------
419 Purpose: Create the MidiScriptEngine object (so it is owned in this
420@@ -111,7 +200,8 @@
421 unsigned static id = 0; //the id of this thread, for debugging purposes //XXX copypasta (should factor this out somehow), -kousu 2/2009
422 QThread::currentThread()->setObjectName(QString("MidiScriptEngine %1").arg(++id));
423
424- //qDebug() << QString("----------------------------------MidiScriptEngine: Run Thread ID=%1").arg(QThread::currentThreadId(),0,16);
425+ // Prevent the script engine from strangling other parts of Mixxx
426+ QThread::currentThread()->setPriority(QThread::LowPriority);
427
428 m_scriptEngineLock.lock();
429 initializeScriptEngine();
430@@ -143,6 +233,7 @@
431 bool MidiScriptEngine::execute(QString function) {
432 m_scriptEngineLock.lock();
433 bool ret = safeExecute(function);
434+ if (!ret) qDebug() << "MidiScriptEngine: Invalid script function" << function;
435 m_scriptEngineLock.unlock();
436 return ret;
437 }
438@@ -155,6 +246,7 @@
439 bool MidiScriptEngine::execute(QString function, QString data) {
440 m_scriptEngineLock.lock();
441 bool ret = safeExecute(function, data);
442+ if (!ret) qDebug() << "MidiScriptEngine: Invalid script function" << function;
443 m_scriptEngineLock.unlock();
444 return ret;
445 }
446@@ -170,6 +262,7 @@
447 QString group) {
448 m_scriptEngineLock.lock();
449 bool ret = safeExecute(function, channel, control, value, status, group);
450+ if (!ret) qDebug() << "MidiScriptEngine: Invalid script function" << function;
451 m_scriptEngineLock.unlock();
452 return ret;
453 }
454@@ -182,22 +275,79 @@
455 bool MidiScriptEngine::safeExecute(QString function) {
456 //qDebug() << QString("MidiScriptEngine: Exec1 Thread ID=%1").arg(QThread::currentThreadId(),0,16);
457
458- if(m_pEngine == NULL) {
459- return false;
460- }
461-
462- if (!m_pEngine->canEvaluate(function)) {
463- qCritical() << "MidiScriptEngine: ?Syntax error in function " << function;
464- return false;
465- }
466+ if(m_pEngine == NULL)
467+ return false;
468
469 QScriptValue scriptFunction = m_pEngine->evaluate(function);
470
471- if (checkException()) return false;
472- if (!scriptFunction.isFunction()) return false;
473-
474- scriptFunction.call(QScriptValue());
475- if (checkException()) return false;
476+ if (checkException())
477+ return false;
478+
479+ if (!scriptFunction.isFunction())
480+ return false;
481+
482+ scriptFunction.call(QScriptValue());
483+ if (checkException())
484+ return false;
485+
486+ return true;
487+}
488+
489+
490+/* -------- ------------------------------------------------------
491+Purpose: Evaluate & run script code
492+Input: Code string
493+Output: false if an exception
494+-------- ------------------------------------------------------ */
495+bool MidiScriptEngine::internalExecute(QString scriptCode) {
496+ // A special version of safeExecute since we're evaluating strings, not actual functions
497+ // (execute() would print an error that it's not a function every time a timer fires.)
498+ if(m_pEngine == NULL)
499+ return false;
500+
501+ // Check syntax
502+ QScriptSyntaxCheckResult result = m_pEngine->checkSyntax(scriptCode);
503+ QString error="";
504+ switch (result.state()) {
505+ case (QScriptSyntaxCheckResult::Valid): break;
506+ case (QScriptSyntaxCheckResult::Intermediate):
507+ error = "Incomplete code";
508+ break;
509+ case (QScriptSyntaxCheckResult::Error):
510+ error = "Syntax error";
511+ break;
512+ }
513+ if (error!="") {
514+ error = QString("%1: %2 at line %3, column %4 of script code:\n%5\n")
515+ .arg(error)
516+ .arg(result.errorMessage())
517+ .arg(result.errorLineNumber())
518+ .arg(result.errorColumnNumber())
519+ .arg(scriptCode);
520+
521+ if (m_midiDebug) qCritical() << "MidiScriptEngine:" << error;
522+ else {
523+ qDebug() << "MidiScriptEngine:" << error;
524+ qWarning() << "There was an error in a MIDI script."
525+ "\nA control you just used is not working properly and you may experience erratic behavior."
526+ "\nCheck the console or mixxx.log file for details.";
527+ }
528+ return false;
529+ }
530+
531+ QScriptValue scriptFunction = m_pEngine->evaluate(scriptCode);
532+
533+ if (checkException())
534+ return false;
535+
536+ // If it's not a function, we're done.
537+ if (!scriptFunction.isFunction())
538+ return true;
539+
540+ // If it does happen to be a function, call it.
541+ scriptFunction.call(QScriptValue());
542+ if (checkException())
543+ return false;
544
545 return true;
546 }
547@@ -214,11 +364,6 @@
548 return false;
549 }
550
551- if (!m_pEngine->canEvaluate(function)) {
552- qCritical() << "MidiScriptEngine: ?Syntax error in function " << function;
553- return false;
554- }
555-
556 QScriptValue scriptFunction = m_pEngine->evaluate(function);
557
558 if (checkException())
559@@ -250,11 +395,6 @@
560 return false;
561 }
562
563- if (!m_pEngine->canEvaluate(function)) {
564- qCritical() << "MidiScriptEngine: ?Syntax error in function " << function;
565- return false;
566- }
567-
568 QScriptValue scriptFunction = m_pEngine->evaluate(function);
569
570 if (checkException())
571@@ -296,15 +436,22 @@
572 error << filename << errorMessage << QString(line);
573 m_scriptErrors.insert(filename, error);
574
575- qCritical() << "MidiScriptEngine uncaught exception : "
576- << errorMessage
577- << "at " << filename << " line"
578- << line;
579- // qCritical() << "MidiScriptEngine: uncaught exception"
580- // << m_pEngine->uncaughtException().toString()
581- // << "\nBacktrace:\n"
582- // << m_pEngine->uncaughtExceptionBacktrace();
583-
584+ if (m_midiDebug)
585+ qCritical() << "MidiScriptEngine: uncaught exception:"
586+ << errorMessage
587+ << "in" << filename << "at line"
588+ << line
589+ << "\nBacktrace:\n"
590+ << backtrace;
591+ else {
592+ qDebug() << "MidiScriptEngine WARNING: uncaught exception:"
593+ << errorMessage
594+ << "in" << filename << "at line"
595+ << line;
596+ qWarning() << "There was a problem with a MIDI script."
597+ "\nA control you just used is not working properly and you may experience erratic behavior."
598+ "\nCheck the console or mixxx.log file for details.";
599+ }
600 return true;
601 }
602 return false;
603@@ -331,7 +478,8 @@
604
605 // qDebug() << "MidiScriptEngine: m_scriptCode=" << m_scriptCode;
606
607- qDebug() << "MidiScriptEngine:" << codeLines.count() << "lines of code being searched for functions";
608+ if (m_midiDebug)
609+ qDebug() << "MidiScriptEngine:" << codeLines.count() << "lines of code being searched for functions";
610
611 // grep 'function' midi/midi-mappings-scripts.js|grep -i '(msg)'|sed -e 's/function \(.*\)(msg).*/\1/i' -e 's/[= ]//g'
612 QRegExp rx("*.*function*(*)*"); // Find all lines with function names in them
613@@ -345,16 +493,13 @@
614
615 if (line.indexOf('#') != 0 && line.indexOf("//") != 0) { // ignore commented out lines
616 QStringList field = line.split(" ");
617- if (m_pMidiDevice && m_pMidiDevice->midiDebugging())
618- qDebug() << "MidiScriptEngine: Found function:" << field[0]
619- << "at line" << position;
620-// functionList.append(field[0]);
621+ if (m_midiDebug) qDebug() << "MidiScriptEngine: Found function:" << field[0]
622+ << "at line" << position;
623 m_scriptFunctions.append(field[0]);
624 }
625 position = codeLines.indexOf(rx);
626 }
627
628-// m_scriptFunctions = functionList;
629 }
630
631 ControlObjectThread* MidiScriptEngine::getControlObjectThread(QString group, QString name) {
632@@ -478,13 +623,8 @@
633 return false;
634 }
635
636- if (!m_pEngine->canEvaluate(function)) {
637- qCritical() << "MidiScriptEngine: ?Syntax error in function " << function;
638- return false;
639- }
640 QScriptValue slot = m_pEngine->evaluate(function);
641
642-
643 if(!checkException() && slot.isFunction()) {
644 if(disconnect) {
645 // qDebug() << "MidiScriptEngine::connectControl disconnected " << group << name << " from " << function;
646@@ -560,6 +700,8 @@
647 return false;
648 }
649
650+ qDebug() << "MidiScriptEngine: Loading" << filename;
651+
652 // Read in the script file
653 QFile input(filename);
654 if (!input.open(QIODevice::ReadOnly)) {
655@@ -574,8 +716,33 @@
656 scriptCode.append('\n');
657 input.close();
658
659- if (!m_pEngine->canEvaluate(scriptCode)) {
660- qCritical() << "MidiScriptEngine: ?Syntax error in script file:" << filename;
661+ // Check syntax
662+ QScriptSyntaxCheckResult result = m_pEngine->checkSyntax(scriptCode);
663+ QString error="";
664+ switch (result.state()) {
665+ case (QScriptSyntaxCheckResult::Valid): break;
666+ case (QScriptSyntaxCheckResult::Intermediate):
667+ error = "Incomplete code";
668+ break;
669+ case (QScriptSyntaxCheckResult::Error):
670+ error = "Syntax error";
671+ break;
672+ }
673+ if (error!="") {
674+ error = QString("%1 at line %2, column %3 in file %4: %5")
675+ .arg(error)
676+ .arg(result.errorLineNumber())
677+ .arg(result.errorColumnNumber())
678+ .arg(filename)
679+ .arg(result.errorMessage());
680+
681+ if (m_midiDebug) qCritical() << "MidiScriptEngine:" << error;
682+ else {
683+ qDebug() << "MidiScriptEngine:" << error;
684+ qWarning() << "There was an error in the MIDI script file" << filename
685+ << "\nThe functionality provided by this script file will be disabled."
686+ "\nCheck the console or mixxx.log file for details.";
687+ }
688 return false;
689 }
690
691@@ -587,7 +754,6 @@
692 return false;
693
694 // Add the code we evaluated to our index
695- qDebug() << "MidiScriptEngine: Loading" << filename;
696 generateScriptFunctions(scriptCode);
697
698 return true;
699@@ -616,3 +782,99 @@
700 }
701
702
703+/* -------- ------------------------------------------------------
704+ Purpose: Creates & starts a timer that runs some script code
705+ on timeout
706+ Input: Number of milliseconds, script function to call,
707+ whether it should fire just once
708+ Output: The timer's ID, 0 if starting it failed
709+ -------- ------------------------------------------------------ */
710+int MidiScriptEngine::beginTimer(int interval, QString scriptCode, bool oneShot) {
711+ // When this function runs, assert that somebody is holding the script
712+ // engine lock.
713+ bool lock = m_scriptEngineLock.tryLock();
714+ Q_ASSERT(!lock);
715+ if(lock) {
716+ m_scriptEngineLock.unlock();
717+ }
718+
719+ if (interval<20) {
720+ qDebug() << "Timer request for" << interval << "ms is too short. Setting to the minimum of 20ms.";
721+ interval=20;
722+ }
723+ // This makes use of every QObject's internal timer mechanism. Nice, clean, and simple.
724+ // See http://doc.trolltech.com/4.6/qobject.html#startTimer for details
725+ int timerId = startTimer(interval);
726+ QPair<QString, bool> timerTarget;
727+ timerTarget.first = scriptCode;
728+ timerTarget.second = oneShot;
729+ m_timers[timerId]=timerTarget;
730+ if (timerId==0) qDebug() << "MIDI Script timer could not be created";
731+ else if (m_midiDebug) {
732+ if (oneShot) qDebug() << "Starting one-shot timer:" << timerId;
733+ else qDebug() << "Starting timer:" << timerId;
734+ }
735+ return timerId;
736+}
737+
738+/* -------- ------------------------------------------------------
739+ Purpose: Stops & removes a timer
740+ Input: ID of timer to stop
741+ Output: -
742+ -------- ------------------------------------------------------ */
743+void MidiScriptEngine::stopTimer(int timerId) {
744+ // When this function runs, assert that somebody is holding the script
745+ // engine lock.
746+ bool lock = m_scriptEngineLock.tryLock();
747+ Q_ASSERT(!lock);
748+ if(lock) m_scriptEngineLock.unlock();
749+
750+ if (!m_timers.contains(timerId)) {
751+ qDebug() << "Killing timer" << timerId << ": That timer does not exist!";
752+ return;
753+ }
754+ if (m_midiDebug) qDebug() << "Killing timer:" << timerId;
755+
756+ killTimer(timerId);
757+ m_timers.remove(timerId);
758+}
759+
760+/* -------- ------------------------------------------------------
761+ Purpose: Stops & removes all timers (for shutdown)
762+ Input: -
763+ Output: -
764+ -------- ------------------------------------------------------ */
765+void MidiScriptEngine::stopAllTimers() {
766+ // When this function runs, assert that somebody is holding the script
767+ // engine lock.
768+ bool lock = m_scriptEngineLock.tryLock();
769+ Q_ASSERT(!lock);
770+ if(lock) m_scriptEngineLock.unlock();
771+
772+ QMutableHashIterator<int, QPair<QString, bool> > i(m_timers);
773+ while (i.hasNext()) {
774+ i.next();
775+ stopTimer(i.key());
776+ }
777+}
778+
779+/* -------- ------------------------------------------------------
780+ Purpose: Runs the appropriate script code on timer events
781+ Input: -
782+ Output: -
783+ -------- ------------------------------------------------------ */
784+void MidiScriptEngine::timerEvent(QTimerEvent *event) {
785+ int timerId = event->timerId();
786+
787+ m_scriptEngineLock.lock();
788+ if (!m_timers.contains(timerId)) {
789+ qDebug() << "Timer" << timerId << "fired but there's no function mapped to it!";
790+ return;
791+ }
792+
793+ QPair<QString, bool> timerTarget = m_timers[timerId];
794+ if (timerTarget.second) stopTimer(timerId);
795+
796+ internalExecute(timerTarget.first);
797+ m_scriptEngineLock.unlock();
798+}
799
800=== modified file 'mixxx/src/midi/midiscriptengine.h'
801--- mixxx/src/midi/midiscriptengine.h 2009-10-27 07:31:35 +0000
802+++ mixxx/src/midi/midiscriptengine.h 2010-03-24 04:40:48 +0000
803@@ -38,16 +38,6 @@
804 bool hasErrors(QString filename);
805 const QStringList getErrors(QString filename);
806
807- // Evaluate a script file
808- bool evaluate(QString filepath);
809- // Execute a particular function
810- bool execute(QString function);
811- // Execute a particular function with a data string (e.g. a device ID)
812- bool execute(QString function, QString data);
813- // Execute a particular function with all the data
814- bool execute(QString function, char channel,
815- char control, char value, MidiStatusByte status, QString group);
816-
817 // Lookup registered script functions
818 QStringList getScriptFunctions();
819
820@@ -58,19 +48,35 @@
821 Q_INVOKABLE bool connectControl(QString group, QString name,
822 QString function, bool disconnect = false);
823 Q_INVOKABLE void trigger(QString group, QString name);
824+ Q_INVOKABLE int beginTimer(int interval, QString scriptCode, bool oneShot = false);
825+ Q_INVOKABLE void stopTimer(int timerId);
826
827 public slots:
828 void slotValueChanged(double value);
829+ // Evaluate a script file
830+ bool evaluate(QString filepath);
831+ // Execute a particular function
832+ bool execute(QString function);
833+ // Execute a particular function with a data string (e.g. a device ID)
834+ bool execute(QString function, QString data);
835+ // Execute a particular function with all the data
836+ bool execute(QString function, char channel,
837+ char control, char value, MidiStatusByte status, QString group);
838+ void loadScriptFiles(QList<QString> scriptFileNames);
839+ void initializeScripts(QList<QString> scriptFunctionPrefixes);
840+ void gracefulShutdown(QList<QString> scriptFunctionPrefixes);
841
842 signals:
843 void initialized();
844
845 protected:
846 void run();
847+ void timerEvent(QTimerEvent *event);
848
849 private:
850 // Only call these with the scriptEngineLock
851 bool safeEvaluate(QString filepath);
852+ bool internalExecute(QString scriptCode);
853 bool safeExecute(QString function);
854 bool safeExecute(QString function, QString data);
855 bool safeExecute(QString function, char channel,
856@@ -79,18 +85,20 @@
857
858 void generateScriptFunctions(QString code);
859 bool checkException();
860+ void stopAllTimers();
861
862 ControlObjectThread* getControlObjectThread(QString group, QString name);
863
864
865 MidiDevice* m_pMidiDevice;
866+ bool m_midiDebug;
867 QHash<ConfigKey, QString> m_connectedControls;
868 QScriptEngine *m_pEngine;
869 QStringList m_scriptFunctions;
870 QMap<QString,QStringList> m_scriptErrors;
871 QMutex m_scriptEngineLock;
872 QHash<ConfigKey, ControlObjectThread*> m_controlCache;
873+ QHash<int, QPair<QString, bool> > m_timers;
874 };
875
876 #endif
877-
878
879=== modified file 'mixxx/src/mixxx.cpp'
880--- mixxx/src/mixxx.cpp 2010-02-23 12:10:51 +0000
881+++ mixxx/src/mixxx.cpp 2010-03-24 04:40:48 +0000
882@@ -476,11 +476,6 @@
883
884 qDebug() << "delete config, " << qTime.elapsed();
885 delete config;
886-
887-// Why is this here? The (MSVC 2008) linker even complains about it.
888-//#ifdef __WINDOWS__
889-// _exit(0);
890-//#endif
891 }
892
893 int MixxxApp::noSoundDlg(void)