[Problem]: Incorrect use of revision number in dacp.c

[ejurgensen]

What happened?

Hi @mikebrady, a user posted this issue in the OwnTone repo. The core issue is the different ID's for Airplay 1 and 2 that you already fixed, but from the log that the user included I notice that there is also an issue with the dacp requests that Shairport is making, e.g.:

[2023-04-03 10:28:27] [DEBUG]     dacp: DACP request '/ctrl-int/1/playstatusupdate?revision-number=7' in worker thread 112962
[2023-04-03 10:28:27] [ WARN]     dacp: Connection to '::ffff:192.168.1.75' was closed
[2023-04-03 10:28:27] [DEBUG]     dacp: Update request: client closed connection

I could reproduce and see that Shairport's logs also has errors:

"dacp.c:295" dacp_send_command: receiving error 11: "Resource temporarily unavailable"

I checked dacp.c and I think the way revision number is handled is incorrect. It looks to me like dacp.c monitors status by regular polling, and in that case it should always use revision number 1. However, dacp.c seems to use the value returned from the server is used, but then the client must be able to do long polling. It looks like dacp.c doesn't support that.

I see some special handling for forked-daapd was at some point added that means 1 is in fact always used, but that doesn't do anything any more since the user agent is now owntone. However, I don't think that special handling should be kept. OwnTone should work like an Apple dacp server, and have confirmed that iTunes behaves the same as OwnTone.

If you're interested, I can make a PR with a proposed fix, but of course I don't want to mess up anything. So I was wondering which kind of dacp server is the main target for the monitoring in dacp.c? I suppose it's neither OwnTone nor iTunes. Then I will make sure to check how that works.

Relevant log output

No response

Configuration Information.

[development branch]

How did you install Shairport Sync?

Built from source

Check previous issues

• [X] Confirm

[mikebrady]

Thanks for the report. I'll take a look at this (and I might also take you up on your offer 😊, but let me look at it first!)

[ejurgensen]

Sounds great. Fyi I tested iTunes by connecting with Remote and then sniffing the session id with Wireshark. Using that I tested various revision numbers from command line with curl and dmap-parser:

$ curl --silent --header "Viewer-Only-Client: 1" "http://[ituneshost]:3689/ctrl-int/1/playstatusupdate?revision-number=1&session-id=12345678" | ./dmapprint

[mikebrady]

Thank you (again) for this great information! Mike

On 4 Apr 2023, at 20:30, ejurgensen @.***> wrote:

Sounds great. Fyi I tested iTunes by connecting with Remote and then sniffing the session id with Wireshark. Using that I tested various revision numbers from command line with curl and dmap-parser https://github.com/mattstevens/dmap-parser:

$ curl --silent --header "Viewer-Only-Client: 1" "http://[ituneshost]:3689/ctrl-int/1/playstatusupdate?revision-number=1&session-id=12345678" | ./dmapprint — Reply to this email directly, view it on GitHub https://github.com/mikebrady/shairport-sync/issues/1658#issuecomment-1496489542, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABARPONDBAALHA5NVBJKX63W7RZGXANCNFSM6AAAAAAWSVWPQ4. You are receiving this because you were mentioned.

[mikebrady]

Hi there @ejurgensen. Apologies, but I have not forgotten this! There is rather a lot going on right now, so I'll get to it when I can.

[ejurgensen]

No problem, I know the feeling :-)

[mikebrady]

Hello again. Thanks again for your attention to this issue -- I'm just beginning to look at it now. Is it your suggestion that SPS should just use revision number 1 all the time, since if does use regular polling?

[ejurgensen]

Yes. Using 1 should also work with iTunes/Apple Music, so I also suggest removing the user-agent logic. Would of course be good to test, but I'm not sure which other target dacp servers SPS might have? Maybe HomePods?

[mikebrady]

Thanks. It can't hurt to try (famous last words). There seem to be a few other remote control protocols in use as well.

[mikebrady]

Hello again. I just pushed a quick-and-dirty change in the development branch to see if it works -- it seems to be okay for macOS Apple Music and Classic Airplay. It has no effect on iOS and doesn't (currently) apply to AirPlay 2.

If you got a chance, at your leisure, to see if it works, that would be great. If successful, the change will be made permanent.

[ejurgensen]

Yes, I'll try to test. @thundershare you reported the issue, so if you can test Shairport from the development branch it would also be great.

[ejurgensen]

I tested this now, and looks like it is working fine. No disconnects or errors.

[mikebrady]

Thanks for testing, and for your very accurate diagnosis. I’ll make the changes permanent.

[github-actions[bot]]

This issue has been inactive for 45 days so will be closed 7 days from now. To prevent this, please remove the "stale" label or post a comment.