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

Fix raspijamulus #2267

Merged
merged 6 commits into from
Jan 26, 2022
Merged

Fix raspijamulus #2267

merged 6 commits into from
Jan 26, 2022

Conversation

corrados
Copy link
Contributor

@corrados corrados commented Jan 23, 2022

Short description of changes
raspijamulus.sh did not work for newest Raspbian OS.

Context: Fixes an issue?
Missing install packets, new jack2 version needed.

Does this change need documentation? What needs to be documented and how?
No.

Status of this Pull Request
Unforunately, the current Jamulus source code does not compile on my Raspberry Pi with the following error message:

src/util.cpp: In static member function ‘static bool NetworkUtil::ParseNetworkAddress(QString, CHostAddress&, bool)’:
src/util.cpp:708:28: error: variable ‘QRegularExpression rx1’ has initializer but incomplete type
  708 |     QRegularExpression rx1 ( "^\\[([^]]*)\\](?::(\\d+))?$" ); // [addr4or6] or [addr4or6]:port
      |                            ^
src/util.cpp:709:28: error: variable ‘QRegularExpression rx2’ has initializer but incomplete type
  709 |     QRegularExpression rx2 ( "^([^:]*)(?::(\\d+))?$" );       // addr4 or addr4:port or host or host:port
      |                            ^
src/util.cpp:713:29: error: variable ‘QRegularExpressionMatch rx1match’ has initializer but incomplete type
  713 |     QRegularExpressionMatch rx1match = rx1.match ( strAddress );
      |                             ^~~~~~~~
src/util.cpp:714:29: error: variable ‘QRegularExpressionMatch rx2match’ has initializer but incomplete type
  714 |     QRegularExpressionMatch rx2match = rx2.match ( strAddress );
      |                             ^~~~~~~~
make: *** [Makefile:1762: util.o] Error 1

What is missing until this pull request can be merged?
The above compilation error is independent of this pull request.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@corrados corrados changed the title Corrados fix raspijamulus Fix raspijamulus Jan 23, 2022
@softins
Copy link
Member

softins commented Jan 23, 2022

Hi Volker, nice to see you here again!

I recently updated my Pi to the latest Raspberry Pi OS, and Jamulus still compiles fine. Here is my OS version:

tony@pi:~ $ cat /etc/issue
Raspbian GNU/Linux 11 \n \l

tony@pi:~ $ cat /etc/os-release
PRETTY_NAME="Raspbian GNU/Linux 11 (bullseye)"
NAME="Raspbian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=raspbian
ID_LIKE=debian
HOME_URL="http://www.raspbian.org/"
SUPPORT_URL="http://www.raspbian.org/RaspbianForums"
BUG_REPORT_URL="http://www.raspbian.org/RaspbianBugs"

The Qt packages I have installed are:

tony@pi:~ $ apt list qt\* --installed
Listing... Done
qt3d5-doc/stable,now 5.15.2+dfsg-2+rpi1 all [installed,automatic]
qt5-assistant/stable,now 5.15.2-5 armhf [installed,automatic]
qt5-doc/stable,now 5.15.2-2 all [installed]
qt5-gtk-platformtheme/stable,now 5.15.2+dfsg-9+rpi1 armhf [installed,automatic]
qt5-gtk2-platformtheme/stable,now 5.0.0+git23.g335dbec-4+rpt1 armhf [installed,automatic]
qt5-qmake-bin/stable,now 5.15.2+dfsg-9+rpi1 armhf [installed,automatic]
qt5-qmake/stable,now 5.15.2+dfsg-9+rpi1 armhf [installed]
qt5-qmltooling-plugins/stable,now 5.15.2+dfsg-6+rpi1 armhf [installed,automatic]
qt5-style-plugin-cleanlooks/stable,now 5.0.0+git23.g335dbec-4+rpt1 armhf [installed,automatic]
qt5-style-plugin-motif/stable,now 5.0.0+git23.g335dbec-4+rpt1 armhf [installed,automatic]
qt5-style-plugin-plastique/stable,now 5.0.0+git23.g335dbec-4+rpt1 armhf [installed,automatic]
qt5-style-plugins/stable,now 5.0.0+git23.g335dbec-4+rpt1 armhf [installed,automatic]
qt5ct/stable,now 1.1-1+rpt1 armhf [installed,automatic]
qtattributionsscanner-qt5/stable,now 5.15.2-5 armhf [installed,automatic]
qtbase5-dev-tools/stable,now 5.15.2+dfsg-9+rpi1 armhf [installed,automatic]
qtbase5-dev/stable,now 5.15.2+dfsg-9+rpi1 armhf [installed,automatic]
qtbase5-doc/stable,now 5.15.2+dfsg-9+rpi1 all [installed,automatic]
qtcharts5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtchooser/stable,now 66-2 armhf [installed,automatic]
qtconnectivity5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtcreator-data/stable,now 4.14.1-1 all [installed]
qtcreator-doc/stable,now 4.14.1-1 all [installed]
qtcreator/stable,now 4.14.1-1 armhf [installed]
qtdatavisualization5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtdeclarative5-dev-tools/stable,now 5.15.2+dfsg-6+rpi1 armhf [installed,automatic]
qtdeclarative5-dev/stable,now 5.15.2+dfsg-6+rpi1 armhf [installed]
qtdeclarative5-doc/stable,now 5.15.2+dfsg-6+rpi1 all [installed,automatic]
qtgraphicaleffects5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtlocation5-doc/stable,now 5.15.2+dfsg-2 all [installed,automatic]
qtmultimedia5-doc/stable,now 5.15.2-3 all [installed,automatic]
qtnetworkauth5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtquickcontrols2-5-doc/stable,now 5.15.2+dfsg-2 all [installed,automatic]
qtquickcontrols5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtscript5-doc/stable,now 5.15.2+dfsg-2 all [installed,automatic]
qtscxml5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtsensors5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtserialbus5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtserialport5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtsvg5-doc/stable,now 5.15.2-3 all [installed,automatic]
qttools5-dev-tools/stable,now 5.15.2-5 armhf [installed]
qttools5-doc/stable,now 5.15.2-5 all [installed,automatic]
qttranslations5-l10n/stable,now 5.15.2-2 all [installed,automatic]
qtvirtualkeyboard5-doc/stable,now 5.15.2+dfsg-2 all [installed,automatic]
qtwayland5-doc/stable,now 5.15.2-3 all [installed,automatic]
qtwebchannel5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtwebengine5-doc/stable,now 5.15.2+dfsg-3 all [installed,automatic]
qtwebsockets5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtwebview5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtx11extras5-doc/stable,now 5.15.2-2 all [installed,automatic]
qtxmlpatterns5-dev-tools/stable,now 5.15.2-3 armhf [installed,automatic]
qtxmlpatterns5-doc/stable,now 5.15.2-3 all [installed,automatic]

Maybe something is missing on your Pi?

@softins
Copy link
Member

softins commented Jan 23, 2022

Although I only compile using qmake Jamulus.pro && make clean && make, and not using the script distributions/raspijamulus.sh. I don't know if that makes a difference; I'll have to try it.

@ann0see
Copy link
Member

ann0see commented Jan 23, 2022

Hi Volker! Great to see you again.
Hope you’re fine.

Concerning the error you’ve mentioned, does this occur independently of your changes? If yes, probably it’s better to have a separate issue.

@ann0see
Copy link
Member

ann0see commented Jan 23, 2022

I think this should probably be in 3.8.2?

@hoffie
Copy link
Member

hoffie commented Jan 23, 2022

Hi Volker, great to see you here! :)

As far as I understand, this PR provides improvements/updates for the raspijamulus.sh script, so it basically fixes compatibility with latest (2021-10-30?) Raspbian versions, right?

In addition, you've encountered a compile error, right? I've opened issue #2272 (+ PR #2273) for the latter to keep this PR focussed on the proposed changes.

@corrados
Copy link
Contributor Author

Thanks for your immediate response. First, it is a pleasure for me to see that the Jamulus project is in such a good shape. Many thanks (again) for you to organize and manage the project so well :-).

The reason I am using the raspijamulus script is that I have build a prototype hardware for my new project Edrumulus which is an open source e-drum trigger module. I am using a microprocessor (ESP32) running the Edrumulus software and a Raspberry Pi 3B+ running Drumgizmo and Jamulus at the same time. Here is a picture of that prototype:

Prototype 3 (HD-1)

I recently updated my Pi to the latest Raspberry Pi OS, and Jamulus still compiles fine.
[...]
Although I only compile using qmake Jamulus.pro && make clean && make, and not using the script distributions/raspijamulus.sh. I don't know if that makes a difference; I'll have to try it.

The normal qmake Jamulus.pro && make clean && make works fine for me, too.

Concerning the error you’ve mentioned, does this occur independently of your changes?

At least the correct Raspbian packages needed to be installed. The package qt5-default was not found in the current Raspbian OS.

this PR provides improvements/updates for the raspijamulus.sh script, so it basically fixes compatibility with latest (2021-10-30?) Raspbian versions, right?

