FDH2 / UxPlay

AirPlay Unix mirroring server
GNU General Public License v3.0
1.35k stars 72 forks source link

Ignore questionable RTP packets instead of crashing #103

Closed ephemient closed 2 years ago

ephemient commented 2 years ago

Otherwise I consistently hit

raop_rtp type_c 0x56, packetlen = 8
raop_rtp audio resent: rtp=0
uxplay: /home/dlin/src/UxPlay/lib/raop_rtp.c:487: raop_rtp_thread_udp:
Assertion `result >= 0' failed.
Aborted (core dumped)

once every couple of days.

fduncanh commented 2 years ago

Interesting!

A type_c = 0x56 control packet would have to have packetlen at least 16 to contain valid re-sent audio data.

bytes 6:7 should be the sequence number as a big-endian short, bytes 8:12 should be the rtp timestamp as a big-endian int. bytes 16 and above should be the encrypted resent audio data with length packetlen - 16.

So it definitely should not be enqueued.

I'd be interested to see what the packet contains. I'll reject it for enqueuing, and make the LOGGER_INFO statement print it out as bytes, and maybe you can report any you see. They might just be corrupt packets, but could be some unknown meaningful part of the protocol. (Once I have some reports, I'll change LOGGER_INFO to LOGGER_DEBUG)

fduncanh commented 2 years ago

@ephemient

were they always packetlen = 8?

fduncanh commented 2 years ago

please test the latest github! please report any too-short data packets that are seen. Thanks!

ephemient commented 2 years ago

I've pulled and am now running dc103af6f319cb2055f2df56844e52c9db3f0b18 with -d. It may take a while to reproduce.

ephemient commented 2 years ago

And yes, all the crashes I ran into before, and all the logs I saw with this workaround, had packetlen = 8.

fduncanh commented 2 years ago

The output of the short packets will be LOGGER_INFO so doesnt need -d debug.

ephemient commented 2 years ago

@fduncanh I haven't been able to reproduce the original crash (I even added abort() back in to make sure I wasn't missing them in the logs). But I did pull master@ 4491175e452d006d7e8ada387eba88a5e5b0defa, which now dies on every startup with

raop_rtp start_time = 1655184883.628801 (raop_rtp audio)

raop_rtp type_c 0x54, packetlen = 20
uxplay: /home/dlin/src/UxPlay/lib/raop_rtp.c:528: raop_rtp_thread_udp: Assertion `rtp_clock_started' failed.
Aborted (core dumped)
fduncanh commented 2 years ago

Thanks for telling me. I'll check again right now.

What system are you using?

fduncanh commented 2 years ago

Yes I saw it. will fix now (was a last minute change) oops

fduncanh commented 2 years ago

should be fixed in master @ 296915

ephemient commented 2 years ago

Thanks. For future reference, I am airplaying from a macbook pro (macos 12.4) to a thinkpad (ubuntu 22.04).

fduncanh commented 2 years ago

I think this is fixed now.

Thanks for your help!