-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow improve_contrast to be toggled via mqtt #3011
Allow improve_contrast to be toggled via mqtt #3011
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me and in line with what is implemented. You will also need to update the mqtt docs to include this in returns as well as showing the new topic that can be set.
Thanks! Let me know if I missed anything else. |
Sorry, user error. Didn't mean to close. |
frigate/video.py
Outdated
@@ -377,6 +378,7 @@ def receiveSignal(signalNumber, frame): | |||
objects_to_track, | |||
object_filters, | |||
detection_enabled, | |||
improve_contrast_enabled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than passing this to process_frames
, can it be passed to the motion_detector
instantiation above so you don't have to pass it around on every call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, that's actually cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the motion detection is run in a subprocess and motion_detector
is instantiated once, I'll just create a variable in the class that has an initial value based on the config entry. Then the call to detect
doesn't have to be as messy.
frigate/video.py
Outdated
@@ -491,6 +494,8 @@ def process_frames( | |||
logger.info(f"{camera_name}: frame {frame_time} is not in memory store.") | |||
continue | |||
|
|||
motion_detector.improve_contrast = improve_contrast_enabled.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you just pass improve_contrast_enabled
to the constructor for the MotionDetector
class? Then there is no need to set this on each loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I can because after the class is instantiated, the value can change (similar to the way detection_enabled
does also in process_frames
) after process creation. Right? Unless there's something I don't understand about Python classes, which could totally be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I just tried it and I guess you're right. Apologies for my lack of Python knowledge. I'll get this changed shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I think it's good now. Let me know how it looks. Thanks Blake!
frigate/video.py
Outdated
@@ -458,6 +460,7 @@ def process_frames( | |||
objects_to_track: List[str], | |||
object_filters, | |||
detection_enabled: mp.Value, | |||
improve_contrast_enabled: mp.Value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this parameter is now unused and can be removed.
frigate/video.py
Outdated
@@ -377,6 +378,7 @@ def receiveSignal(signalNumber, frame): | |||
objects_to_track, | |||
object_filters, | |||
detection_enabled, | |||
improve_contrast_enabled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be removed now. I think this should be last change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks for your patience with me!
Are we going to need to cherry-pick the improve_contrast commit or merge master back into release-0.11.0 for this to be mergeable? |
There is only one conflict in |
Just tried running and am seeing this:
because the code access config.motion.improve_contrast which doesn't exist |
#2942 introduced |
I love the
improve_contrast
setting on some of my cameras at night, but didn't want to enable the setting during the day. This PR enables toggling it via mqtt in the same way that detection is toggled:frigate/<camera name>/improve_contrast/set