Skip to content
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

Fix Tapo 2-way audio #1460

Merged
merged 4 commits into from
May 4, 2024
Merged

Fix Tapo 2-way audio #1460

merged 4 commits into from
May 4, 2024

Conversation

xdissent
Copy link
Contributor

@xdissent xdissent commented May 4, 2024

The tapo stream just needed another \r\n to accept the requests. Working great for me via the UI and HomeKit (after reload).

@koush
Copy link
Owner

koush commented May 4, 2024

This obviously looks like a bug at first glance, but now I'm curious how it ever worked in the first place?

@koush koush merged commit 5ce1a2b into koush:main May 4, 2024
@xdissent xdissent deleted the tapo-2way branch May 4, 2024 18:41
@koush
Copy link
Owner

koush commented May 6, 2024

@xdissent i did notice that the audio payloads are new line terminated here but not in go2rtc where I translated the code from. Not sure that’s correct.

ostensibly new line shouldn’t be necessary in either case because the content length is used for payloads, and multi part boundary is used for message delimiter.

@xdissent
Copy link
Contributor Author

xdissent commented May 6, 2024

Oh, interesting - I would have sworn both the setup request and the audio payloads had the extra crlf in go2rtc, but looking again it is only on the setup request. Doesn't seem to hurt though. I'll try without it and see if it still works.

I agree it shouldn't be necessary, but clearly there's a bug in their http implementation in one place or the other.

@xdissent
Copy link
Contributor Author

xdissent commented May 6, 2024

Yep, works fine without the crlf on the audio packets. I guess their main request handler or json content-type parser requires it though. Doesn't work without it on that request.

@koush
Copy link
Owner

koush commented May 6, 2024

chatgpt could be lying, will check the spec proper later though

image

@koush
Copy link
Owner

koush commented May 6, 2024

ahh yep

Note that the encapsulation boundary must occur at the beginning of a line, i.e., following a CRLF, and that that initial CRLF is considered to be part of the encapsulation boundary rather than part of the preceding part. The boundary must be followed immediately either by another CRLF and the header fields for the next part, or by two CRLFs, in which case there are no header fields for the next part (and it is therefore assumed to be of Content-Type text/plain).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants