owntone / owntone-server

Linux/FreeBSD DAAP (iTunes) and MPD audio server with support for AirPlay 1 and 2 speakers (multiroom), Apple Remote (and compatibles), Chromecast, Spotify and internet radio.
https://owntone.github.io/owntone-server
GNU General Public License v2.0
2.09k stars 237 forks source link

Forked-daapd using libevent 2.0.21? #30

Closed rbalint closed 10 years ago

rbalint commented 10 years ago

Would it be possible to make Forked-daapd work with libevent 2.0.21 with limited functionality/performance?

I will happily package and upload Forked-daapd to experimental based on libevent 2.1.4, but since libevent 2.2 most probably won't be released before Debian Jessie's freeze Forked-daapd will not be part of Debian stable for at least two years unless it could be made compatible with latest stable libevent.

For reference here is an old branch not very different from 2.0.21 which I used for testing forked-daapd's libevent compatibility: https://github.com/rbalint/libevent/commits/forked-daapd-changes-on-2.0.21

ejurgensen commented 10 years ago

Yes, I'd like to do that. You patched libevent with two features, evhttp_send_reply_chunk_with_cb() and evhttp_request_set_header_cb(), right? The latter is not used for anything essential (just for getting Shoutcast metadata), so could just be disabled. The former is used for a rescheduling callback made when streaming files to DAAP clients, and I'm not completely sure how that works. Guess it must be important since the original authors decided it was necessary to include a patched version of libevent 1? Well, I'll see what I can do.

ejurgensen commented 10 years ago

I think I've managed to do make it work with libevent 2.0. I disabled the Shoutcast metadata stuff, and I found an alternative way of implementing the streaming callback. The resulting limitations are quite small, I would think most people would never notice the difference. I might even be able to amend the Shoutcast support, because I see libav 10 has native support for it (but how far is libav 10 from Debian?).

The modifications are in this branch: https://github.com/ejurgensen/forked-daapd/tree/dev

It needs to be tested some more, I made a lot of changes. If you or anyone else is willing to try it please go ahead. When I'm more confident about it, I'll merge it, and then that will become version 21.

rbalint commented 10 years ago

It is great news! Libav 10 is already in Debian testing so forked-daapd can depend on it. I have adopted and updated the Debian package with dev's HEAD since it worked fine for me: http://packages.qa.debian.org/f/forked-daapd.html Let's see how it builds/works in unstable. :-)

rbalint commented 10 years ago

So far everything looks fine, the code built on practically every architecture. I went through the bugs opened against the old forked-daapd versions and handled the ones which I could. Could you please comment on the remaining two unclassified bug covering performance of forked-daapd? https://bugs.debian.org/cgi-bin/pkgreport.cgi?repeatmerged=no&src=forked-daapd I use only a small library for testing.

chme commented 10 years ago

Did a test on my rpi with libevent 2.0 and the dev-branch and everything is working fine.

ejurgensen commented 10 years ago

Excellent! Good to hear it is building as it should. My library is not to large either, but I've asked @freultwah about the bug status, I think he has a more respectable library.

Some suggestions for the debian stuff:

  1. Omit --enable-flac and --enable-musepack. Libav/ffmpeg has had native support at least since 0.8, so the external libraries are no longer required (so you can also drop the dependencies on libflac-dev, libtagc0-dev)
  2. I don't think the build-dependency on libtre-dev and the dependency on psmisc is needed either.
  3. I suggest you remove the libavformat and libavcodec version requirements
  4. Update the description (AirTunes is now AirPlay and I think Frontrow no longer exists)
  5. Line 37 and 38 in the postinst probably should be updated too
  6. There's an improved LSB for the init script in the INSTALL file which you can copy
  7. Is Debian going to use upstart? If so, some futher updates might be needed - you can look here for inspiration: http://gyfgafguf.dk/debian/forked-daapd.debian.tar.gz
rbalint commented 10 years ago

Thanks for the suggestions, I will merge the changes. The default init system for Jessie is going to be systemd, but upstart ,sysvinit and openrc are supported as well. Would you like to join the Debian packaging as a co-maintainer?

rbalint commented 10 years ago

BTW to celebrate the reviving of the package I wrote a blog post about switching the package to your fork: http://balintreczey.hu/blog/i-can-hear-music-again-thanks-to-forked-daapddebian/

ejurgensen commented 10 years ago

Very nice blog post, thanks! An interesting blog you have, didn't know you also did work on Debian xbmc.

I'm very happy that you have taken it upon you to be the Debian maintainer, and also for the suggestions and contributions you have provided, so I think I'll pass on becoming co-maintainer :-)

I've merged the dev branch into master now. Now I'll wait a couple of weeks and see if any bugs show up. If everything seems alright I'll tag the current code as version 21.0. Hope you'll then upload that to Debian.

About the two unclassified bugs:

ejurgensen commented 10 years ago

I've released version 21 of forked-daapd: https://github.com/ejurgensen/forked-daapd/releases/tag/21.0

Will you upload it to Debian?

BTW I think the package needs a prerm script to stop the server? Right now removing the package won't stop the server, it seems. Also when upgrading, the old server won't be stopped, so the new can't start - this produces some errors. The prerm I use for Raspbian is in the debian.tar.gz I linked to above.

rbalint commented 10 years ago

@ejurgensen Sure, I'll update it in a few days. The daemon start/stop is generated by debhelper. Please try to build the package and if you still find it missing in the binary package please tell me.

rbalint commented 10 years ago

Uploaded :-)

rbalint commented 10 years ago

For the record I have uploaded 21.0 to wheezy-backports and to experimental where it uses Libevent 2.1.4 thus it can stream to multiple clients and supports Shoutcast metadata.

ejurgensen commented 10 years ago

Thanks for the heads up, that is great news. It will be interesting to see what effect the availability of the new version will have on the popcon.

Btw, I’m planning to test despotify one of these days.

DeepFriedTwinkie commented 10 years ago

@rbalint & @ejurgensen Thanks again for all the work you all have put into this! I've attempted to build a number of times from this repo with no luck on Ubuntu server. (I get "Permission Denied" errors on the ./configure step.) So, I'd love to just install the latest version to see what happens. So (and sorry for the noob question) is it just a matter of installing one of the .deb packages using dpkg?

Once I get this going, I'll spend more time trying to figure out my build issues. (Although, @ejurgensen once mentioned this could be an Ubuntu server issue... which would take more effort to fix.)

ejurgensen commented 10 years ago

I've just tried building forked-daapd on Ubuntu Server 14, and the only issue I had was having to run "autoreconf -vi" twice before proceeding to configure (I don't know what is up with that). I did not see any permission denied errors. I built it as a normal user. It installed ok.

I don't know if installing a .deb package would work, but it will probably be difficult finding a .deb with dependencies matching your Ubuntu server.