-
Notifications
You must be signed in to change notification settings - Fork 540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds two way audio and support for Abode IOTA via HomeKit #1429
base: master
Are you sure you want to change the base?
Conversation
…th values would fail, example is Abode IOTA gives 0xff0x00 in between other TLV8's
I'm not sure your TLV changes don't break anything. |
I’m not either lol but I did run through pairing which has TLVs larger than 255 bytes and that worked fine and I’m pretty familiar with HAP as I originally patched the PDU fragmentation issue that prevented certain BLE devices from functioning properly with the lib so I am fairly confident this change is solid. |
Oh, great. I wrote that code completely blind. Just on a guess as to how it should work. |
@AlexxIT I'm looking more into the HomeKit side and wanted to get your thoughts on some things. My camera has two instances of the CameraRTPStreamManagement service and there actually is a difference in the video and audio formats offered on them. My thought is it would be nice to be able to specify the iid in the configuration to force it to use the specific service as their streaming statuses do appear to accurately reflect which one is in use. The other thing I was looking at is allowing the user to specify the max width and max height to try to cap out the highest negotiated resolution. The idea with this would be for use with Frigate where I may want one stream to be lower resolution for detect and a higher resolution for record. The last thing I was looking at is two way audio to the homekit device. It looks like there's a handful of scenarios to differentiate around it
|
I'm also interested in two-way audio. I haven't tested it yet, but do native homekit cameras with two-way audio proxied from go2rtc to homekit still have a working talkback button? In the case of non-native homekit cameras with two-way audio, like a tapo camera, there is no talkback button, right? |
|
I’ll try to get that payload for you. in the meantime I’ve been trying to get two way audio going and I am hitting some walls that I’m not sure how to properly address. The scenario I’m targeting is getting it to work from webrtc to the HomeKit device. The problem is the protocol does not even support opus/48000/2 and the non standard aac won’t work either. The best I’ve been able to do is add opus/48000/2 to the producer manually and get the pipelining to actually route the audio packets to me, but then I need to transcode and the only codecs I think can actually be supported are opus formats unless you’ve compiled ffmpeg with fdk_aac. The other route i tried was to just add support for opus/16000/1 (what my camera supports) to the webrtc consumer and I was able to get the pipelining setup correctly that way, but haven’t been able to get the rtp packets to actually land in the sender’s handler. It seems like from a compatibility position that what I really need is a layer between similar to how ffmpeg can be used to convert audio from the device to the browser in a supported format, but in this case it needs to go the other way, from browser to the device. Curious what your thoughts are on the best way to handle this. |
Apple uses non standard OPUS format. You can't use it anywhere without fixing. |
Ah, I thought it was just the aac format that was non-standard. |
Maybe this will helps you https://github.com/AlexxIT/go2rtc/blob/master/pkg/opus/homekit.go |
Here's the json payload you wanted, this thing has a ton of accessories connected so I cleaned those out for brevity. If you want to see those as well then just lmk and I can send them
|
I have two way audio working now from webrtc to the camera. The way I did this is to just slip opus/48000/2 as a sendonly in the producer and implemented AddTrack in the producer so it will match up and basically just slam the packet as is at the audioSession. The only thing I did have to do is set the PayloadType to 110 and swap the SSRC on the packet with the one that we told HomeKit about when exchanging endpoints. I didn't see it until I did a packet capture, but despite setting the PayloadType and SSRC in the handler, they were not set on the wire because inside WriteRTP there's....
I changed that to just copy it off the packet, but my question really is were those taken off the session intentionally or was that just not a concern previously? |
I don't understand what part of the code you are referring to. |
pkg/srtp/session.go
Outdated
@@ -83,10 +83,10 @@ func (s *Session) WriteRTP(packet *rtp.Packet) (int, error) { | |||
Header: rtp.Header{ | |||
Version: 2, | |||
Marker: packet.Marker, | |||
PayloadType: s.PayloadType, | |||
PayloadType: packet.PayloadType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line and a few lines down is what was preventing me from changing the payload type and ssrc on the rtp packets
We set these values when we set up the connection. I don't see why they need to be changed. go2rtc/pkg/homekit/consumer.go Lines 82 to 127 in 5cafc05
|
Ah, it's because the HomeKit producer doesn't call that, from what I see that's only in play when your running the HomeKit module to proxy a device. I'll take a closer look and see if I can bring the producer inline. Does this method look better? |
… ensures that RTCPInterval is set correctly as well
Fixes #1295
This adds the ability to discover the Abode IOTA and pair it over HomeKit. It also fixes the TLV8 parsing that would make this particular camera fail to playback due to it producing 0xff, 0x00 in between it's TLV's. I updated the TLV8 parsing to handle scenarios like this generically because in this case 0xff is type 255 with length 0 so you can just handle them like normal 0 length TLV's and move on.