I worked on this custom component to try and get it more in line with 0.88.1+, as well as to (hopefully) get it ready to possibly be submitted as a PR to update the standard component in the (hopefully) not too distant future. (Did I give enough disclaimers?! )
Anyway, you can check out the current state of the component here on this branch:
It probably still needs some work (e.g., all my log messages don’t conform to the old style of using the % operator. Argh!) And I’d like to do some cleanup on the standard component (like get rid of the motion_detection sensor since there’s now a binary_sensor which makes more sense; also get rid of all the extra, repeated attributes on all the sensors and switches and move them to the camera entity; etc.)
But, in the meantime, if you’d like to give it a try, I’d appreciate any feedback.
Thanks again to @danny for trying to submit the changes, and basically kicking me in the ___. Still not promising anything, but hopefully…
@pnbruckner this is really great, thank you for updating everything Just a small piece of advice when you go to submit all your hard work. Remember to break things up and not to submit everything at once. The smaller the PR’s you submit the better, they will get merged and reviewed at a quicker pace. For example just adding the binary sensor (replacing the sensor) should be 1 breaking change PR. For the camera (which has an additional 500 lines of code) add the audio services in one PR and work your way down the list in additional PR’s. You will get things cleaned up and reviewed a lot quicker in my experience. Looking forward to seeing the progress make its way into the final product
I’m testing it now. Let’s get this across the line, everyone. For the documentation you can check my (now closed) PR and copy that files changes. They already approved the docs.
This is an interesting idea. I can certainly understand how it would be easier for others to review in smaller chunks. On the other hand, it could be easier on users to get all the changes at once rather than dealing with the component changing every release. Hmm.
And do I start with breaking changes, or leave them to last (when maybe reviewers get used to me submitting changes on this component)? I can see advantages and disadvantages both ways.
As far as what to do first, in my mind, that would be adding the command lock, because I think this has been a problem for people for a long time. E.g., if you go to the amcrest PyPI project, they’re still talking about weird things happening and not knowing why. I can’t say this is at the root of all the issues, but I strongly suspect it is for many. I know I was getting errors left and right when I first started using this component until I added the lock.
Anyway, thanks for the suggestion. It’s a big hill to climb no matter what, so maybe “baby steps” is a good idea!
I decided to try and move some of my changes down into the amcrest python package, since really that’s where they belong.
I started with adding an optional lock that, if supplied by the application (i.e., the HA amcrest component), will be used to serialize command/response communications with the camera. The change so far is on my fork, and I’ve tested it independently of HA. Next I’m going to modify my HA amcrest component changes to use it and test some more. Assuming that goes well I’ll go ahead and submit the change to the amcrest python package github repository.
If that change is accepted I’ll move on to moving some of the “low level” commands I added also down into the amcrest python package.
Depending on how all that goes, and how quickly those changes are published (if accepted), will determine how I move forward on the HA amcrest component changes. I’ll keep everyone posted!
After some more testing, I’ve concluded that only the snapshot commands need to be serialized. As best I can tell, multiple other commands can be running simultaneously, even while a snapshot command is also running, but only one snapshot command can be running at the same time. Furthermore, even when the snapshot command has completed, there seems to be a small time period after it in which another snapshot command, if started, can experience problems (connection issues/retries and/or errors returned by the camera.)
So, although there still is at least one fix I’d like to submit to the amcrest python package (i.e., it was “succeeding” even if the last retry of a command resulted in an error), I don’t think I should suggest adding locking at that level. It seems more appropriate (now) that it is done at the HA component level (as I had been doing.) And now I need to look into adding a minimum gap between snapshot commands at the HA component level, too.
Thanks for both your hard work. So after putting your updated components Phil, I’ve been getting a bunch of retry/connection errors in the log. They don’t appear when I use the original ones from your git. Were any other changes meant to be made?
Blockquote
Log Details (WARNING)
Fri Mar 01 2019 17:03:56 GMT-0500 (Eastern Standard Time)
Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by ‘ProtocolError(‘Connection aborted.’, RemoteDisconnected(‘Remote end closed connection without response’))’: /cgi-bin/snapshot.cgi?channel=1
How long ago was that? I just checked more changes in about 4 hours ago. This is still very much a work in progress.
Even so, I haven’t done anything yet about making sure a minimum spacing between snapshot commands is maintained. And I’m not sure I should. But from my testing I’ve noticed if two snapshot commands, even though serialized via a lock, come too close together, it’s possible to see warnings. At least the camera model/version I’m using seems to need some time after a snapshot command before it’s ready to accept another. I think that spacing is somewhere around 300 msec, again, for my particular model. If they do come too close you’ll see the warning from the urllib3 library that you saw. Sometimes there will also be an “error 401” reported from the amcrest package (amcrest.http to be precise.) But even so, it seems to work on the retry (again, at least in my situation.)
I’ve also submitted a fix to the amcrest python component. See this PR. Without this fix, if the retries are exhausted, and the last retry fails, the code was not properly handling that scenario. With this change it should. At least it does in my testing.
Ok. The main difference is now the lock is only used to serialize snapshot commands. Prior to that it was being used to serialize all commands (well, except for audio.) Some extensive testing I did today seems to imply that only the snapshot commands need to be serialized.
BTW, I have these two models:
IP3M-943W
IP2M-841W
They’re both relatively new. One of the problems I’ll have with trying to make this work for everyone is I have no way to test with all the various models and firmware versions out there.
I have two of the same models: IP2M-841 (B and W). Adding your new components in didn’t appear to stop the errors on the log as they’re still coming. Also, RTSP was working in your previous update (looking at the files, I copied them over on 2/26), but now using your newest ones looks like RTSP stopped working. Not trying to pile on any issues or anything, but just wanted to report my usage if it’s at all helpful
I didn’t expect the latest changes to impact that.
So, I’ve never had any luck getting mjpeg or rtsp working. On the RPi3 I just assumed it was due to no native ffmpeg implementation. But I recently tried on an x86 VM running Linux Mint and I couldn’t get it working there either. Would you mind sharing your amcrest and ffmpeg config?
You say you copied them on 2/26. Do you know when? I made quite a few check-ins that day, the last being a little after 5 PM CST. It would be nice to know if it was just the last two check-ins (from today) was the cause, or if it was something earlier.
Also, you may have said, but what version of HA are you on?
No worries. In fact I’m indebted to you. Like I said, there’s no way I can test every combination, so the more feedback the better!
I’ve been using your custom Amcrest code (from last December) on a remote HA location still running Hassio 85. It has been working fine, including using RTSP in high res on a RPi3 The models I have are IP2M-841 and IP2M-851. The config is:
Although RTSP works, it is not at all streaming well. That is mostly due to the remote location only having at best a 1Mbps uplink. The images update very slowly when viewed remotely, so no real practical advantage over snapshot.cgi’s I guess. I’m just pointing out that RTSP does actually work on an RPi3 running Hassio which has all the ffmpeg dependencies built-in.
I have been following the progress in merging your component in this thread. I sure hope it succeeds!
Yes, this refreshes my memory. It’s been a while since I tried to use RTSP on the RPi3.
The first problem I ran into was that I’m not using the standard RTSP port on my cameras. This is due to how the cameras work with some other apps, and the fact that I wanted to be able to access them from outside my LAN. If I port forwarded the standard ports to different external ports, these other apps wouldn’t work, because they would be told (by the camera) to use the standard RTSP port (because it didn’t know about the port forwarding), and then the app running outside the LAN would use the wrong port. So I had to configure each camera to use a different set of ports, and then these were forwarded as is.
The next problem was that the amcrest package had no provisions to specify a non-standard RTSP port number. I solved this problem by modifying the amcrest package to use a hard-coded port number for RTSP (as a temporary work-around.) Then it would at least work.
Yes, this was the next problem. I attributed it to the RPi3 ffmpeg install that was not optimized to use hardware acceleration. When I looked into how to get that to work on an RPi3, it appeared to be way more work than I was willing to attempt at the time.
So, due to these issues, I gave up and just stuck with snaphot streaming mode.
Yeah, the ffmpeg docs do say you have to roll your own to get HW acceleration on RPi3. What isn’t clear to me is if the devs have already done this for Hassio on RPi3 or not. They just say “For Hassio, the requirements have already been fulfilled”.
There was a 2 year old H.264 video feature request (with 21 votes) that indicated that the ffmpeg component was converting to the inefficient MJPEG stream for displaying on the front-end. I’m not sure that’s still the case.
Looking at the ffmpeg source now, and it is being actively supported by @awarecan and was updated a month ago to fix a ffmpeg v4 streaming issue.
In any event, attempting to get live streaming from multiple cameras on an RPi3 with or without HW acceleration over a less than 1Mbit uplink is pointless, and wasn’t really my objective for security monitoring anyway.
FWIW, I was talking about the ffmpeg program, not the HA ffmpeg component (which uses the ffmpeg program) or the ffmpeg camera platform (which uses the other two.)
This new video stream component sounds fantastic, especially being able to stream not only across any major browser, but to Chromecast devices as well. Wow, that will sure be useful!
Just have to hope that Dahua and Amcrest components are now, or will be soon, compatible. I can assist with testing and will follow this PR.
FYI, some fixes and improvements have already been added to the PyPI amcrest package and released as version 1.2.4. Also PR #21664 has been submitted that bumps the package version accordingly, and adds a thread lock to the camera_image method (which calls the amcrest package’s snapshot command) of the AmcrestCam class in homeassistant/components/amcrest/camera.py.
Assuming that gets accepted I’ll work on another incremental, small change.
That’s great news (and great work!). I can see from all the back and forth that it hasn’t been easy. I could not tell though if merge will make it into version 89.0, including being available as a Hassio Amcrest 1.2.4 add-on. Do you know?
)
Just a small piece of advice when you go to submit all your hard work. Remember to break things up and not to submit everything at once. The smaller the PR’s you submit the better, they will get merged and reviewed at a quicker pace. For example just adding the binary sensor (replacing the sensor) should be 1 breaking change PR. For the camera (which has an additional 500 lines of code) add the audio services in one PR and work your way down the list in additional PR’s. You will get things cleaned up and reviewed a lot quicker in my experience. Looking forward to seeing the progress make its way into the final product 
