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] - features #7093

Merged
merged 8 commits into from Jul 7, 2015

Conversation

Projects
None yet
7 participants
@Memphiz
Copy link
Member

commented May 8, 2015

This PR bumps libshairplay (used for AirTunes music streaming) to latest master which incorporates all the patches we carried along till now and the following features which i hereby add to Kodi:

  1. don't only support jpeg coverart but also png coverart
  2. Implement a subset of DACP (digital audio control protocol) for allowing kodi to control the AirTunes client (support volumeup, volumedown, next track, prev track, pause, play, stop)
  3. AirTunes is basically one endless audio stream without that much metadata inside of the stream itself (it only contains frame information like samplerate and thelike). All other metadata is delivered via callbacks outside of the stream data. ATM kodi support song metadata (name, album, artist, coverart). With this bump track metadata is also provided (track duration and current playtime). This PR adds evaluation of those progress data and injects it into the player.

The 3rd point is the one where is expect some cross winds. I tried to do it as non-intrusive as i could. I guess @FernetMenta is the one to comment on the approach i took here.

For the interested i have posted testbuilds of this PR here:

http://forum.kodi.tv/showthread.php?tid=203356&pid=2000370#pid2000370

The libshairplay maintainer works on a bigger revamp of the API which gives us audio sync information (for multi room synced playback) and maybe more. This PR is ment to be the last featureset based on the old api. The 3rd point might be overworked heavily (maybe based on elupus airtunes sync branch) so that progress injection might not stay for ever...

J* material - needs ppa bump of shairplay when the PR is approved and merged...

Memphiz added some commits May 4, 2015

[depends+win32/shairplay] - bump libshairplay to current master (cont…
…ains png coverart support, dacp remote control support, progress support) - also drop all patches because there were upstreamed 100%
[network/dacp] - add subset of DACP (Digital Audio Control Protocol)
implementation for usage with airtunes clients
[IPlayer] - add methods for injecting totaltime and time - this is ne…
…eded for airtunes where we basically play an endless stream and need to inject current platime and duration of the tracks which are selected in the client
[AudioDecoder] - allow injection of TotalTime into the decoder - need…
…ed for decoders which don't have duration information (like with airtunes where the duration is set from the outside - not through the stream data)
[PaPlayer] - allow to set time and totaltime from the outside - used …
…for injecting the time/totaltime metadata during airtunes streaming

@Memphiz Memphiz force-pushed the Memphiz:airtunes_features branch from 82f5c70 to d511283 Jul 6, 2015

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2015

As this was in the millhouse OE test builds for some months now i tend to merge this soonish. @wsnipex are you around for the needed ppa bump?

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2015

OE will also need to bump the libshairplay package to 0f41ade once this PR is merged.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2015

How do we handle those normally? OE pulls from kodi - realises it doesn't compile and digs up the change? ;)

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2015

Probably :)

Thought I'd mention it just as a "heads up", although probably no reason for the libshairplay package bump not to proceed this pull.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2015

I will ping someone from OE once ppa is set ;)

@wsnipex

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

PPA updated, but not tested.

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jul 7, 2015

@wsnipex thx a bunch jenkins build and merge

@MartijnKaijser MartijnKaijser added this to the Isengard 16.0-alpha1 milestone Jul 7, 2015

@jenkins4kodi jenkins4kodi merged commit 6f6a8e2 into xbmc:master Jul 7, 2015

1 check was pending

default Merged build started.
Details

frace added a commit to frace/kodi-overlay that referenced this pull request Jul 7, 2015

@olafhering

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2015

I think this commit lacks a configure check to catch an old shairplay-devel.rpm package from the OS. There is one already in configure.ac, perhaps it should now check for ->audio_remote_control_id instead of ->cls?

@Memphiz

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2015

sounds like a valid point

@Memphiz Memphiz deleted the Memphiz:airtunes_features branch Jul 8, 2015

@FernetMenta

This comment has been minimized.

Copy link
Member

commented on xbmc/network/AirTunesServer.cpp in d511283 Jan 10, 2016

come on :)
GetInternal suggests that it is not supposed to be used from outside. It really needs to be protected. the way you are using it isn't thread safe anyway. please implement those methods in ApplicationPlayer.

This comment has been minimized.

Copy link
Member Author

replied Jan 10, 2016

it was public - i called it ... no problem to change it though ;)

This comment has been minimized.

Copy link
Member Author

replied Jan 10, 2016

@FernetMenta is applicationplayer ment to be accessed from outside? Accessing g_application.m_pPlayer?

This comment has been minimized.

Copy link
Member Author

replied Jan 10, 2016

i mean - is this ok then?

http://pastebin.com/bWjNczgg

This comment has been minimized.

Copy link
Member

replied Jan 10, 2016

this question is obsolete now, right?

This comment has been minimized.

Copy link
Member Author

replied Jan 10, 2016

yep

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.