Yes, this is the purpose.

In addition, you've encountered a compile error, right? I've opened issue #2272 (+ PR #2273) for the latter to keep this PR focussed on the proposed changes.

Thank you. And you already have a fix, that was quick! :-)

BTW: In the raspijamulus script I was using a custom OPUS library because it was compiled with fixed point which gave me some speed improvements on my old Raspberry Pi Zero back in the days: ./configure --enable-custom-modes --enable-fixed-point. I can see that you have added a section in the Jamulus.pro file like, e.g., :

QMAKE_EXTRA_COMPILERS += sse_cc sse2_cc sse4_cc

Does this mean that the normal Jamulus compilation now uses fixed point on ARM by default? Or is this just using SSE extensions which cannot be used on ARM?

@@ -5,7 +5,7 @@ OPUS="opus-1.3.1"
NCORES=$(nproc)

# install required packages
pkgs='alsamixergui build-essential qt5-default libasound2-dev cmake libglib2.0-dev'
pkgs='alsamixergui build-essential qt5-qmake qtdeclarative5-dev qttools5-dev-tools libjack-jackd2-dev cmake libglib2.0-dev'
Copy link
Member

Choose a reason for hiding this comment

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

I have just tried this script on a Pi with fresh install of Raspbian 10 (which has Qt 5.11.3). I found that it still needs qt5-default and libasound2-dev:

  • without libasound2-dev, the build of jack2 fails
  • without qt5-default, the build of Jamulus fails.
    I know that qttools5-dev-tools is needed for Qt >= 5.12 to provide lrelease.
    And I have just seen your comment that qt5-default is not in Raspbian 11 (I was just about to try it myself). Interesting... it would be good if we could make this script work on both OS versions.

Copy link
Member

Choose a reason for hiding this comment

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

  • without libasound2-dev, the build of jack2 fails

I have just found this is also true on Raspbian 11, so libasound2-dev must still be included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting. It did compile fine on my Raspbian. But maybe I had this package libasound2-dev was already installed. I'll do some more tests and update the PR, soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding libasound2-dev is no problem because it is available. But since qt5-default is not available and cannot be installed, running the raspijamulus script will always ask Do you want to install missing packages? but if you answer with yes you'll get

Package qt5-default is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source

E: Package 'qt5-default' has no installation candidate

