Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Boot - set env vars for pipewire (matching pw-jack's behaviour) #3349

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

Merged
samaaron merged 4 commits into dev from pipewire-env
Nov 1, 2023

Conversation

@samaaron
Copy link
Collaborator

@samaaron samaaron commented Oct 31, 2023

pw-jack sets the PIPEWIRE_QUANTUM and LD_LIBRARY_PATH env vars before execing a binary. To use this we'd need to run sonic pi from pw-jack (so the exec would do its work correctly). Instead of that we set the env vars within the call to popen2e when starting scsynth to match this behaviour.

It is now also possible to use new change pipewire's buffsize and samplerate with two new audio-settings.toml config options:

linux_pipewire_buffsize = 1024
linux_pipewire_samplerate = 4800

pw-jack sets the PIPEWIRE_QUANTUM and LD_LIBRARY_PATH env vars before execing a binary. To use this we'd need to run sonic pi from pw-jack (so the exec would do its work correctly). Instead of that we set the env vars within the call to popen2e when starting scsynth to match this behaviour.
It is now also possible to use new change pipewire's buffsize and samplerate with two new audio-settings.toml config options:
```
linux_pipewire_buffsize = 1024
linux_pipewire_samplerate = 4800
```
Copy link
Collaborator Author

@rbnpi , @porras could you please take a look at this draft PR. It's completely untested, but feels like a good first try at solving the pw-jack issues discussed here: #3347 (comment)

Copy link
Contributor

rbnpi commented Oct 31, 2023
edited
Loading

The default server sample rate should be 48000 not 4800 Change throughout in daemon and toml file. YOu've consistently used 4800.
Also, daemon is picking up changes for samplerate and buffer size for pipewire but is still using default values to start. See daemon log below

[2023年10月31日 22:56:33] Got Audio Settings toml hash: {:linux_pipewire_buffsize=>512, :linux_pipewire_samplerate=>96000}
[2023年10月31日 22:56:33] Unified Audio Settings toml hash: {}
[2023年10月31日 22:56:33] Combined Audio Settings toml hash with GUI scsynth inputs hash: {}
[2023年10月31日 22:56:33] Merged Audio Settings toml hash: {"-u"=>33476, "-a"=>"1024", "-m"=>"131072", "-D"=>"0", "-R"=>"0", "-l"=>"1", "-i"=>"2", "-o"=>"2", "-b"=>"4096", "-B"=>"127.0.0.1", "-c"=>"128", "-z"=>"128", "-U"=>"/usr/lib/SuperCollider/plugins"}
[2023年10月31日 22:56:33] Using default pipewire buffsize of: 1024
[2023年10月31日 22:56:33] Using default pipewire buffsize of: 48000
[2023年10月31日 22:56:33] Starting scsynth with LD_LIBRARY_PATH set to "/usr/${LIB}/pipewire-0.3/jack" so it uses pipewire's jack

otherwise starting everything fine.
EDIT NOTE I'd changed the 4800 to 48000 in my copy already: hence 48000 in the log

Copy link
Collaborator Author

samaaron commented Oct 31, 2023
edited
Loading

@rbnpi - thanks for testing. I had already spotted and fixed the 4800 issue in a subsequent commit to this PR. I've also just pushed something which will hopefully fix the default-only behaviour you noticed. Let me know how that goes.

Copy link
Contributor

rbnpi commented Oct 31, 2023

Works fine now. Well done!
One other useful addition would be to add different default connection scripts so that the user could choose what to connect to in the toml file by default At present can be done by the program I include in the build instructions which could be put into the init.rb file. I'm just about to pr this as is together with the change to the pi_built_gui.sh file to add environmental variable to find qt6 on the Pi.

Copy link
Collaborator Author

Great, although I'm rather curious as to why the LD_LIBRARY_PATH returned from pw-jack is "/usr/${LIB}/pipewire-0.3/jack" in that it explicitly contains ${LIB} which I would have thought should have been expanded out both when it was set via the export in the pw-jack script and then subsequently in the echo in the one-liner shell script in daemon.rb.

Out of interest, what does the following output on your Pi when entered directly into the terminal:

pw-jack /bin/sh -c 'echo $LD_LIBRARY_PATH'

With respect to your PRs, please use different ones for the Qt6 env var and the pipewire redirect stuff. For the latter, I'd much rather look into a nicely engineered solution rather than rely on overloading init.rb and would like to consider other people's thoughts on the matter too.

Copy link
Collaborator Author

@rbnpi Could you also let me know the output of this:

cat `which pw-jack`

Copy link
Contributor

rbnpi commented Nov 1, 2023

cat `which pw-jack`
#!/bin/sh
# This file is part of PipeWire.
#
# Copyright �© 2020 Wim Taymans
#
# 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 (including the next
# paragraph) 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.
#
DEFAULT_SAMPLERATE=48000
while getopts 'hr:vs:p:' param ; do
	case $param in
		r)
			PIPEWIRE_REMOTE="$OPTARG"
			export PIPEWIRE_REMOTE
			;;
		v)
			if [ -z "$PIPEWIRE_DEBUG" ]; then
				PIPEWIRE_DEBUG=3
			else
				PIPEWIRE_DEBUG=$(( PIPEWIRE_DEBUG + 1 ))
			fi
			export PIPEWIRE_DEBUG
			;;
		s)
			SAMPLERATE="$OPTARG"
			;;
		p)
			PERIOD="$OPTARG"
			;;
		*)
			echo "0ドル - run JACK applications on PipeWire"
			echo " "
			echo "0ドル [options] application [arguments]"
			echo " "
			echo "options:"
			echo "	-h show brief help"
			echo "	-r <remote> remote daemon name"
			echo "	-v verbose debug info"
			echo "	-s samplerate (default \"$DEFAULT_SAMPLERATE\")"
			echo "	-p period in samples"
			exit 0
			;;
	esac
