Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OSC Messaging to avoid calling QMessageBox from the core #2252

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ ADD_SUBDIRECTORY(plugins)
ADD_SUBDIRECTORY(tests)
ADD_SUBDIRECTORY(data)
ADD_SUBDIRECTORY(doc)
ADD_SUBDIRECTORY(lib)

# post-install tasks
ADD_SUBDIRECTORY(cmake/postinstall)
Expand Down
24 changes: 10 additions & 14 deletions include/Engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <QtCore/QString>
#include <QtCore/QObject>

#include "Messenger.h"

#include "export.h"

Expand All @@ -42,9 +43,8 @@ class Song;
class Ladspa2LMMS;


class EXPORT Engine : public QObject
class EXPORT Engine
{
Q_OBJECT
public:
static void init( bool renderOnly );
static void destroy();
Expand Down Expand Up @@ -85,23 +85,16 @@ class EXPORT Engine : public QObject
return s_dummyTC;
}

static float framesPerTick()
static Messenger * messenger()
{
return s_framesPerTick;
return &s_messenger;
}
static void updateFramesPerTick();

static inline Engine * inst()
static float framesPerTick()
{
if( s_instanceOfMe == NULL )
{
s_instanceOfMe = new Engine();
}
return s_instanceOfMe;
return s_framesPerTick;
}

signals:
void initProgress(const QString &msg);
static void updateFramesPerTick();


private:
Expand All @@ -127,6 +120,9 @@ class EXPORT Engine : public QObject

static Ladspa2LMMS * s_ladspaManager;

// object to send/receive Open Sound Control messages
static Messenger s_messenger;

// even though most methods are static, an instance is needed for Qt slots/signals
static Engine * s_instanceOfMe;

Expand Down
46 changes: 43 additions & 3 deletions include/GuiApplication.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@
#ifndef GUIAPPLICATION_H
#define GUIAPPLICATION_H

#include <QtCore/QObject>
#include <QObject>
#include <QString>

#include "export.h"
#include "OscMsgListener.h"

class QLabel;
class QSplashScreen;

class AutomationEditorWindow;
class BBEditor;
Expand All @@ -40,11 +43,12 @@ class PianoRollWindow;
class ProjectNotes;
class SongEditorWindow;

class EXPORT GuiApplication : public QObject
class EXPORT GuiApplication : public QObject, public OscMsgListener
{
Q_OBJECT;
public:
explicit GuiApplication();
explicit GuiApplication(const QString &fileToLoad, const QString &fileToImport,
bool fullscreen, bool exitAfterImport);
~GuiApplication();

static GuiApplication* instance();
Expand All @@ -57,12 +61,41 @@ class EXPORT GuiApplication : public QObject
ProjectNotes* getProjectNotes() { return m_projectNotes; }
AutomationEditorWindow* automationEditor() { return m_automationEditor; }
ControllerRackView* getControllerRackView() { return m_controllerRackView; }

// override of callback defined by OscMsgListener
void processMessage(const QByteArray &msg);

public slots:
void displayInitProgress(const QString &msg);

private slots:
void childDestroyed(QObject *obj);
// this slot should always be connected via Qt::QueuedConnection so that it operates in the gui thread
void processOscMsgInGuiThread(QByteArray msg);
// staged initialization (in order to avoid blocking the gui thread)
void initEngine();
void initMainWindow();
void initSongEditorWindow();
void initFxMixerView();
void initControllerRackView();
void initProjectNotes();
void initBbEditor();
void initPianoRoll();
void initAutomationEditor();
void handleCtorOptions();
signals:
void postInitEngine();
void postInitMainWindow();
void postInitSongEditorWindow();
void postInitFxMixerView();
void postInitControllerRackView();
void postInitProjectNotes();
void postInitBbEditor();
void postInitPianoRoll();
void postInitAutomationEditor();
// sent whenever we receive an Open Sound Control message
// (presumably from the core or another section of the UI)
void receivedOscMessage(QByteArray msg);

private:
static GuiApplication* s_instance;
Expand All @@ -76,6 +109,13 @@ private slots:
ProjectNotes* m_projectNotes;
ControllerRackView* m_controllerRackView;
QLabel* m_loadingProgressLabel;
QSplashScreen* m_splashScreen;

// initialization arguments:
QString m_fileToLoad;
QString m_fileToImport;
bool m_fullscreen;
bool m_exitAfterImport;
};

#define gui GuiApplication::instance()
Expand Down
9 changes: 9 additions & 0 deletions include/MainWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ public slots:
void undo();
void redo();

private slots:
void exportProject();
void exportProjectTracks();
void exportProjectMidi();


protected:
virtual void closeEvent( QCloseEvent * _ce );
Expand All @@ -150,6 +155,10 @@ public slots:
void toggleWindow( QWidget *window, bool forceShow = false );
void refocus();

// returns true if the project can be exported,
// shows a warning message & returns false if the project is empty
bool checkProjectExportable();


QMdiArea * m_workspace;

Expand Down
93 changes: 93 additions & 0 deletions include/Messenger.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Messenger: class to abstract the routing of Open Sound Control messages from the core
* to another area of the core and/or the gui
*
* (c) 2015 Colin Wallace (https://github.com/Wallacoloo)
*
* This file is part of LMMS - http://lmms.io
*
* This file is released under the MIT License.
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

#ifndef MESSENGER_H
#define MESSENGER_H

#include <cstring>

#include <QReadWriteLock>
#include <QSet>

class QString;

class OscMsgListener;


class Messenger
{
public:
// const strings to specify the Open Sound Control endpoint locations,
// e.g. "/mixer/ch/n/volume"
class Endpoints
{
public:
// Endpoints to send general and already-translated warnings / error messages
static const char *Warning;
static const char *Error;

static const char *WaveTableInit;
static const char *MixerDevInit;
static const char *MixerProcessingStart;
};

// Send message to indicate that the following components have completed their initialization
void broadcastWaveTableInit();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method and some of the ones following it look quite specialized and explicitly tailored to LMMS. I assume that they are just workarounds to make the current code structure work and that the goal later on is to refactor to get rid of them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelgregorius I intended to keep those methods going forward. The most important thing is to keep string constants related to OSC (i.e. the endpoint names) from being defined in more than one file, as mismatched strings cannot be detected as a compile-time error. Hence the Endpoints class.

Secondly, any library-specific OSC interactions should be limited to as few files as possible - I don't want the engine to have to encode its own OSC messages, for example. That essentially leaves two options:

  1. Declare the Warning and Error broadcast functions and then a single InitComponent(QString endpoint) function.
  2. Declare the Warning and Error broadcast functions and then another InitComponentX() function for each possible component.

I chose no.2 so that each endpoint would correspond to exactly one broadcast function. One could make an argument for no.1 on the basis that it further decouples this file from the lmms internals. If so, that also requires that the string constants be declared in each component's header rather than Messenger.h, otherwise you're still making assumptions about lmms internals. My argument for no.2 is that this gives you all the documentation for the communications interface in one single location, so you don't have to go hunting down for a list of endpoints that's spread out through the whole code base when it comes to interfacing with lmms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wallacoloo I definitively agree on keeping all the string constants in one file and not have them scattered as hard coded strings in several files. If possible these strings should be implementation details that are not visible to the outside.

Concerning my remarks about methods like broadcastWaveTableInit: I was wondering why any component should need to know that the wavetables have finished being initialized. I would assume that things like (global) wavetables should be initialized before the components are wired together in the first place. Or am I missing some important detail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelgregorius Oh, thanks for clarifying your remark. The only component that cares about the wave tables being initialized is the gui's splash screen. It uses that info to show initialization progress messages.

// Mixer has opened its audio devices
void broadcastMixerDevInit();
// Indicates that mixer has started its processing thread(s).
void broadcastMixerProcessing();

// whenever the core encounters a warning, it can broadcast it to listeners
// rather than explicitly pop a Qt Dialog / log it, etc.
void broadcastWarning(const QString &brief, const QString &warning);

// broadcast an error message. The fact that it's an error does *not* imply
// that the core/gui should exit.
// @brief is one-line summary of the error,
// @msg is the full message
void broadcastError(const QString &brief, const QString &msg);

// add a function to listen for OSC messages directed to a gui
// NOTE: This handler should have hard timing guarantees
// preferrably, it will just enqueue the message and then process it later *in a separate thread*
void addGuiOscListener(OscMsgListener *listener);
private:
// dispatch an OSC formatted message to ALL attached listeners
// if length is 0, an error will be logged and no action taken
// Therefore this can be safely used like so:
// char buffer[512];
// broadcast(buffer, rtosc_message(buffer, 512, "/test", "s", "Test broadcast"))
void broadcast(const char *buffer, std::size_t length);

// list of handlers that are called whenever we wish to broadcast a message to all GUIs
QSet<OscMsgListener*> m_guiListeners;
QReadWriteLock m_guiListenersLock;
};

#endif
54 changes: 54 additions & 0 deletions include/OscMsgListener.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* OscMsgListener: virtual base class to listen for Open Sound Control messages sent from core/gui
* and handle them appropriately.
* If you have a class that wants to listen in on OSC messages, just derive it from this,
* implement the appropriate callbacks & register it with some OSC broadcaster (e.g. Messenger)
*
* (c) 2015 Colin Wallace (https://github.com/Wallacoloo)
*
* This file is part of LMMS - http://lmms.io
*
* This file is released under the MIT License.
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

#include <QByteArray>
#include <QMutex>
#include <QQueue>
#include <QWaitCondition>

class OscMsgListener
{
public:
virtual ~OscMsgListener();
// call to asyncronously push a message into the received queue
void queue(const char *msg, std::size_t length);
// Continually wait for new messages and process them as they arrive.
void listenerLoop();
// spawn a new thread and have it call listenerLoop()
void listenInNewThread();
protected:
// called whenever a message is received
virtual void processMessage(const QByteArray &msg) = 0;
private:
// queue of unprocessed received messages
QQueue<QByteArray> m_rxQueue;
QMutex m_rxMutex;
QWaitCondition m_rxConditionVar;
};
7 changes: 4 additions & 3 deletions include/Song.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ class EXPORT Song : public TrackContainer
return m_timeSigModel;
}

void exportProject( bool multiExport = false );
void exportProjectTracks();
void exportProjectMidi();


public slots:
void playSong();
Expand All @@ -264,9 +268,6 @@ public slots:
void stop();

void importProject();
void exportProject( bool multiExport = false );
void exportProjectTracks();
void exportProjectMidi();

void startExport();
void stopExport();
Expand Down
4 changes: 4 additions & 0 deletions lib/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
CMAKE_MINIMUM_REQUIRED(VERSION 2.8)

# Build RTOSC from its CMakeFiles.txt
ADD_SUBDIRECTORY(rtosc)
30 changes: 30 additions & 0 deletions lib/rtosc/.travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
language: cpp

compiler:
- gcc
- clang

before_install:
- if [[ $NEW_GCC == true ]]; then sudo apt-get --purge remove gcc; fi;
- if [[ $NEW_GCC == true ]]; then sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test; fi;
- if [[ $NEW_GCC == true ]]; then sudo apt-get -qq update; fi;
- if [[ $NEW_GCC == true ]]; then sudo apt-get -qq install g++-4.8; fi;
- if [[ $NEW_GCC == true ]]; then sudo apt-get -qq install gcc-4.8; fi;
- if [[ $NEW_GCC == true ]]; then sudo update-alternatives --install /usr/bin/g++ g++ /usr/bin/g++-4.8 90; fi;
- if [[ $NEW_GCC == true ]]; then sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-4.8 90; fi;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file used? I am assuming not in this instance and is included for completeness.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. The .travis file is not used. I included the entire repository as a dependency. Could maybe reduce that to just src/ and include/ folders if desirable.

- sudo apt-get install liblo-dev

script:
- mkdir build && cd build
- cmake ..
- make
- ctest --output-on-failure

env:
- NEW_GCC=true
- NEW_GCC=false

matrix:
exclude:
- compiler: clang
env: NEW_GCC=true
Loading