The idea was that the script simply runs without interaction as soon as it was once installing everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ups... I just noticed another thing which was incorrect (it's a very long time ago when I was initially writing this script): Since we checkout our own version of jack2 and compile it, we also use the headers and libraries from that location and do not use the system jack2-dev files. So, I also removed the libjack-jackd2-dev again. Sorry for the inconvenience...

Copy link
Member

Choose a reason for hiding this comment

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

One thing I have found when running this script on a fresh OS (same on both 10 and 11), is that when Jamulus compilation is finished, and the script runs Jamulus, it complains that it cannot connect to or start a jack server, and suggests starting jackd manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You need to have an USB audio card connected when you start the script. It takes the first device it finds (ADEVICE=$(aplay -l|grep "USB Audio"|tail -1|cut -d' ' -f3)) and tells you as an output what device it found and wants to use: echo "Using USB audio device: ${ADEVICE}". I assume that in your case no audio device was found and therefore the jack daemon could not be started.

@softins
Copy link
Member

softins commented Jan 24, 2022

BTW: In the raspijamulus script I was using a custom OPUS library because it was compiled with fixed point which gave me some speed improvements on my old Raspberry Pi Zero back in the days: ./configure --enable-custom-modes --enable-fixed-point. I can see that you have added a section in the Jamulus.pro file like, e.g., :

QMAKE_EXTRA_COMPILERS += sse_cc sse2_cc sse4_cc

Does this mean that the normal Jamulus compilation now uses fixed point on ARM by default? Or is this just using SSE extensions which cannot be used on ARM?

I think it is just SSE, but I am not certain. The SSE stuff was added by @npostavs back in February.

@softins
Copy link
Member

softins commented Jan 24, 2022

I have just done a standard compile after a make distclean, and I can see that there is no ./configure step for the OPUS libs included in the Jamulus tree, before they start to be compiled. So presumably they are being compiled with the default configuration that is already there, and might not be optimised for the particular system?

@corrados
Copy link
Contributor Author

The way I integrated the OPUS library was to include all the OPUS sources and compile it together with all the other Jamulus source file. The original ./configure of the OPUS library was therefore not used. So, I assume that the normal Jamulus compilation does not use an optimized OPUS build and therefore it makes sense to keep the custom OPUS library creation in the raspijamulus script.

@softins
Copy link
Member

softins commented Jan 24, 2022

The way I integrated the OPUS library was to include all the OPUS sources and compile it together with all the other Jamulus source file. The original ./configure of the OPUS library was therefore not used. So, I assume that the normal Jamulus compilation does not use an optimized OPUS build and therefore it makes sense to keep the custom OPUS library creation in the raspijamulus script.

I suppose the question then is: what compilation defines get set up by ./configure --enable-custom-modes --enable-fixed-point, and could those be replicated in the Jamulus.pro setup appropriately for RPi?

@npostavs
Copy link
Contributor

I think it is just SSE, but I am not certain. The SSE stuff was added by @npostavs back in February.

Yes, basically I added the arch-specific optimizations into the Jamulus qmakefile. The referenced line is just for x86/64, but for arm I enabled NEON intrinsics (I don't really know if/how it works, but the #defines are there, as far as I know). I didn't try to enable fixed-point.

jamulus/Jamulus.pro

Lines 714 to 717 in 2501880

contains(QT_ARCH, armeabi-v7a) | contains(QT_ARCH, arm64-v8a) {
HEADERS_OPUS += $$HEADERS_OPUS_ARM
SOURCES_OPUS_ARCH += $$SOURCES_OPUS_ARM
DEFINES_OPUS += OPUS_ARM_PRESUME_NEON=1 OPUS_ARM_PRESUME_NEON_INTR=1

@corrados
Copy link
Contributor Author

Actually, I would not change anything in the Jamulus.pro file to support fixed-point OPUS. For most of the Jamulus use cases it is not needed. Just for very special cases like running Jamulus on a Raspberry Pi Zero, you will see a small improvement in speed if using a fixed-point OPUS library. And exactly for these special cases there is the raspijamulus script which can be used. This uses an external OPUS library which is configured with fixed-point support.

@softins
Copy link
Member

softins commented Jan 26, 2022

I have studied and tested this on both Raspbian 10 and 11, with the aim of making the script compatible with both. This is what I have found:

  1. On 10, the package qt5-default does two main things: pull in qtbase5-dev as a dependency, and set the default version of Qt for qtchooser. Without this, there is no default version.
  2. On 11, the package qt5-default does not exist. qtbase5-dev is still available as the main package to install, and the default version of Qt for qtchooser is automatically set to 5 by one of the installed dependencies.
  3. Using qtchooser -list-versions is an OS-independent way of determining whether there is already a default or qt5-default is required.
  4. Package qtdeclaractive5-dev is not required at all, as it only provides modules not used by Jamulus.
  5. Package qttools5-dev-tools is available on both 10 and 11, and is required on 11 to provide lrelease for qmake.

So I would recommend the following section of code for installing packages on both 10 and 11 (I have tested on fresh installs of both):

# install required packages
pkgs='alsamixergui build-essential qtbase5-dev qttools5-dev-tools libasound2-dev cmake libglib2.0-dev'
if ! dpkg -s $pkgs >/dev/null 2>&1; then
  read -p "Do you want to install missing packages? " -n 1 -r
  echo
  if [[ $REPLY =~ ^[Yy]$ ]]; then
    sudo apt-get install $pkgs -y
    # Raspbian 10 needs qt5-default; Raspbian 11 doesn't need or provide it
    if ! qtchooser -list-versions | grep -q default; then
      sudo apt-get install qt5-default -y
    fi
  fi
fi

I have a version with this change at https://github.com/softins/jamulus/tree/fix-pi

@softins softins added this to the Release 3.8.2 milestone Jan 26, 2022
@corrados
Copy link
Contributor Author

Thanks Tony for your effort. I really like your solution.
I tried to merge your changes but I had some Git issues so that my branch is now in an invalid state. Please do not merge.

# Conflicts:
#	distributions/raspijamulus.sh
@corrados
Copy link
Contributor Author

I think I have fixed my Git issue. Now it's ready for merge.

Package qtdeclaractive5-dev is not required at all, as it only provides modules not used by Jamulus.

I took the required packages from the COMPILING.md file from section "On Ubuntu-based distributions 18.04+, Debian 9+, and Raspberry Pi OS:". Maybe this should be updated, too.

@ann0see
Copy link
Member

ann0see commented Jan 26, 2022

Yes. And probably the CI too.

Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@softins softins requested a review from hoffie January 26, 2022 17:55
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

CHANGELOG: Build: Raspijamulus build script has been improved to work with the latest Raspberry Pi OS releases and to include a newer JACK version.

This pull request was closed.
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.

5 participants