done
shift $(( OPTIND - 1 ))
if [ -n "$PERIOD" ]; then
	if [ -n "$SAMPLERATE" ]; then
		PIPEWIRE_QUANTUM="$PERIOD/$SAMPLERATE"
	else
		PIPEWIRE_QUANTUM="$PERIOD/$DEFAULT_SAMPLERATE"
	fi
	export PIPEWIRE_QUANTUM
fi
LD_LIBRARY_PATH='/usr/${LIB}/pipewire-0.3/jack'"${LD_LIBRARY_PATH+":$LD_LIBRARY_PATH"}"
export LD_LIBRARY_PATH
exec "$@"

Copy link
Collaborator Author

samaaron commented Nov 1, 2023

@rbnpi and this:

pw-jack /bin/sh -c 'echo $LD_LIBRARY_PATH'

@samaaron samaaron merged commit 5fbe330 into dev Nov 1, 2023
Copy link
Contributor

porras commented Nov 1, 2023

I just tested this and works fine for the Linux flatpak 👍

I think it will break in Linux systems without pipewire (the pw-jack call will fail and we'll call scsynth with a load path overwritten to an empty string, or garbage like command not found , which... will probably fail I guess). I don't know if keeping that backwards compatibility at least for a couple of releases should be a goal, but if it's not, then it can also be removed from the other places (not necessarily now but it's dead code). E.g. https://github.com/sonic-pi-net/sonic-pi/blob/dev/app/server/ruby/bin/daemon.rb#L1282

Copy link
Collaborator Author

samaaron commented Nov 1, 2023

@porras agreed. I'm all in favour of completely dropping support for vanilla jack as that eases the maintenance burden - but I also realise that might be ideal for everyone.

At the very least we should update the linux build instructions to communicate this :-)

Copy link
Collaborator Author

samaaron commented Nov 1, 2023

@porras BTW, is your Flatpak build using Qt5 or Qt6?

Copy link
Contributor

porras commented Nov 1, 2023 via email

Qt5. That's an upgrade I wanted to tackle independently of the release, together with other dependencies (it's also bundling slightly outdated versions of ruby, Erlang and elixir).
...
On Wed, Nov 1, 2023, 09:54 Sam Aaron ***@***.***> wrote: @porras <https://github.com/porras> BTW, is your Flatpak build using Qt5 or Qt6? — Reply to this email directly, view it on GitHub <#3349 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAA55U27NGR6X45MA7LSW3YCIE5HAVCNFSM6AAAAAA6YM4CQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBYGYYTQNRUG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Copy link
Collaborator Author

samaaron commented Nov 1, 2023

Cool - Qt6 shouldn't be any problem, I've been running it for years on Windows and macOS and @rbnpi just got things built over there for the latest Pi OS.

Copy link
Collaborator Author

samaaron commented Nov 1, 2023

@porras - I've tagged v4.5.0 - let me know if you need any help making the Flatpak. Will it be published to Flathub?

Copy link
Contributor

porras commented Nov 2, 2023 via email

It's already there :) https://flathub.org/apps/net.sonic_pi.SonicPi (I just realized, with the wrong date 🙄 I added the date when I created the branch, then forgot to update it yesterday. No big deal I guess, or I'll see if it can be fixed without rebuild)
...
On Wed, Nov 1, 2023, 17:10 Sam Aaron ***@***.***> wrote: @porras <https://github.com/porras> - I've tagged v4.5.0 - let me know if you need any help making the Flatpak. Will it be published to Flathub? — Reply to this email directly, view it on GitHub <#3349 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAA55QX7UI6ZBAEXSYJKXDYCJX6BAVCNFSM6AAAAAA6YM4CQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBZGIZTKNZTGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>
samaaron reacted with thumbs up emoji

Copy link
Collaborator Author

samaaron commented Nov 2, 2023

@porras lovely stuff. Thanks so much for your hard work with this. It's really great to have an up-to-date Flatpak build.

Out of interest - what is the required format for the change log? It's currently empty and it would be nice to be able to show what's changed.

Copy link
Contributor

porras commented Nov 3, 2023

It's a subset of HTML. I already added it, I had never noticed that it shows on the page, it looked quite sad: https://github.com/flathub/net.sonic_pi.SonicPi/blob/0e7f1570f4fd0aa3096d21459e04d6b03a350b11/net.sonic_pi.SonicPi.appdata.xml#L40

Unfortunately I got the subset wrong, I removed some things after converting from markdown but left <code>, which I shouldn't have 🙄:

Screenshot_20231103-092735-694

I'm going to leave it as is, it's ugly but not that bad I guess. Updating it after the release (which I already did exceptionally to add it) is kind of unpolite since it triggers a (very big in GB) update for those that had already downloaded it, for no reason because there are no changes to the package. So I'll just take it into account on future releases.

Copy link
Collaborator Author

samaaron commented Nov 3, 2023

Thanks for making the effort to add it. Totally agreed on not changing it again - it's much better to have something than nothing and you've also learned something about the formatting options!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rbnpi rbnpi Awaiting requested review from rbnpi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /