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

[airtunes/linux] - fix airtunes/shairplay not working on linux #15102

Merged
merged 5 commits into from Jan 3, 2019

Conversation

@Memphiz
Copy link
Member

commented Dec 20, 2018

Description

With latest iOS clients airtunes did not work on linux (tested on ubuntu 18.04 here) - it just didn't start the music stream.

This PR fixes it by:

  1. bumping lib shairplay to latest master of upstream
  2. fixing PaPlayer which ignored forced current and total times when there was no stream yet
  3. Ensure that AirTunes server remembers the progress values (current time / total time) from the lib shairplay callback and forward those to the player once the player is available.

This fixes also wrong track duration after the first start of airplay (seeking in the iOS client fixed it before), by delaying the progress information until the player our available.

Motivation and Context

AirTunes non-working on linux

How Has This Been Tested?

Tested on linux ubuntu 18.04 and MacOS High Sierra with iPhone running 12.1.1.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed
@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2018

@wsnipex fyi for the bumped lib
@FernetMenta for the paplayer change
@MartijnKaijser for deciding if this can go to Leia or not.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 20, 2018

For completeness. I also tested on windows 10. There is no issue with the old lib. Technically there was no issue with the old lib on mac OS too. Only linux didn't work. But i figured if i bump depends i bump it for all depends platforms to prevent any ifdefery in there. So Windows does not need a lib bump (but also benefits from the paplayer fix ...)

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

I'm ok with it

Copy link

left a comment

xbmc/platform/linux/network/ZeroconfAvahi.cpp line 366
return false must be inside of {}

@pkerling

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

@mkreisl you can add comments to the code :-)

@mkreisl

This comment has been minimized.

Copy link

commented Dec 21, 2018

@pkerling

@mkreisl you can add comments to the code :-)

Was trying it, but it seems I'm too stupid for that

Edit: Ah, got it. clicked on the wrong place, sorry

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2018

ha good catch - and that’s why I hate missing braces - I tend to be to stupid to add them at the right spots ;)

@Memphiz Memphiz force-pushed the Memphiz:update_shairplay branch from 1e869a3 to 1b83cda Dec 21, 2018
@pkerling

This comment has been minimized.

Copy link
Member

commented Dec 21, 2018

and that’s why I hate missing braces

I'm also a strong proponent of having braces everywhere, but with the coding style Kodi has (every curly brace on new line) that does tend to take up a lot of vertical space

@pkerling pkerling dismissed their stale review Dec 21, 2018

issue solved

@MartijnKaijser MartijnKaijser added this to the Leia 18.0-rc4 milestone Dec 21, 2018
@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2018

investigating the build errors ...

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 21, 2018

Updated with a compile fix which i also proposed upstream as PR. Lets see what jenkins thinks about it now :)

Memphiz added 3 commits Dec 20, 2018
…am is available. If not - keep the requested forced times until the stream is there.
…ted. Cache the reported start, end and current time and inform the player about those times once he is playing. Fixes missing progress/total time right after starting airplaying music
…n for dns_sd.h (which is only needed when libdl is not found) - PR for this is upstream
@Memphiz Memphiz force-pushed the Memphiz:update_shairplay branch from aea0da6 to 9c1f68e Dec 22, 2018
@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 22, 2018

fixed the double semicolons ... jenkins build error for iOS-arm64 was unrelated. But it seems like master iOS ARM 64 is broken ...

@wsnipex

This comment has been minimized.

Copy link
Member

commented Dec 22, 2018

new shairplay version pushed to the nightly PPA, builds and runs in general, but I cannot test airplay support.

@chewitt

This comment has been minimized.

Copy link
Member

commented Dec 23, 2018

I'm streaming SomaFM from an iPhone 8 (iOS 12.2.1) to an Amlogic mainline LE device with this PR and the corresponding LE shairport package change in LibreELEC/LibreELEC.tv#3179 and all works fine.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Dec 23, 2018

Thx for the test :)

@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Jan 3, 2019

@Memphiz merge ready?

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jan 3, 2019

Yep

@Memphiz Memphiz merged commit cf7f07a into xbmc:master Jan 3, 2019
1 check passed
1 check passed
default You're awesome. Have a cookie
Details
@Memphiz Memphiz deleted the Memphiz:update_shairplay branch Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.