Fix ffmpeg error when streaming over LTE/5G

[tsightler]

Super small fix here. I appears the tweaks to Opus encoding which disabled variable bitrate encoding caused ffmpeg to produce RTP packets which slight exceed the current maximum configured packet size of 188 (176 data bytes).

This size works over local networks because Homekit requests a packet time (frame size) of 20ms which produces packets of ~60-80 bytes. However, over LTE/5G network Homekit requests packet time of 60ms, which produces packets of 3x that size, or 180-240 bytes, which is over the hardcoded 188 byte size. This causes ffmpeg to exit unexpectedly due to RTP packet size too big error and thus streaming will not work over LTE/5G networks (I'm not convinced this is not a bug with ffmpeg, as I would have expected the packet to be fragmented, but I don't really understand the code path here).

However, simply increasing the packet size seems to prevent the ffmpeg process from crashing. This probably will not fix all issues with LTE/5G streaming, but it will fix this critical issue which keeps streaming from working in most cases.

I'm not 100% sure why this value was set to begin with. In my testing the 276 byte value seems to be sufficient, but I wonder if we need any setting here at all since packet size should largely be dictated by bitrate and frame size parameters which are already explicitly set. @dgreif perhaps you have some history here you could share?

[dgreif]

Thanks for the fix, and for digging into these really obscure bugs. The 188 packet size is something I played with a bit when I first set up HomeKit streaming, but haven't touched in a long time. I likely pulled the 188 from another streaming plugin at the time, and I remember trying larger/smaller values and going with this since it appeared to provide the smoothest stream. I think bumping it makes sense, so let's get this released.

One small favor, could you run npx changest on this branch and add a changeset which explains what's in this PR? I'd consider this a "patch" change. Once that's added, we can squash/merge and release

[tsightler]

@dgreif Sorry for the delay, but I've really been digging into the entire streaming pipeline and, now that I understand it better, as well as the way Homekit expects audio, I think there is likely a more technically correct fix that should be made, I just need some additional time to complete the testing.

[tsightler]

Hi @dgreif,

So I think this has reached the stage where it is ready to merge. I know this patch looks a bit more scary, but it's actually not as bad as it seems. Mostly it does the following:

Updated Opus processing logic
This puts the Opus pipeline in homebridge-ring on par with the current state in Scrypted, on which it appeared the original code was based. There were a few funny things in the original code, like the fact that opusRepacketizer was always initialized to produce one packet per frame, which effectively rendered it a no-op. We could just throw it away completely in that case.

The new code allocates the frame per packet based on requested frame size and also properly scales the RTP timestamp for sample rates other than 24Khz (at least Apple watch seems to request 8Khz). Also the new packetizer code properly handles the variable bit rate packets and this seems to fix the VBR encoding issues that were seen before. The HAP spec technically "requires" VBR, even though it seemed to work fine with CBR, but I've re-enabled VBR with the new code since it seems to work well now.

Updated Opus return audio
I finally tracked down the weird bug that was causing packets to be dropped and caused the super choppy return audio when Opus VBR mode was enabled (as it should be) on return audio. This turned out to be caused by an attempt to decrypt RTP payloads that were not encrypted. I'm not really sure why that was ever in there, my gut says maybe in the early days you were trying to just copy the Opus packets received from Homekit directly to Ring and decrypt them before sending them on with werift? However, currently, the return audio is always transcoded by ffmpeg, even for Opus, so it's the ffmpeg process that decrypts the inbound audio from Homekit, and it outputs unencrypted RTP which is fed locally over the socket back into the splitter where it is eventually re-encrypted by werift before being sent on it's way back to Ring over WebRTC. However, the returnAudioTranscodedSplitter still attempts to decrypt these packets, which fails at a fairly high rate. Honestly, I have no idea how it ever works, it seems like if packets are over a given size the decrypt function just silently returns the packet unchanged.

Anyway, I've removed the decrypt functions and the variables used to store the keys, since those aren't needed, and it seems to work with variable bitrate again.

Minor tweaks to AAC-ELD encoding options
Looking at the Unifi Protect homebridge plugin I saw some interesting options for AAC encoding and, after some research and trying them out, I have to say it seems to improve the audio quite a bit. It's still not as good as Opus, but I think it's mostly due to the fact that Homekit is only using 16k sampling and 24k bitrate, which is below the ideal for AAC-ELD, but these options do seem to bring it closer.

Even with these tweaks the audio with Homekit is not as good as the Ring app, but I think this is mostly due to the lower sampling and bitrate. When I compare ring-mqtt audio, which streams via RTSP, to the Ring app, it is indistinguishable (as it should be, as it's using ffmpeg with copy), but Homekit uses a sample rate of 24Khz instead of 48Khz, and a bitrate of no higher than 24Kbps vs native which is more like 64Kbps. I don't know if Homekit supports 48Khz or higher bandwidths in newer specs, but this is something I need to research more.

Increase maximum packet size
This fixes the ffmpeg crash when doing LTE/5G due to the larger Opus packets. 376 seemed completely safe and had no measurable negative impact on AAC-ELD, so I went with it. Just for reference, Scrypted uses 400, but appears to have fully deprecated AAC-ELD and is moving toward Opus 100% it seems.

Regardless, I'm hopeful bringing this code up to par with current Scrypted will help with these issues. It seems to work well for me, but there of course could still be problems I haven't uncovered.

[dgreif]

Gave these a try locally (at least the Opus side) and everything looks great! Thank you for all your hard work on this @tsightler, I think this will be a huge improvement 😁