openairplay / airplay2-receiver

AirPlay 2 Receiver - Python implementation
2.13k stars 133 forks source link

Ptp sync callback #35

Open glmnet opened 3 years ago

glmnet commented 3 years ago

Clean up and apply changes on latest master.

Works: PTP Syncs! Skip track ahead (had to modify other code)

Does not work: offset: back and forth is buggy

Needs more testing: Starting as single receiver, sometimes AP uses Realtime instead of buffered, don't know why.

TODO:

[x] test without the PTP bit flags (don't know yet how to do that) [x] clean up audio.py: since the audio callback is now used, there is no point in having two threads.

TheSpookyCat commented 3 years ago

[x] test without the PTP bit flags (don't know yet how to do that)

You can now remove Feat.Ft41_PTPClock from ap2-receiver.py at L#149. Maybe that will let you test properly?

systemcrash commented 3 years ago

I'm thinking about separating a number of the bitmasks into command line flags which can be en/disabled, but that's a separate conversation.

systemcrash commented 3 years ago

Thank you for this tasty contribution. Here is my recommendation:

A new branch here or under my repo (with the PTP module as is), and we put all of the PTP stuff in it; I don't think PTP is ready for prime-time on the master branch yet. There are a few other PTP edge cases remaining which need fixing: some Apple specific TLVs need a bit more love since their format is not yet clear. I think a few functions to change the time(line) format are necessary, so that when e.g. masters change, music continues to play smoothly.

glmnet commented 3 years ago

I'm so excited about this. I didn't know PTP was "optional" how else can sync work reliably?

Are there some docs / chat histories I'm missing?

I've put the PR here for visibility mostly, hence marked draft as this definitely might break other scenarios. I'm still learning how this all works, also I'm not testing any real-time stream either.

Regarding the missing stuff, I believe clock id is key to sync to the right clock. It is available on the "rate": 1 message, so we can reconfigure the PTP here (no need to recycle the whole thing)

This one is also CPU intensive. I'm not sure if it's a threading loop or what's going on.

systemcrash commented 3 years ago

Welcome to open source 😉

PTP is mandatory in airplay v2. I built the module by following the IEEE PTP standards and building around the data received in airplay scenarios, which have a few mandates from Apple on how it must behave. Those apple bits are unknown...

I'm also excited to get it working, but I want fewer problems later, without having to change things internally which affect behaviour later. So let's take our time with this. You seem happy to integrate, and I'm happy to work on the internals of PTP. So there's forward movement. But we must also ensure current performance remains available via the bit masks I.e. Without PTP - which works pretty well.

When things are more polished, this will be the reference implementation in python for airplay v2, I think.

systemcrash commented 3 years ago

Regarding the missing stuff, I believe clock id is key to sync to the right clock. It is available on the "rate": 1 message, so we can reconfigure the PTP here (no need to recycle the whole thing)

There are some pieces via mdns and bonjour which regulate how things work. Clock id are propagated in the signaling, but that's just a "listen for this one when it comes" piece of the puzzle. But passing fewer bits around is simpler, so the PTP operates independently of the signaling layer.

All will become more obvious with time 😎

glmnet commented 3 years ago

In case sb is following this, I just rebased with latest changes on master, high CPU usage is still an issue

systemcrash commented 3 years ago

I think it might continue to be. The best we might be able to do is to lower the sampling rate somehow. E.g. time calculations only once every fourth follow-up packet. Might be able to ignore some packet types. I checked your repo, and the PTP is not current to what I have shared in mine. I did put a few CPU aving measures in my most recent commits.

What's your platform, BTW? And what is 'high' usage for you?

glmnet commented 3 years ago

It uses all CPU, this is a 6 core i5 9th gen desktop

It creates several processes. IMHO is a threading / processing issue. Like they are talking as fast as they can (this also happens after stopping playback) image

Usage on master branch is just < 1.5%, but sync doesn't work.

So definitely something wrong with this PR only, I lack python experience to profile / debug this.

I'll try to get your changes here

systemcrash commented 3 years ago

Well, the good news is that, that doesn't sound right: with PTP on here, my MBP doesn't go over ~15%. Quad core i5.

Screenshot 2021-08-03 at 03 05 31

How to fix it... not so sure. One guess is that everything happening in user-space causes lots of context switches. The screenshot shows a column called idle-wakeups of ~60000. So yeah, threading and how we handle received packets.

systemcrash commented 3 years ago

OK - well, I did some profiling. It turns out that PTP uses almost no CPU 😄

It appears to be the the reader thread which eats up CPU. So, either I'm doing something wrong, or there is a bug in Python(!).

To determine whether this is also the case for you, comment out the lines under reader, near the end:

        # reader_p = threading.Thread(target=self.reader, args=((p_input),))
        # # reader_p.daemon = True #must be True or shutdown hangs here when in pure thread mode
        # reader_p.start()

When I run with this change, I get almost zero CPU usage. 💪

I figured this out by doing:

brew install py-spy
sudo py-spy record -o trace.html -p <pid>
systemcrash commented 3 years ago

OK - simple fix.

Change:

    def reader(self, conn):
        try:
            while True:
                if conn.poll():

to

    def reader(self, conn):
        try:
            while True:
                if conn.poll(None):
systemcrash commented 3 years ago

I'll (force) push this fix and some other updates out to my ptp branch.

systemcrash commented 3 years ago

Try the new picks. I'm streaming audio with PTP here:

Screenshot 2021-08-04 at 17 08 55

blackadar commented 1 year ago

Is there still work to be done here to get PTP working for audio sync? Looking for a good open source project to get into :)

systemcrash commented 1 year ago

Not specifically on this PR, but I keep my PTP branch up to date. It has PTP integrated, but not really doing any sync. It just shows what it would be.

PTP works as is, but it needs some more love to integrate it and sync from.

One of the not obvious yet obvious issues is that Python won't give amazing sync, but it can get you fairly close.

@glmnet had an interesting approach to integrate. Although if we can unify the RTCP and PTP methods for timesync, that'd be nice.

My Python PTP is possibly the only one I know of. It follows the IEEE spec, minus a few bits and pieces like state machines.