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

Add rich camera support for: Reolink Cameras #1027

Open
skynet01 opened this issue Mar 27, 2023 · 30 comments
Open

Add rich camera support for: Reolink Cameras #1027

skynet01 opened this issue Mar 27, 2023 · 30 comments
Labels
feature New feature or request

Comments

@skynet01
Copy link

skynet01 commented Mar 27, 2023

As per #884 this is a request to add media browsing support for Reolink cameras. There is an official Reolink integration https://www.home-assistant.io/integrations/reolink/ that’s used by about 14k (5.4%) people and Jinstar’s integration https://github.com/JimStar/reolink_cctv (which the official one is forked from)

Describe the solution you'd like

the media functionality works within a card just like frigate cameras. Would be great if it worked with both integrations as a considerable amount of people still use Jimstars integration

@dermotduffy
Copy link
Owner

Hi @skynet01 . Thank you for the specific request to add media browsing capabilities for Reolink cameras into this card.

Looking at the integration code for the official one, it does not appear to expose media at all (i.e. no Media browsing support in HA)? Is that right? If yes, there's nothing I can do on the card to fix that. Perhaps @starkillerOG (the codeowner for the official Reolink integration) could comment on whether there's any plan to add media browsing support there, or if I'm missing anything.

A quick look at the JimStar version suggests it does expose media (source), so having the card pull out its media would be possible.

Obviously though, given the choice, I'd rather only support the official one since it will likely have the widest reach given time.

@skynet01
Copy link
Author

I am still using the JimStar integration as it has the most features and I think it actually is more popular, as the official one just launched a month or so back. I think the official one will add media browsing as well. I asked the author I'll update this thread with his answer. Thank you for considering

@skynet01
Copy link
Author

The author confirmed its already being developed in the core integration I believe since its a fork from them Jimstar integration implementing it for the Jimstar will make it work with the other when it's released.

@starkillerOG
Copy link

There is some work beeing done on media browsing for the build-in reolink integration, see here: Question about media platform. · Issue #9 · starkillerOG/reolink_aio · GitHub 2
But currently I am too busy with other features that I am implementing and testing to work on that.
Probably it will get added at some point, but it is not my first priority.

@dermotduffy
Copy link
Owner

Thanks @starkillerOG . Should we expect that the media source in the core HA version of the integration would be a "clean" fork of the JimStar version, or might it be entirely unrelated?

[If it's going to likely be very similar, we could safely do development on this end. If you don't know, we still could, but at much greater risk we'd need to change it later]

@dermotduffy dermotduffy changed the title Support generic media library queries as events for - Reolink Cameras Add rich camera support for: Reolink Cameras Mar 30, 2023
@starkillerOG
Copy link

@dermotduffy it will probably be losely based on, but no I will build it up from the ground.
I do not like the thumbnail chaching for instance.
And there was some problem with DST time offsets that I would like to fix in the upstream library instead of the HomeAssistant code.

@dermotduffy
Copy link
Owner

@starkillerOG Thank you, will keep that in mind.

@dermotduffy dermotduffy added the votes needed Vote for this feature to encourage prioritization! label May 21, 2023
@skynet01
Copy link
Author

The native Reolink plugin now supporst media playback any chance this can now be added into the cards :)

@dermotduffy
Copy link
Owner

The native Reolink plugin now supporst media playback any chance this can now be added into the cards :)

Aha, great! Every chance, but more upvotes would make it more likely for me to prioritize this. I also plan to rename the card after the v6 release (see rename bug), and I'm hoping it can help accelerate support for other camera engines like Reolink.

@skynet01
Copy link
Author

Any update on this? Reolink integration is being used by 20,000 people (6.7%) (https://www.home-assistant.io/integrations/reolink/) , more then ONVIF , Ring or even Unifi. I just think no one know about this amazing card so people are not voting for it:

@dermotduffy dermotduffy added feature New feature or request and removed enhancement labels Sep 22, 2024
@skynet01
Copy link
Author

Ooh I see the feature tag, does it mean we may see it in the future ❤️😉

@dermotduffy
Copy link
Owner

@skynet01 Honestly, I am hoping someone with Reolink cameras might be interested in implementing this, possibly after I do the card rename and advertise it a little ... as you said above, I suspect the card is not that well known outside the Frigate world. So I'm definitely open to it, just harder for me to develop for a camera platform I do not use.

@skynet01
Copy link
Author

yeah very good point 👌

@dermotduffy dermotduffy removed the votes needed Vote for this feature to encourage prioritization! label Sep 28, 2024
@dermotduffy
Copy link
Owner

Due to @skynet01 's impressive persistence, I've started playing with this :-) I found an old WiFi Reolink camera and put an SD card in it. Here's what I have so far:

  • A Basic Frigate card instance that can look at live and fetch video clips from the Reolink camera. The card is fetching the media via the "media source interface" in HA (code), the same thing the "Media" tab in your HA instance uses.
  • Caution: I have no idea if the same card code would work with a real Reolink NVR, but I'm guessing it probably would.

