Add Reolink proxy for playback #133916
Add Reolink proxy for playback #133916
Conversation
|
Already tested this code and it works. |
|
|
|
A alternative to this PR would be to use hass-web-proxy-lib from @dermotduffy for the person reviewing this PR, I would like to get your opinion on what's the best approach? |
|
|
Can we create a generic solution that can be used by all integrations that want to proxy something through HA? |
| host.api.nvr_name, | ||
| ) | ||
|
|
||
| reolink_response.release() |
There was a problem hiding this comment.
Should it be in a finally block?
There was a problem hiding this comment.
I don't think it is nessesary.
I dont even know if the release() is actually needed, I think aiohttp will clean it up by itself.
But since it is a rather large payload (few Mb) I think it is nice to release it as soon as possible to free resources.
Just to clarify: The external library already offers this, since any integration (core or custom) could use that library to proxy anything (media or not). It would certainly be better if something like this was "baked in", though.
I can imagine many ways something like this could run into "principled disagreements" (e.g. "Home Assistant shouldn't proxy anything", "Home Assistant shouldn't offer unauthenticated proxying", "Home Assistant should insist on valid certificates when it proxies", etc), that would ideally be settled before development time is spent towards adding this functionality to Core. Since I wrote the external library, perhaps I should start an architecture discussion on offering this solution "baked in"? This might let us discuss the general, without blocking this Reolink specific work from @starkillerOG . |
|
I agree @dermotduffy |
|
@edenhaus all review feedback has been processed. |
|
@starkillerOG - Thank you for your work on this. This PR sought to address an issue I encountered for the first time today, so I was happy to see this. I just upgraded my instance to 2025.1.0 to test this out, but it doesn't seem to be working for me. I am attempting to play media from a PoE Doorbell that is saving recordings to a local SD card and I typically access my HA instance via an NGINX reverse proxy using SSL. My doorbell is on a separate VLAN from my HA instance, but my HA instance is allowed to access the camera VLAN. Prior to the update, I was able to view clips in the media browser if I accessed my HA instance directly via its LAN IP/port, but playback did not work when accessing HA externally through my reverse proxy. I (perhaps incorrectly) attributed the discrepancy to the issues mentioned in this PR. After the update to 2025.1.0, playback does not work at all. I tested several different browsers and cleared cache. I also took a look at the network log in Chrome dev tools and did see 404 errors when I try to play a recording. Is there some new configuration needed to make this work or a specific combination of settings that allows it to work? Please let me know if there's any information that might steer me in the right direction. I'm happy to open an issue for this as well if you'd prefer. |
|
@maniacaris you are right, I checked and indeed this PR introduced a small bug wich now broke playback. A fix is here: #134652. This also gives a explanation of what is happening. Should be fixed tommorow with HA 2025.1.1 |
|
@maniacaris HA 2025.1.1 is now available, could you let me know if playback now fully works for you? |
Breaking change
Proposed change
Instead of directly supplying the local URL to the playback file (mp4), proxy the mp4 through HomeAssistant.
This ensures playback is also possible when not on the local network since the playback file URL is a local URL using the IP of the camera on the local network.
This also solves the issues with invalid SSL certificates, since the clients will now see the SSL certificate of HomeAssistant instead of the problamatic Reolink camera self-signed SSL certificates. When requesting the mp4 we ignore the SSL check properly.
For the Home Hub the Download command instead of Playback command will be used, restoring the ability to playback video files, which broke with the most recent Home Hub firmware.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: