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 audio/video enable properties #113

Merged
merged 1 commit into from
Mar 22, 2019
Merged

Add audio/video enable properties #113

merged 1 commit into from
Mar 22, 2019

Conversation

pnbruckner
Copy link
Collaborator

Add properties for determining if any audio/video streams are enabled, and corresponding setters to enable/disable audio/video. Bump version to 1.3.0.

Add properties for determining if any audio/video streams are enabled, and corresponding setters to enable/disable audio/video.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 33.478% when pulling da4f3d3 on streaming-on-off into 2872882 on master.

@pnbruckner
Copy link
Collaborator Author

pnbruckner commented Mar 16, 2019

I'd like for you to test this on whatever cameras you have. I know it works on my cameras (IP2M-841W/2.520.AC00.18.R/2017-06-29 & IP3M-943W/2.400.AC02.15.R/2017-07-31).

To test:

from amcrest import AmcrestCamera
cam = AmcrestCamera(...).camera
cam.audio_enabled
cam.audio_enabled = False
cam.audio_enabled # should return False
cam.audio_enabled = True
cam.audio_enabled # should return True

Same for video, but cam.video_enabled instead.

Oh, BTW, if a camera does not support an audio output stream (i.e., no microphone, like my IP3M-943W), there will be no errors or exceptions, the audio_enabled property will simply always return False.

Also, of course, let me know if I should change anything in the way I implemented it.

Assuming this looks ok I have some other commands I want to add, but I'll probably just commit them directly to master. I figured I'd use a PR here to give you a chance to decide if you like the direction I'm going. :)

@pnbruckner pnbruckner changed the title WIP Add audio/video enable properties Add audio/video enable properties Mar 16, 2019
@pnbruckner
Copy link
Collaborator Author

pnbruckner commented Mar 20, 2019

@tchellomello @dougsland

Hi guys. Have you had a chance to look this over? And test it? Thoughts?

The biggest question I have, besides whether or not, in general, this works on other camera models & firmware versions than I have, is if these lines are appropriate:

https://github.com/tchellomello/python-amcrest/blob/streaming-on-off/src/amcrest/video.py#L148-L155

They set the "SmartIR" mode so that the IR LEDs come on when necessary (when video is enabled), or are turned completely off (when the video stream is disabled.) Do all cameras support this feature? Or would it need to be conditional based on camera model and/or software version? Or, since the response isn't checked, does it not hurt to send it even if the camera doesn't support it?

@pnbruckner
Copy link
Collaborator Author

@tchellomello @dougsland

I'm assuming you guys are too busy to provide feedback, so I'll go ahead with this.

@pnbruckner pnbruckner merged commit 31e6982 into master Mar 22, 2019
@pnbruckner pnbruckner deleted the streaming-on-off branch March 22, 2019 13:24
@tchellomello
Copy link
Owner

@pnbruckner It was a busy week!! It's an awesome addition 🥇

You rock!

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.

3 participants