techyian / MMALSharp

C# wrapper to Broadcom's MMAL with an API to the Raspberry Pi camera.
MIT License
195 stars 33 forks source link

Improved motion perf #171

Closed MV10 closed 3 years ago

MV10 commented 3 years ago

Motion-detection performance improvements, fixes #170

MV10 commented 3 years ago

Question for you, Ian:

Currently it aborts processing as soon as a single cell diff exceeds the threshold, which makes sense. But I want to "post-process" the cell diffs, which means I want all cells processed. Would you be open to a flag of some kind to disable early exit so that I can subclass for post-processing? I'd also have to change the access of cell data fields and CheckForChanges which are currently private.

I'm thinking protected rather than public, although maybe a motion detection consumer might have a use for the cell data in response to the onDetect callback? But that's probably an edge case, and for performance reasons we'd want to keep them as fields rather than properties, which argues for protected access.

I should clarify I'm proposing this for some future PR, if I can work out proximity processing in some useful way -- unless you think public is worthwhile in which case I'll do it now.

Also, if you think this is worthwhile, are we getting into territory where a diff-specific interface might be sensible?

techyian commented 3 years ago

Would you be open to a flag of some kind to disable early exit so that I can subclass for post-processing?

I don't have any issues with this, I'd suggest adding it to the MotionConfig class. I would prefer protected access rather than opening everything up. Are you proposing making any of the methods virtual or would you do your post processing in the Apply method?

Also, if you think this is worthwhile, are we getting into territory where a diff-specific interface might be sensible?

Potentially, yes. At this stage I'm happy with how this PR is looking, I'm really impressed at how low you've got the latency so I'm going to get this merged into dev now. Due to the change in #169 where the FrameBufferCaptureHandler is now doing the motion detection, it might be worthwhile adding the interface in that branch and making the relevant changes required to call Apply on a IFrameDiffAnalyser interface (or other suitable name).

MV10 commented 3 years ago

Sounds good. I was thinking of making CheckForChanges virtual and part of the diff interface.

I think we're home all day tomorrow, I'll work on it then.

MV10 commented 3 years ago

I'm going to get this merged into dev now

If you would do the honors, sir... (Since I now have two branches impacting MMALSharp.Processing I can't easily continue the discussed changes in the other PR until this is merged... 😄)