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

make it compile and work under Linux #151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dehnhardt
Copy link
Contributor

I had a hard time getting ctrlr to work - at least partly - under Ubuntu 20.04.
With this changes

  • compilation is successfull
  • I can run ctrl - but only when compiled in debug mode. In release mode I get an exception somewhere in the early startup phase.
  • MIDI ports can be opened
  • Receiving of MIDI is successfull
  • Sending does not work
    I don't think this PR can be applied at all, but it might give you some hints where changes are required for Linux.

.vscode/**
Source/Core/stdafx_luabind.h.gch
Source/Core/stdafx.h.gch
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be ok to ignore generated files

Choose a reason for hiding this comment

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

This is only Windows related and should be removed from the patch

@@ -62,7 +62,7 @@ ifeq ($(CONFIG),Debug)

JUCE_CFLAGS += $(JUCE_CPPFLAGS) $(TARGET_ARCH) -fPIC -g -ggdb -O0 -w $(CFLAGS)
JUCE_CXXFLAGS += $(JUCE_CFLAGS) -std=gnu++14 $(CXXFLAGS)
JUCE_LDFLAGS += $(TARGET_ARCH) -L$(JUCE_BINDIR) -L$(JUCE_LIBDIR) $(shell pkg-config --libs alsa freetype2 libcurl) -fvisibility=hidden -lrt -ldl -lpthread -lz -ludev -lbfd $(LDFLAGS)
JUCE_LDFLAGS += $(TARGET_ARCH) -L$(JUCE_BINDIR) -L$(JUCE_LIBDIR) $(shell pkg-config --libs alsa x11 freetype2 libcurl) -fvisibility=hidden -lrt -ldl -lpthread -lz -ludev -lbfd $(LDFLAGS)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

x11 is still needed

@@ -348,21 +347,9 @@ $(JUCE_OUTDIR)/$(JUCE_TARGET_VST) : $(OBJECTS_VST) $(RESOURCES) $(JUCE_OUTDIR)/$
-$(V_AT)mkdir -p $(JUCE_VSTDESTDIR)
-$(V_AT)cp -R $(JUCE_COPYCMD_VST)

$(JUCE_OUTDIR)/$(JUCE_TARGET_VST3) : $(OBJECTS_VST3) $(RESOURCES) $(JUCE_OUTDIR)/$(JUCE_TARGET_SHARED_CODE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these parts in the Makefile gives errors, Is VST3 Relevant for Linux at all?

Choose a reason for hiding this comment

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

yes

But I'd think this is a bug in Projucer

Copy link

Choose a reason for hiding this comment

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

these parts in the Makefile gives errors, Is VST3 Relevant for Linux at all?

Why would VST3 not be "relevant" for Linux?

@@ -5,8 +5,8 @@ echo "CTRLR[linux]: Building for $HOSTTYPE, JOBS $MAXJOBS"
if [ "$1" == "-f" ]; then
echo "CTRLR[linux]: Compile PCH"
echo "stadfx.h"
g++ -w -std=c++14 -D "LINUX=1" -D "NDEBUG=1" \
-D "JUCE_FORCE_DEBUG=0" -D "CTRLR_NIGHTLY=1" \
g++ -w -std=c++14 -D "LINUX=1" -D "NDEBUG=0" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

application does nut run in release mode.

Choose a reason for hiding this comment

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

Debug settings should not be changed in this patch.

@@ -6,7 +6,7 @@
#include "CtrlrProcessor.h"
#include "CtrlrLog.h"

CtrlrMIDIDevice::CtrlrMIDIDevice(CtrlrMIDIDeviceManager &_owner, const int idx, const String name, const bool type)
CtrlrMIDIDevice::CtrlrMIDIDevice(CtrlrMIDIDeviceManager &_owner, const int idx, const String name, const String identifier, const bool type)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The identifier is needed to open the port

@@ -272,32 +272,32 @@ const StringArray CtrlrMIDIDeviceManager::getManagedDevices(const CtrlrMIDIDevic

void CtrlrMIDIDeviceManager::refreshDevices()
{
// inDevs.clear();
// outDevs.clear();
inDevs.clear();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not clearing this way?

Copy link

@keinstein keinstein left a comment

Choose a reason for hiding this comment

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

I think you should split the patch into 3 parts: MIDI, Linux and Debug.

.vscode/**
Source/Core/stdafx_luabind.h.gch
Source/Core/stdafx.h.gch

Choose a reason for hiding this comment

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

This is only Windows related and should be removed from the patch

@@ -5,8 +5,8 @@ echo "CTRLR[linux]: Building for $HOSTTYPE, JOBS $MAXJOBS"
if [ "$1" == "-f" ]; then
echo "CTRLR[linux]: Compile PCH"
echo "stadfx.h"
g++ -w -std=c++14 -D "LINUX=1" -D "NDEBUG=1" \
-D "JUCE_FORCE_DEBUG=0" -D "CTRLR_NIGHTLY=1" \
g++ -w -std=c++14 -D "LINUX=1" -D "NDEBUG=0" \

Choose a reason for hiding this comment

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

Debug settings should not be changed in this patch.

@@ -46,7 +46,7 @@ fi
echo "CTRLR[linux]: Build now"
echo
#make -j$JOBS CONFIG=Release ARCH=$HOSTTYPE BINDIR=$BUILDDIR LIBDIR=$BUILDDIR OBJDIR=$BUILDDIR
make CONFIG=Release -j$MAXJOBS
make CONFIG=Debug -j$MAXJOBS
if [ $? -ne 0 ]; then
echo -e "CTRLR[linux]: build failed\n"
exit 1

Choose a reason for hiding this comment

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

A patch that fixes a particular issue should not contain anything else, especitally debug or non-debug.

static const char *ctrlrRevision = "5.6.0";
static const char *ctrlrRevisionDate = "Wed, Jan 27, 2021 3:40:04 PM";

#endif

Choose a reason for hiding this comment

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

Is the version change necessary? Are you shure, that you want to step by two ?

@@ -348,21 +347,9 @@ $(JUCE_OUTDIR)/$(JUCE_TARGET_VST) : $(OBJECTS_VST) $(RESOURCES) $(JUCE_OUTDIR)/$
-$(V_AT)mkdir -p $(JUCE_VSTDESTDIR)
-$(V_AT)cp -R $(JUCE_COPYCMD_VST)

$(JUCE_OUTDIR)/$(JUCE_TARGET_VST3) : $(OBJECTS_VST3) $(RESOURCES) $(JUCE_OUTDIR)/$(JUCE_TARGET_SHARED_CODE)

Choose a reason for hiding this comment

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

yes

But I'd think this is a bug in Projucer

@dehnhardt
Copy link
Contributor Author

I think you should split the patch into 3 parts: MIDI, Linux and Debug.

As I said in the notes on the top, this PR was not intended to be applied, because still some things doe not work. I had no time to dig deeper into that, I just did not want to throw away my findings.
Related to the debug settings: Strangely, the application only runs when I do a debug build. With a release build, it crashes as soon as I open a panel. Therefore, they somehow already belong in this patch ;-)

@mtarenskeen
Copy link

mtarenskeen commented Mar 22, 2021

My coding skills are too limited to contribute much, but following this issue with interest.
I would love to see a reliable stable new Linux build. Just one request: Ubuntu is very popular, but Linux is more than just Ubuntu. Would be nice if the build scripts will work on other Linux distributions as well.

@keinstein
Copy link

Could you please try current master? Some of my changes have been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants