Ptp sync callback #35
Conversation
You can now remove |
|
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. |
|
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. |
|
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. |
|
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.
|
|
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 😎
|
systemcrash
force-pushed
the
master
branch
2 times, most recently
from
b88bcc6
to
57e4a3b
Compare
This commit implements PTPv2 Slave in Python 3.
It will listen and slave to other Masters. There is no code to run as
Master yet. Everything necessary for Airplay v2. To evaluate, import:
from ap2.connections.ptp_time import PTP
At the end of do_SETUP:
if "timingProtocol" in plist:
if plist['timingProtocol'] == 'PTP':
print('PTP Startup')
mac = int((ifen[ni.AF_LINK][0]["addr"]).replace(":", ""), 16)
self.ptp_proc, self.ptp_link = PTP.spawn(mac)
At the end of do_TEARDOWN:
#PTP tear-down
self.ptp_proc.terminate()
Bit-shifting is an expensive operation in python. It's possible that
for every new byte shifted to the left, a new object is created to hold
the data.
int.from_bytes appears to be the most efficient binary unpacking method
(without requiring imports) - sometimes unpack is a bit faster:
--
data = bytes.fromhex('1b02005400000608000000000000000000000000'\
'010203040506000583bf017905fe00000000000000000000000000f8f8fe4'\
'36af8010203fffe0405060001a000080010010203fffe0405060202030405060005')
--
def test3():
correctionNanoseconds = int.from_bytes(data[8:16], byteorder='big')
print(timeit.timeit("test3()", globals=locals(), number=100000000))
>>> 26.746907015000033
--
import struct
def test2():
correctionNanoseconds = struct.unpack(">Q", data[8:16])[0]
print(timeit.timeit("test2()", globals=locals(), number=100000000) )
>>> 26.19966978500088
--
def test():
correctionNanoseconds = \
data[8] <<40|data[9] <<32|data[10]<<24| \
data[11]<<16|data[12]<< 8|data[13]
print(timeit.timeit("test()", globals=locals(), number=100000000) )
>>> 43.50181922199772
--
glmnet
force-pushed
the
ptp-sync-callback
branch
from
038621b
to
4c576f1
Compare
|
In case sb is following this, I just rebased with latest changes on master, high CPU usage is still an issue |
|
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? |
|
It uses all CPU, this is a 6 core i5 9th gen desktop It creates several processes. 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 |
|
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.
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. |
|
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: |
|
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): |
|
I'll (force) push this fix and some other updates out to my ptp branch. |
|
Try the new picks. I'm streaming audio with PTP here:
|
systemcrash
force-pushed
the
master
branch
8 times, most recently
from
35bba3c
to
52f827e
Compare
|
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 :) |
|
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. |
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.