Add Airplay 2 support

fabienheureux · 2023-01-22T10:10:46Z

Description

Following #607
This is an attempt to bring airplay 2 support to balena sound.

What's working ?

Current setup

a Raspberry pi 4 + hifiberry DAC, that is my current multiroom master
a raspberry pi zero w + hifiberry miniamp, that is a multiroom client

Features tested

Streaming to a single client
Streaming to multiple clients at once
Select volume on each client

I tested all this with SOUND_MODE=standalone and SOUND_MODE=MULTI_ROOM or SOUND_MODE=MULTI_ROOM_CLIENT

Issues

I noticed even before this upgrade that my raspberry pi zero had high CPU usage, and the sound was stuttering while streaming as a multiroom client.
I was not able to properly test the sound output for this device, the stuttering persisted after the shairport's version upgrade. I am pretty sure this is a different issue, but I would be glad if other users tested on their properly working Rpi zero to ensure this is not an issue caused by these changes.

How to test it locally

Install Balena Cloud CLI
Clone my repo git clone -b feature/bring-airplay-2-support https://github.com/fabienheureux/balena-sound.git
Go to the repo directory cd balena-sound
Publish it to your fleet using the cli balena push YOUR_FLEET_NAME
Check on the dashboard that airplay is downloaded and properly restarted
Enjoy 🎉

fabienheureux · 2023-01-22T10:27:43Z

~~Note to self : first try, got warnings & errors below~~

airplay  warning: could not acquire a Shairport Sync native D-Bus interface "org.gnome.ShairportSync.i8" on the system bus.
airplay  warning: could not acquire an MPRIS interface named "org.mpris.MediaPlayer2.ShairportSync.i8" on the system bus.
airplay  fatal error: Can't access NQPTP! Is it installed and running?

Edit : This was fixed by starting NQPTP before shairport-sync

fabienheureux · 2023-01-22T12:11:33Z

The current version works on my devices, I would be glad to have a review at that point 😇

laytonhayes · 2023-01-23T21:03:32Z

Just tested this on a RasPi3 with HiFiBerry AMP2 and RasPi3 with USB DAC. Works great!

maggie44 · 2023-01-24T10:29:00Z

~~Note to self : first try, got warnings & errors below~~

airplay  warning: could not acquire a Shairport Sync native D-Bus interface "org.gnome.ShairportSync.i8" on the system bus.
airplay  warning: could not acquire an MPRIS interface named "org.mpris.MediaPlayer2.ShairportSync.i8" on the system bus.
airplay  fatal error: Can't access NQPTP! Is it installed and running?

Edit : This was fixed by starting NQPTP before shairport-sync

Wow, thanks for looking in to this!

I had a read through, it seems that the NQPTP service is already scheduled to start in the container (https://github.com/mikebrady/shairport-sync/blob/master/docker/etc/s6-overlay/s6-rc.d/03-nqptp/run), but we override this in our script ENTRYPOINT []. That still makes sense, but now we have two processes running in one container. I can see that it would function, but it would be harder to debug because the logs of the first service (NQPTP) may not make it back up without running as PID1, and when it comes to shutdown it will probably end up getting killed. Hard to predict exactly what would happen in these circumstances, but it's never ideal.

I think the easiest way, without deviating too far from how it already is built by the maintainer, would be to use supervisor (apk add supervisor). We could use s6 like they do, but I would really like to try and keep it as simple and user friendly as possible, and that's probably using Superisor. It adds a bit of weight to the image sizes, but negligible.

Here is a good guide on implementation: https://towardsdatascience.com/run-multiple-services-in-single-docker-container-using-supervisor-b2ed53e3d1c0. Do you think this is something you would be able to add in to this PR? It would mean that both services are started, but that they are all connected to the initial running process and are managed properly by balena-engine.

maggie44 · 2023-01-24T10:33:33Z

I also noticed it is running with #!/bin/ash. I know this isn't your addition, but as you are refactoring could you switch that to sh for us?

#!/usr/bin/env sh

/bin/sh ...

fabienheureux · 2023-01-24T10:54:23Z

I was not clear about what was started by the container actually, without the manual start of NQPTP.
I will look into this, I am not very familiar with this kind of development (being more a web developper usually), so no problem with doing these changes 👍

fabienheureux · 2023-01-24T11:17:03Z

I made some changes @Maggie0002 but we might have an issue here.
It appears that shairport-sync can't be demonized with the default build.

See the logs below (sorry for the typo in shairpot 😅)

24.01.23 12:08:16 (+0100)  airplay  2023-01-24 11:08:16,860 INFO supervisord started with pid 7
24.01.23 12:08:17 (+0100)  airplay  2023-01-24 11:08:17,866 INFO spawned: 'nqptp' with pid 9
24.01.23 12:08:17 (+0100)  airplay  2023-01-24 11:08:17,878 INFO spawned: 'shairpot-sync' with pid 10
24.01.23 12:08:18 (+0100)  airplay  shairport-sync was built without libdaemon, so does not support daemonisation using the -d, --daemon, -j or --justDaemoniseNoPIDFile options
24.01.23 12:08:18 (+0100)  airplay  2023-01-24 11:08:18,025 INFO exited: shairpot-sync (exit status 1; not expected)
24.01.23 12:08:19 (+0100)  airplay  2023-01-24 11:08:19,030 INFO success: nqptp entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)
24.01.23 12:08:19 (+0100)  airplay  2023-01-24 11:08:19,040 INFO spawned: 'shairpot-sync' with pid 11
24.01.23 12:08:19 (+0100)  airplay  shairport-sync was built without libdaemon, so does not support daemonisation using the -d, --daemon, -j or --justDaemoniseNoPIDFile options
24.01.23 12:08:19 (+0100)  airplay  2023-01-24 11:08:19,104 INFO exited: shairpot-sync (exit status 1; not expected)
24.01.23 12:08:21 (+0100)  airplay  2023-01-24 11:08:21,118 INFO spawned: 'shairpot-sync' with pid 12
24.01.23 12:08:21 (+0100)  airplay  shairport-sync was built without libdaemon, so does not support daemonisation using the -d, --daemon, -j or --justDaemoniseNoPIDFile options
24.01.23 12:08:21 (+0100)  airplay  2023-01-24 11:08:21,181 INFO exited: shairpot-sync (exit status 1; not expected)
24.01.23 12:08:24 (+0100)  airplay  2023-01-24 11:08:24,196 INFO spawned: 'shairpot-sync' with pid 13
24.01.23 12:08:24 (+0100)  airplay  shairport-sync was built without libdaemon, so does not support daemonisation using the -d, --daemon, -j or --justDaemoniseNoPIDFile options
24.01.23 12:08:24 (+0100)  airplay  2023-01-24 11:08:24,257 INFO exited: shairpot-sync (exit status 1; not expected)
24.01.23 12:08:25 (+0100)  airplay  2023-01-24 11:08:25,260 INFO gave up: shairpot-sync entered FATAL state, too many start retries too quickly

I am not clear on the best approach here.
I checked supervisor docs and it does not seem possible to run an application in the foreground, as supervisor will already be.

fabienheureux · 2023-01-24T11:18:09Z

We could build a custom docker image based on the official one, but it would mean deviate even farer from how it is currently built by the maintainer...

maggie44 · 2023-01-24T11:22:55Z

Have you tried with the --with-libdaemon flag?

mikebrady/shairport-sync#909

maggie44 · 2023-01-24T11:24:45Z

Or --with-dbus-interface? Although they may actually be build time flags rather than runtime flags. Hmm..

mikebrady/shairport-sync#1137

maggie44 · 2023-01-24T11:33:53Z

Hang on, I'm a bit confused. Looking again, I think you are going from the script to SupervisorD. But SupervisorD should be PID 1. So we could either add that in as the CMD in the Dockerfile directly and then use it to call the service. Or alternatively, have SupervisorD call the .sh script. Probably the latter is the closest to the original configuration. So it would be SupervisorD is the first thing called by the Dockerfile, not the .sh script. Then SupervisorD will call the .sh script.

fabienheureux · 2023-01-24T11:50:37Z

Yes they are build time flags, hence the suggestion to use a custom dockerfile, but this is not an ideal setup in my opinion.
I will try to use supervisor instead of the .sh script 👍

fabienheureux · 2023-01-24T11:58:43Z

I have tried your suggestion and it works !
I rebased and am ready for a new review

Logs below