I found a few major problems (these are all equally problems with the existing HA interface, but I think they are more apparent/painful in the card):

  1. Media listing is slow. My test camera has a lot of media (500/day) and fetching the media list takes about 5s, even if the card is only going to list ~50 of them. This majorly slows the card down. Potential ideas to speed it up:
    • Idea 1: Have the integration offer a breakdown by hour, instead of just day.
    • Idea 2: A much better (but more complex) solution would be if the integration offered a service call or websocket that took start/end date parameters and returned the media between start/end.
  2. Media clips don't work in Companion Apps, since the integration is returning the the self-signed https certificate on the camera. Potential solutions:
    • Idea 1: We could require the user to add a valid certificate.
    • Idea 2: This would be another usecase for a generic proxy like Develop a general purpose proxy integration #1299 .
    • Idea 3: The integration could proxy the traffic itself (like Frigate).
  3. There are no thumbnails.

Curious what @starkillerOG thinks about these problems / ideas.

@skynet01
Copy link
Author

Woo, thanks for working on this🎉

    1. I personally don't mind if its just the last 5 media items, if I need to really find something I just use the app and not HA
    1. Hmm the clips work fine in the companion app how come it works through media library? Building proxy sounds really complicated.
    1. That is annoying, maybe if users enable taking of a snapshot on motion detection those snapshots can be paired with video clips?

Sorry that this turned out to be much harder then I thought.

@dermotduffy
Copy link
Owner

I personally don't mind if its just the last 5 media items, if I need to really find something I just use the app and not HA

The issue isn't the amount requested, it's that the card must fetch a whole day to return any amount. 5 or 50 will take approximately the same amount of time to fetch (and it's too long).

Hmm the clips work fine in the companion app how come it works through media library? Building proxy sounds really complicated.

If it works for you through the media library in the companion app, the card would likely work fine for you too. Neither work for me, presumably because I'm using the default self-signed certificate my camera came with. Your camera/NVR likely either has a valid certificate, or you're accessing everything over http?

That is annoying, maybe if users enable taking of a snapshot on motion detection those snapshots can be paired with video clips?

This should be handled at the integration level in my opinion, so that both the integration and card could benefit.

@skynet01
Copy link
Author

  1. I see, maybe it can still be enabled, I have about 5 events on average daily. So maybe only some people will have slow down issues? My assumption is that people set up their cameras to only trigger when key events happen and minimize the false positives.
  2. I am using Home Assistant cloud so the main HA URL is going through https, I do use NVR for some cameras so I have some reolink stand-alone one. None of them have https / certificates setup (both NVR and Stand alone), the port is not even open and both android and ios apps work and can playback the camera recorded media through the HA companion app media library.
  3. Agree 100% about the thumbnail part, would you have to define where you are going to look for those thumbnails? Cause I don't think the media library has thumbnails either.

@dermotduffy
Copy link
Owner

I see, maybe it can still be enabled, I have about 5 events on average daily. So maybe only some people will have slow down issues? My assumption is that people set up their cameras to only trigger when key events happen and minimize the false positives.

Yeah. In general, my fear is just that for some users this sets them up for a poor experience, which they then rightfully complain about! Lets see what the integration owner says. If they are open to adding a service call to fetch between arbitrary dates, this could be much smoother.

I am using Home Assistant cloud so the main HA URL is going through https, I do use NVR for some cameras so I have some reolink stand-alone one. None of them have https / certificates setup (both NVR and Stand alone), the port is not even open and both android and ios apps work and can playback the camera recorded media through the HA companion app media library.

Huh. OK. I didn't try just using http, will try. Would have thought you'd be blocked for having mixed-content by your browser if you visit the HA Media tab (the app might not enforce this, not sure).

Agree 100% about the thumbnail part, would you have to define where you are going to look for those thumbnails? Cause I don't think the media library has thumbnails either.

If implemented in the integration itself, the HA Media Library and the card would automatically have access to those thumbnails. Of the three issues, this is probably the hardest to fix unless the camera itself supports thumbnails. The only alternative is really icky stuff like downloading media automatically, pulling frames out of it, saving them locally to use as thumbnails, etc. A lot of complexity and performance issues. In this issue the integration owner chimes in on this topic:

"Bottom line: I would suggest not adding thumbnails unless we can convince reolink to build that into their firmware such that we can simply request a thumbnail for a video from the search command (and preferably also info on the trigger for that video)."

@starkillerOG
Copy link

starkillerOG commented Oct 27, 2024

Sorry for the late response.

  1. there actually is a option to fetch the playback files for a specific start to end datetime in the upstream library, see: https://github.com/starkillerOG/reolink_aio/blob/7acf43515cf66acbf7333d93dc09e926c4312b39/reolink_aio/api.py#L4798-L4805
    So if you would be able to acces the config_entry of the reolink integration, you can then take the runtime_data which contains the host object which has api in it wich gives acces to the method above:
    config_entry.runtime_data.host.api.request_vod_files

  2. Yes I am too still strugeling with this and am still looking for a solution. Not sure yet how to solve this in a nice way.

  3. As far as I know there is no way to fetch thumbnails from the Reolink API, I really dislike trying to keep a local library of thumbnails, so as long as we are not able to get them from the Reolink API I dont think we should add thumbnails.

@dermotduffy
Copy link
Owner

@starkillerOG

  1. Noticed that! Can't access the Python from the frontend (Javascript) though. I'm thinking of either (i) adding a new service to the integration that just calls that upstream library (since service calls can now return values), or (ii) modifying the media browser in the integration to break down days into hours. Would you be okay with (i), or would you prefer (ii) or something else?

  2. Certainly a solution is to proxy the media through Home Assistant, as Frigate does. I've created a small integration and library to do this. You could use do something similar, although I'm not sure how HA core maintainers feel about such approaches outside of custom integration. Certainly for my purposes, I think this is sufficient, since I can just have the card detect when hass-web-proxy-integration is installed and proxy the Reolink media through that if so.

  3. Yeah, understood. Such a pity that there's no way to do this. I would have assumed their own apps would benefit from this too.

@dermotduffy
Copy link
Owner

@starkillerOG Thoughts on 1 above? I think the service call is the best path if you're open to it, as it won't necessary need your media browser experience to change.

Related: I can't see any way (in reolink-aio or the underlying Reolink API) to limit the search to "the first X number of records". That would be really useful if it existed, let me know if you have ideas. My only thought so far is to (e.g.) keep fetching hours/days until I get the right number of records.

@starkillerOG
Copy link

@dermotduffy sorry for the late response.

  1. adding a service for this will propably not be accepted by the HA core team reviewing, because that service would be pretty useless for the end-user, it will only be used by the frontend. Not sure about breaking down the days into hours, it will start beeing a lot of clicks to scroll through all footage, it already is a lot of clicks at the moment.

  2. I build in proxying for the media browser in this PR: Add Reolink proxy for playback home-assistant/core#133916 so this will be solved soon.

I don't think the API offers a way to only receive the first X number of records, you can only request records between to datetimes.

@dermotduffy
Copy link
Owner

dermotduffy commented Dec 29, 2024

@starkillerOG For 2, you might consider using https://github.com/dermotduffy/hass-web-proxy-lib/ which is a library for just such a purpose (it's what Frigate uses) -- might make your PR smaller and keep logic centralized for integrations that need proxy capabilities. For users of the card, 2 is already solved since they can use https://github.com/dermotduffy/hass-web-proxy-integration (which uses the library I linked above). It will still be better to have this built into HA though, so looking forward to your PR!

For 1, yes, but honestly all service calls that return JSON require the user to know the format and process it. Wondering is this actually any different?

@starkillerOG
Copy link

@dermotduffy regarding 2, In that case I would suggest you try to get this proxying capabality in HomeAssistant core (maybe as a helper), not as a custom integration.

regarding 1, that is why i never implement a service which responds with JSON and I don't think there are many of such services.

@dermotduffy
Copy link
Owner

@dermotduffy regarding 2, In that case I would suggest you try to get this proxying capabality in HomeAssistant core (maybe as a helper), not as a custom integration.

There's a library, and SEPARATELY a custom integration which uses that library. Using the library in core is no different than the way you use reolink-aio in core.

@dermotduffy
Copy link
Owner

regarding 1, that is why i never implement a service which responds with JSON and I don't think there are many of such services.

I think the key question is whether you'd support such a service as code owner for the reolink integration, were I to implement it. I agree it would also need the core HA team to accept such a service, but not much point in spending time on it if you were personally against it for some reason.

@starkillerOG
Copy link

Aaah did not see it was a pypi library, thanks for the info.

@starkillerOG
Copy link

@dermotduffy No I don't have anything against such a service.
I just don't want to get into the argument with HA core about if this is or is not a good service to add. But If you are willing to, I would say go for it.

If you make the PR, I will review it and approve it.
However I will ask for a second opinion from the HA core team to ask if such a service is acceptable before merging it.

I do see one privacy/security consern: I don't like the service to return the plain text password of the Reolink camera/NVR.
For most reolink devices a token can be used for the playback which is only valid for about 15 minutes, so I am fine with that.
But for NVRs currently you need the plain text password in the playback URL.

@dermotduffy
Copy link
Owner

If you make the PR, I will review it and approve it.
However I will ask for a second opinion from the HA core team to ask if such a service is acceptable before merging it.

Of course, makes complete sense, thanks. I might wait until someone actually complains about the performance of the existing implementation before I work on it though, lets see.

I do see one privacy/security consern: I don't like the service to return the plain text password of the Reolink camera/NVR.

Yes, definitely would not like to do that. In practice though, I was assuming it could just return the media browser identifier, and ask HA to resolve that as it already does with all the media browser calls. All the caller of this new service caller would need is that ID, which is whatever the integration already returns. The existing integration would do the rest as it does today to translate that into a useful URL for your browser, i.e. the security would be no worse than it already is.

@starkillerOG
Copy link

You are probably right, ideed you don't need the actual URL.

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

No branches or pull requests

3 participants