Skip to content

Conversation

@davidandreoletti
Copy link
Contributor

@davidandreoletti davidandreoletti commented Dec 19, 2023

Maintainer: @mikebrady / @thess

Compile tested:

  • arch: armv8
  • model: NanoPi R4S
  • OpenWRT version: 23.05

Run tested:

  • arch: armv8
  • model: NanoPi R4S
  • OpenWRT version: OpenWrt 23.05.2 r23630-842932a63d / LuCI openwrt-23.05 branch git-23.306.39416-c86c256
  • tests:

-- [x] shairport-sync 4.3.2 installed on NanoPi R4S
-- [x] shairport-sync -V reports 'pipe' support

root@test-router-hw1:~# /usr/bin/shairport-sync -V
4.3.2-libdaemon-OpenSSL-Avahi-ALSA-pipe-soxr-metadata-sysconfdir:/etc

-- [x] shairport-sync to write to default pipe on NanoPi R4S with UCI's shairport-sync's 'pipe-name' set to /etc/shairport-sync-audio

root@test-router-hw1:~# ls -alh /tmp/shairport-sync-*
prw-rw-rw-    1 root     root           0 Dec 20 11:58 /tmp/shairport-sync-audio
prw-rw-rw-    1 root     root           0 Dec 20 11:58 /tmp/shairport-sync-metadata
-rw-------    1 root     root      108.9K Dec 20 11:58 /tmp/shairport-sync-openssl_4.3.2-0_aarch64_generic.ipk
  1. Play audio on phone connected to shairport-sync's server instance
cat /tmp/shairport-sync-audio
**** full of gibberish symbol, as I expected :-) ****

Description:

  • Enable pipe audio backend at compile time:
    • UCI's shairport-sync's pipe conf block is defined but has no effect without shairport-sync's pipe support.
    • On SBC with no sound card available (built-in or external) and without ALSA plugins support on OpenwRT (no package) to reroute a stream from shairport-sync's ALSA default output to a PulseAudio controlled Bluetooth sink , then pipe support is required to enable the following flow: shairport-sync -> named pipe -> owntone. See owntone: New package variant: owntone-pulseaudio #22942
    • diff: +746 bytes
      • shairport-sync-openssl_4.3.2-0_aarch64_generic.ipk without --pipe: 110806 bytes
      • shairport-sync-openssl_4.3.2-0_aarch64_generic.ipk with --pipe: 111552 bytes

@davidandreoletti davidandreoletti changed the title WIP: Shairport sync 4.3.2 with pipe+stdout audio backend enabled WIP: Shairport sync 4.3.2 with pipe audio backend enabled Dec 19, 2023
@davidandreoletti davidandreoletti changed the title WIP: Shairport sync 4.3.2 with pipe audio backend enabled Shairport sync 4.3.2 with pipe audio backend enabled Dec 19, 2023
@davidandreoletti davidandreoletti changed the title Shairport sync 4.3.2 with pipe audio backend enabled shairport-sync: Update to 4.3.2 + pipe audio backend enabled Dec 19, 2023
@davidandreoletti davidandreoletti force-pushed the shairport_sync_4_3_2 branch 2 times, most recently from d5b716c to 4dfdbc3 Compare December 19, 2023 16:05
@davidandreoletti davidandreoletti marked this pull request as ready for review December 20, 2023 04:06
@davidandreoletti
Copy link
Contributor Author

davidandreoletti commented Dec 20, 2023

@BKPepe PR ready for a second review round:

  • PR description updated with additional rationales + list of changes.
  • Code fixes implemented.

@davidandreoletti
Copy link
Contributor Author

@BKPepe Do you think there should be 2 package variant instead ?

  • shairport-sync (current)
  • shairport-sync-pipe (additional)

@BKPepe
Copy link
Member

BKPepe commented Dec 20, 2023 via email

@davidandreoletti
Copy link
Contributor Author

@BKPepe Builds issue (missing git signoff) is now resolved. It is ready for review again.

@BKPepe
Copy link
Member

BKPepe commented Dec 21, 2023 via email

@davidandreoletti davidandreoletti force-pushed the shairport_sync_4_3_2 branch 3 times, most recently from c36a2f7 to cb21b1e Compare December 21, 2023 11:00
@davidandreoletti
Copy link
Contributor Author

davidandreoletti commented Dec 21, 2023

@BKPepe

  1. Non shairport-sync related commits removed from the PR/branch
  2. Squashed
  3. Copy-pasted description into top commit

@BKPepe
Copy link
Member

BKPepe commented Dec 21, 2023 via email

@davidandreoletti
Copy link
Contributor Author

@BKPepe Both commits have now the relevant description info.

@BKPepe
Copy link
Member

BKPepe commented Dec 21, 2023

Are you sure? :) Why in the update, there is a size difference, which should be imho in 2nd commit?

@davidandreoletti
Copy link
Contributor Author

davidandreoletti commented Dec 22, 2023

there is a size difference, which should be imho in 2nd commit?

@BKPepe Commit description reported packages have been built --with/without-pipe with 4.3.2, hence logical to report the relative size difference documented in 436ae68 commit since starting with that commit, one can rebuild both packages and get the same size difference.

Had the commit description reported packages been built with 3.3.9, then the size difference would have been stated in 9c9bd57.

Also, best to highlight the size difference for the upcoming release rather than a version which will not have --with-pipe enabled

@neheb
Copy link
Contributor

neheb commented Jan 2, 2024

Needs a rebase.

@davidandreoletti davidandreoletti force-pushed the shairport_sync_4_3_2 branch 2 times, most recently from 01d9837 to e427ef6 Compare January 3, 2024 10:43
@davidandreoletti davidandreoletti changed the title shairport-sync: Update to 4.3.2 + pipe audio backend enabled shairport-sync: pipe audio backend support enabled Jan 3, 2024
@davidandreoletti
Copy link
Contributor Author

Needs a rebase.

@neheb done.

@neheb
Copy link
Contributor

neheb commented Jan 3, 2024

where's the PKG_RELEASE bump?

UCI's shairport-sync's pipe conf block is defined but has no effect without shairport-sync's pipe support

Rationale for pipe support:
- On SBC with no sound card available (built-in or external)
  and without ALSA plugins support on OpenwRT (no package)
  to reroute a stream from shairport-sync's ALSA default output
  to a PulseAudio controlled Bluetooth sink , then pipe support
  is required to enable the following flow:
  - shairport-sync -> fifo pipe -> custom_binary_to_send_stream_to_pulse_audio_BT_sink
- small bump in package size: diff: +746 bytes
  - shairport-sync-openssl_4.3.2-0_aarch64_generic.ipk without --pipe: 110806 bytes
  - shairport-sync-openssl_4.3.2-0_aarch64_generic.ipk with --pipe: 111552 bytes

Signed-off-by: David Andreoletti <[email protected]>
@davidandreoletti
Copy link
Contributor Author

davidandreoletti commented Jan 4, 2024

where's the PKG_RELEASE bump?

@neheb Now bumped again.

PS: It was present and then supersede by the rebase of 7391ca3 :-)

@davidandreoletti
Copy link
Contributor Author

@neheb Do you have a rough timeframe for this to be merged (assuming no further actions needed) ?

@BKPepe BKPepe merged commit 6d47f66 into openwrt:master Jan 5, 2024
@mikebrady
Copy link
Contributor

Many thanks for all your work on this!

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