Add `rtsptowebrtc` integration #59660

allenporter · 2021-11-14T00:50:59Z

Proposed change

Add a new integration rtsptowebrtc which is an integration similar to stream, that supports other integrations. Unlike stream, however, this relies on an external server for decoding the stream contents and the integration itself acts as a passthrough for initiating the stream only. This effectively pulls the custom component https://github.com/AlexxIT/WebRTC into core. See home-assistant/architecture#658 for detailed proposal. This is the initial integration that supports a config flow based configuration for an external RTSPtoWebRTC server.

The rtsptowebrtc integration. does not expose entities or devices itself, but instead offers API methods for other integrations to call: Namely, given an sdp offer and an RTSP URL, get back an sdp answer. This is integration can easily interoperate with the API added as part of home-assistant/architecture#640 used by nest battery cameras. There is also an additional method for determining if a stream source is supported based on checking its prefix.

There are other possible future additions that can happen in followup PRs, depending on additional feedback from the architecture discussion:

Can add support for discovery, for an add-on
Can be added into camera as proposed in the architecture discussion, if we think that is useful
If camera integration is not desired, this can be used by individual camera implementations like nest that could offer webrtc support for older cameras.
Improved fallback between camera stream types to gracefully handle user error or misconfiguration

Integration has 100% test coverage, and static typing support from the start.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Add documentation for the rtsp_to_webrtc integration home-assistant.io#20306

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

allenporter · 2021-11-14T19:31:38Z

One other thought I had was to make this even more independent of RTSPtoWebRTC so that it could be used with other servers. (e.g. is it possible to configure this component to use Janus as your rtsp to webrtc system? I haven't looked at that yet, but could structure the code to make it a little more clear this could be possible)

allenporter · 2021-11-14T19:40:16Z

homeassistant/components/webrtc/config_flow.py

+            return self.async_abort(reason="single_instance_allowed")
+
+        if user_input is not None:
+            await self.async_set_unique_id(DOMAIN)


One other thought here is to do some kind of url validation or health checking of this backend at this point when not using discovery.

homeassistant/components/rtsptowebrtc/config_flow.py

homeassistant/components/rtsptowebrtc/strings.json

homeassistant/components/rtsptowebrtc/translations/en.json

bdraco · 2021-11-24T16:59:33Z

Are there plans to make an addon that runs the https://github.com/deepch/RTSPtoWebRTC server? Right now its a big ask for users to figure out how to build/install it

bdraco · 2021-11-24T17:02:09Z

I think it would be helpful to show the user where the server they configured to use is at?

bdraco · 2021-11-24T17:02:55Z

I shutdown the server and there was no feedback in the UI that it wasn't online

allenporter · 2021-11-24T17:22:28Z

Are there plans to make an addon that runs the https://github.com/deepch/RTSPtoWebRTC server? Right now its a big ask for users to figure out how to build/install it

Yes, that is definitely part of the plan. My assumption is we can add discovery for it as well to make this nice and simple to setup.

I shutdown the server and there was no feedback in the UI that it wasn't online

Great point, I would love your advice here. Here are my current thoughts:

I just added a heartbeat method on the Client library that gets a 200 or raises an exception
I was considering using in the config flow to validate the url
Was thinking about suing during integration setup to verify the server is alive
Does it make sense to expose any information after startup? e.g. background poll. Or expose a device for the server, entity or system health integration? I am not sure what the best practice is here

bdraco · 2021-11-24T17:29:49Z

I was considering using in the config flow to validate the url

Maybe a single connect and make sure the /stream url is working, otherwise raise ConfigEntryNotReady with a human readable string so they can figure out whats broken?

allenporter · 2021-11-24T17:33:03Z

I was considering using in the config flow to validate the url

Maybe a single connect and make sure the /stream url is working, otherwise raise ConfigEntryNotReady with a human readable string so they can figure out whats broken?

I am proposing to hit /static instead just to get a 200 back:
https://github.com/allenporter/rtsp-to-webrtc-client/blob/8414d9234d3f7817bab7753eef6b5c6c27351e77/rtsp_to_webrtc/client.py#L54
thinking that will reduce spam on the server side if missing url parameters.

We have a couple exception types: "Can't talk to the server" vs "server returned a response that was a failure". etc

allenporter · 2021-11-24T19:39:48Z

I think it would be helpful to show the user where the server they configured to use is at?

I've set the url as the config entry title to address this.

allenporter · 2021-11-24T19:40:19Z

I was considering using in the config flow to validate the url

Maybe a single connect and make sure the /stream url is working, otherwise raise ConfigEntryNotReady with a human readable string so they can figure out whats broken?

I am proposing to hit /static instead just to get a 200 back: https://github.com/allenporter/rtsp-to-webrtc-client/blob/8414d9234d3f7817bab7753eef6b5c6c27351e77/rtsp_to_webrtc/client.py#L54 thinking that will reduce spam on the server side if missing url parameters.

We have a couple exception types: "Can't talk to the server" vs "server returned a response that was a failure". etc

I've updated this so this health check happens in the config flow and on startup. There are no ongoing checks after startup however.

bdraco · 2021-11-24T19:41:28Z

There are no ongoing checks after startup however.

Seems fine to me. I think thats enough for the user to figure out what went wrong even if they have to restart

bdraco · 2021-11-24T19:49:16Z

[GIN] 2021/11/24 - 13:48:32 | 200 |         165µs |       127.0.0.1 | GET      "/static/"
[GIN] 2021/11/24 - 13:48:32 | 200 |     160.083µs |       127.0.0.1 | GET      "/static/"

checks seem to work ok

homeassistant/components/rtsptowebrtc/__init__.py

Add the webrtc integration. This integration proxies the signal 'offer' from the client to a RTSPtoWebRTCP server that returns an 'answer'. The RTSPtoWebRTC server is a go binary based on pion, and this is what is currently used by the WebRTC custom_component: https://github.com/AlexxIT/WebRTC https://github.com/deepch/RTSPtoWebRTC

This is to reflect naming as one type of approach to webrtc since other webrtc integrations would look very different.

Co-authored-by: J. Nick Koston <nick@koston.org>

Update rtsptowebrtc to use new API added in home-assistant#62962

homeassistant added the cla-signed label Nov 14, 2021

probot-home-assistant bot added has-tests new-integration integration: webrtc labels Nov 14, 2021

allenporter requested a review from hunterjm November 14, 2021 01:00

This was referenced Nov 14, 2021

Add documentation for the rtsp_to_webrtc integration home-assistant/home-assistant.io#20306

Merged

Integrate with standard Home Assistant WebRTC card AlexxIT/WebRTC#138

Closed

allenporter commented Nov 14, 2021

View reviewed changes

allenporter marked this pull request as draft November 22, 2021 02:32

allenporter removed the request for review from hunterjm November 22, 2021 02:32

allenporter marked this pull request as ready for review November 22, 2021 05:59

allenporter changed the title ~~Add webrtc internal integration~~ Add rtsptowebrtc integration Nov 24, 2021

bdraco reviewed Nov 24, 2021

View reviewed changes

homeassistant/components/rtsptowebrtc/config_flow.py Outdated Show resolved Hide resolved

bdraco reviewed Nov 24, 2021

View reviewed changes

homeassistant/components/rtsptowebrtc/strings.json Outdated Show resolved Hide resolved

bdraco reviewed Nov 24, 2021

View reviewed changes

homeassistant/components/rtsptowebrtc/translations/en.json Outdated Show resolved Hide resolved

allenporter force-pushed the webrtc-rtsp branch from fee61eb to 073d44f Compare November 24, 2021 18:59

allenporter requested review from bdraco and hunterjm November 24, 2021 19:40

MartinHjelmare reviewed Jan 1, 2022

View reviewed changes

homeassistant/components/rtsptowebrtc/__init__.py Outdated Show resolved Hide resolved

allenporter requested a review from MartinHjelmare January 1, 2022 17:54

allenporter mentioned this pull request Jan 1, 2022

Improve nest RTSP to WebRTC support, deferring to parent class for RTSP cameras #63182

Merged

22 tasks

allenporter and others added 21 commits January 1, 2022 10:09

Readability improvements for webrtc

d0c2117

Reach 100% test coverage

96227bd

Use rtsp-to-webrtc client library package

1792a5f

Rename webrtc to rtstptowebrtc

e0f6fb4

This is to reflect naming as one type of approach to webrtc since other webrtc integrations would look very different.

Remove internal quality scale

c3bc9bb

Bump rtsptowebrtc to support heartbeats

9ad44b1

Shorten server url variable and remove const.py

abdb1d6

Add config flow validation for RTSPtoWebRTC server

93fded3

Add RTSPtoWebRTC server health checks

380ee7d

Accept translation suggestion

66ef4cc

Apply suggestions from code review

bae5802

Co-authored-by: J. Nick Koston <nick@koston.org>

Update rtsptowebrtc to use new camera registry API

6153e6e

Update rtsptowebrtc to use new API added in home-assistant#62962

Remove unused variable

9e74d32

Fix lint and typing errors for python 3.8

66af039

Rename to rtsp_to_webrtc to follow standards

bac07b1

Use async_on_unload for unsubscribing camera webrtc provider

89b42a2

Remove unnecessary translations in config flow

662c93a

Remove unnecessary configuration setup

4af4775

Cleanup test setup and typing

abf1576

Patch integration setup to avoid starting the whole integration

b376cd7

allenporter force-pushed the webrtc-rtsp branch from e81d954 to b376cd7 Compare January 1, 2022 18:09

MartinHjelmare approved these changes Jan 1, 2022

View reviewed changes

allenporter merged commit c7b991f into home-assistant:dev Jan 1, 2022

github-actions bot locked and limited conversation to collaborators Jan 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `rtsptowebrtc` integration #59660

Add `rtsptowebrtc` integration #59660

allenporter commented Nov 14, 2021 •

edited

Loading

allenporter commented Nov 14, 2021

allenporter Nov 14, 2021

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

allenporter commented Nov 24, 2021

bdraco commented Nov 24, 2021

allenporter commented Nov 24, 2021

allenporter commented Nov 24, 2021

allenporter commented Nov 24, 2021 •

edited

Loading

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

Add rtsptowebrtc integration #59660

Add rtsptowebrtc integration #59660

Conversation

allenporter commented Nov 14, 2021 • edited Loading

Proposed change

Type of change

Additional information

Checklist

allenporter commented Nov 14, 2021

allenporter Nov 14, 2021

Choose a reason for hiding this comment

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

allenporter commented Nov 24, 2021

bdraco commented Nov 24, 2021

allenporter commented Nov 24, 2021

allenporter commented Nov 24, 2021

allenporter commented Nov 24, 2021 • edited Loading

bdraco commented Nov 24, 2021

bdraco commented Nov 24, 2021

Add `rtsptowebrtc` integration #59660

Add `rtsptowebrtc` integration #59660

allenporter commented Nov 14, 2021 •

edited

Loading

allenporter commented Nov 24, 2021 •

edited

Loading