24.01.23 12:56:34 (+0100)  airplay  2023-01-24 11:56:34,612 INFO supervisord started with pid 1
24.01.23 12:56:35 (+0100)  airplay  2023-01-24 11:56:35,619 INFO spawned: 'nqptp' with pid 8
24.01.23 12:56:35 (+0100)  airplay  2023-01-24 11:56:35,626 INFO spawned: 'shairport-sync' with pid 9
24.01.23 12:56:35 (+0100)  airplay  Starting AirPlay plugin...
24.01.23 12:56:35 (+0100)  airplay  Device name: Jean-Michel Mangemicro
24.01.23 12:56:35 (+0100)  airplay  Starting Shairport Sync
24.01.23 12:56:35 (+0100)  airplay  Shairport-sync started. Device is discoverable as Jean-Michel Mangemicro
24.01.23 12:56:35 (+0100)  airplay  Warning: the option -u is no longer needed and is deprecated. Debug and statistics output to STDERR is now the default. Use "--log-to-syslog" to revert.
24.01.23 12:56:35 (+0100)  airplay  warning: could not acquire a Shairport Sync native D-Bus interface "org.gnome.ShairportSync.i10" on the system bus.
24.01.23 12:56:35 (+0100)  airplay  warning: could not acquire an MPRIS interface named "org.mpris.MediaPlayer2.ShairportSync.i10" on the system bus.
24.01.23 12:56:36 (+0100)  airplay  2023-01-24 11:56:36,811 INFO success: nqptp entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)
24.01.23 12:56:36 (+0100)  airplay  2023-01-24 11:56:36,812 INFO success: shairport-sync entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)

Neustradamus · 2023-01-24T21:05:01Z

To follow!

maggie44 · 2023-01-24T21:16:36Z

Looking solid to me, great work!

@laytonhayes, are you able to test this one for us too?

laytonhayes · 2023-01-24T22:56:52Z

@Maggie0002, tested out the update with both RasPi3s, works great! I'm getting something at startup about a CRIT Supervisor, not sure where that's coming from. Here's the startup log:

airplay  2023-01-24 21:53:58,319 CRIT Supervisor is running as root.  Privileges were not dropped because no user is specified in the config file.  If you intend to run as root, you can set user=root in the config file to avoid this message.
airplay  2023-01-24 21:53:58,354 INFO supervisord started with pid 1
airplay  2023-01-24 21:53:59,365 INFO spawned: 'nqptp' with pid 8
airplay  2023-01-24 21:53:59,379 INFO spawned: 'shairport-sync' with pid 9
airplay  Starting AirPlay plugin...
airplay  Device name: HiFi
airplay  Starting Shairport Sync
airplay  Shairport-sync started. Device is discoverable as HiFi
airplay  Warning: the option -u is no longer needed and is deprecated. Debug and statistics output to STDERR is now the default. Use "--log-to-syslog" to revert.
airplay  warning: could not acquire a Shairport Sync native D-Bus interface "org.gnome.ShairportSync.i10" on the system bus.
airplay  warning: could not acquire an MPRIS interface named "org.mpris.MediaPlayer2.ShairportSync.i10" on the system bus.
airplay  2023-01-24 21:54:00,514 INFO success: nqptp entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)
airplay  2023-01-24 21:54:00,515 INFO success: shairport-sync entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)

fabienheureux · 2023-01-25T06:42:21Z

This is because we added a program (supervisor) that allows to control several processes, that adds this warning when ran as root.
@Maggie0002 would you be happy with me adding the user=root in supervisor config ?

Do you know if docker in balenaOS is ran in rootless mode ? I am not clear of the implication (and risks) of running supervisor as root here.

maggie44 · 2023-01-25T09:45:14Z

@Maggie0002 would you be happy with me adding the user=root in supervisor config ?

Yep, good idea. It is running as root already, and should be, so we can add user=root to silence the message.

Other changes : - Update entrypoint to start NQPTP before starting shairport sync - Add logs when starting both scripts inside entrypoint - Update shairport-sync config to remove default flag Remove flag as it is now the default update supervisor config

fabienheureux · 2023-01-25T11:54:38Z

I have made the changes, as well in this change I removed the --use-stderr flag as it is now the default in shairport-sync https://github.com/mikebrady/shairport-sync/blob/6240fd83707b77fe2fb2250f8136e530e36010d2/shairport.c#LL327C21-L327C21

maggie44 · 2023-01-25T12:11:11Z

This looks tons better, and adds a new feature! @fabienheureux, @laytonhayes, there is no way we would have been able to do this without your help, huge thanks!

@fabienheureux, this looks good to me, if you are happy with the tests and ready for me to pull the trigger and merge then say the word. 👍

fabienheureux · 2023-01-25T12:12:56Z

Always happy to contribute !
Let's merge then 🤘

fabienheureux marked this pull request as ready for review January 22, 2023 12:11

fabienheureux force-pushed the feature/bring-airplay-2-support branch from 55f6f97 to 0a5ecd6 Compare January 22, 2023 12:12

jellyfish-bot changed the title ~~Add Airplay 2 support - Upgrade shairport version~~ Add Airplay 2 support - Upgrade shairport version Jan 22, 2023

fabienheureux mentioned this pull request Jan 23, 2023

Airplay 2 #419

Closed

fabienheureux force-pushed the feature/bring-airplay-2-support branch from 3c20f91 to 686423d Compare January 24, 2023 11:58

fabienheureux mentioned this pull request Jan 24, 2023

Airplay 2 Support ? mikebrady/shairport-sync#535

Closed

fabienheureux force-pushed the feature/bring-airplay-2-support branch from 686423d to 6f77936 Compare January 25, 2023 11:53

maggie44 merged commit 2158f38 into balena-io-experimental:master Jan 25, 2023

maggie44 mentioned this pull request Jan 25, 2023

Upgrade Shairport Sync to version 4.1 #607

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Airplay 2 support - Upgrade shairport version #621

Add Airplay 2 support - Upgrade shairport version #621

fabienheureux commented Jan 22, 2023 •

edited

Loading

fabienheureux commented Jan 22, 2023 •

edited

Loading

fabienheureux commented Jan 22, 2023

laytonhayes commented Jan 23, 2023

maggie44 commented Jan 24, 2023 •

edited

Loading

maggie44 commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

maggie44 commented Jan 24, 2023

maggie44 commented Jan 24, 2023 •

edited

Loading

maggie44 commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023 •

edited

Loading

Neustradamus commented Jan 24, 2023

maggie44 commented Jan 24, 2023 •

edited

Loading

laytonhayes commented Jan 24, 2023

fabienheureux commented Jan 25, 2023 •

edited

Loading

maggie44 commented Jan 25, 2023

fabienheureux commented Jan 25, 2023

maggie44 commented Jan 25, 2023

fabienheureux commented Jan 25, 2023

Add Airplay 2 support - Upgrade shairport version #621

Add Airplay 2 support - Upgrade shairport version #621

Conversation

fabienheureux commented Jan 22, 2023 • edited Loading

Add AirPlay 2 support

Description

What's working ?

Current setup

Features tested

Issues

How to test it locally

fabienheureux commented Jan 22, 2023 • edited Loading

fabienheureux commented Jan 22, 2023

laytonhayes commented Jan 23, 2023

maggie44 commented Jan 24, 2023 • edited Loading

maggie44 commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

maggie44 commented Jan 24, 2023

maggie44 commented Jan 24, 2023 • edited Loading

maggie44 commented Jan 24, 2023

fabienheureux commented Jan 24, 2023

fabienheureux commented Jan 24, 2023 • edited Loading

Neustradamus commented Jan 24, 2023

maggie44 commented Jan 24, 2023 • edited Loading

laytonhayes commented Jan 24, 2023

fabienheureux commented Jan 25, 2023 • edited Loading

maggie44 commented Jan 25, 2023

fabienheureux commented Jan 25, 2023

maggie44 commented Jan 25, 2023

fabienheureux commented Jan 25, 2023

fabienheureux commented Jan 22, 2023 •

edited

Loading

fabienheureux commented Jan 22, 2023 •

edited

Loading

maggie44 commented Jan 24, 2023 •

edited

Loading

maggie44 commented Jan 24, 2023 •

edited

Loading

fabienheureux commented Jan 24, 2023 •

edited

Loading

maggie44 commented Jan 24, 2023 •

edited

Loading

fabienheureux commented Jan 25, 2023 •

edited

Loading