Add analysis interval parameter to monitors settings

[manupap1]

Currently, video analysis is performed at full framerate, this can lead to high cpu load.
In order to reduce the cpu load, this PR add an analysis interval parameter to monitors settings.
Default value is 1, all images are processed.
If a greater value is set, for example 5, only images with count multiple of 5 will be processed.

[connortechnology]

Thank you for doing this. It was on my todo list. I will definitely be reviewing/merging.

[connortechnology]

So far it looks good to me.

[knight-of-ni]

In order to avoid upgrade issues with users who are running master, we need to put the update sql bits into a new file called zm_update-1.28.101.sql. We then need to bump the zm version to .101 in the usual places.

[connortechnology]

My only other comment has to do with int vs unsigned int. I keep seeing the use of int when a negative value makes no sense.

Does a negative analysis interval make any sense?

[connortechnology]

Also I kinda think this setting should be on the first page of the monitor popup...

[connortechnology]

Also: I don't think this algorithm will be effective. This makes analysis happen on every so many frames. The problem is that we have cameras now that send a variable framerate. so I have a camera that normally sends 3fps, but at times will send up to 15. I would like analysis to happen at a fixed 3fps, but recording to be 15fps.

I think this analysis fps should in fact be only analyze so many frames per second.

[newburns]

Would there be a conflict of fps?
So if you are recording at 2fps but trying to analyze 15fps, there is discrepancy there.
Not sure about any coding or how this actually works, but that was the first thought that popped in my head.

[connortechnology]

My point is that I want to be able to set it to analyze at 2fps. The code should then skip however many frames are required to get to 2fps.

Instead, if I set this to 2, it will skip every other frame.

[manupap1]

@knnniggett:

• I missed the database upgrade issue.

@connortechnology:

• About analysis interval, negative values have no sense. By the way, it is not possible to enter a negative value so unsigned int is safe and should be preferred over int.
• You are right about setting location, it should be on the first page.
• Indeed, by design, this algorithm is not able to keep a constant analysis framerate if the capturing framerate change. I will investigate but I am not sure there is a simple way to implement a framerate setting without side effects.

@newburns:

• Capturing and analyzing at different framerates is not a problem with the current algorithm because the analysing fps can not exceed the capturing fps.

[manupap1]

I implemented an analysis fps instead of the interval.
The fps control is done directly in the main loop of the program with a usleep function.
This algorithm is not compatible with the adaptive skip feature so I added some logic to disable it if the analysis fps is lower than the capturing fps.
Also, I added an additional parameter (in misc page) to deal with variable capturing framerate. The capturing framerate can be monitored periodically to update the adaptive skip status and the analysis fps.

[Linwood-F]

Does this replace the Motion Frame Skip on the misc page?

[connortechnology]

Overall this looks great.

Two things to note: the analysisfps can be null, so when loading it from the db, you need to test it for null before calling atol. So something like analysis_fps = dbrow[dbCol] ? atol(dbtow[dbCol]) : 0;

The other is the language files, you don't need to add lines to them. The ui will fall back to english if there is no entry in the other language files.

[manupap1]

@Linwood-F:
It is not my intent to replace the Motion Frame Skip option.
However I am not sure that the Motion Frame Skip option is implemented correctly.
I just did some tests with it (catpure @ 30fps, motion frame skip set to 29 in order to analyse only 1 frame per second).
As expected the CPU load is reduced, but the recorded events include way too much alarmed frames (more than 1 per second).

[manupap1]

@connortechnology:
The test would be redundant because atof returns already zero if no valid conversion can be performed (http://www.cplusplus.com/reference/cstdlib/atof/).
For the language files, I found convenient to use the updateLangs.php script to update all language files in a row.

[manupap1]

I found a bug.
The framerate is not respected for the pre event frames (test @1fps):

[Linwood-F]

@manupap1, I guess I should have worded the question differently -- could this replace the motion frame skip? Unless I have misunderstood, it seems to address the exact same issue -- skip some frames if they are coming in fast. Unlike the frame skip which is frame based, this is (to me at least) more meaningful as time based. Actually I'd prefer to see the (non) motion frame skip also be time based, as it is one less thing to adjust if you decide to change camera speed (or for a variable frame rate camera).

I'm very new to zoneminder, so take with the usual chunk of salt, but to me it's a wonderful thing if a new (and better) feature can replace an old one, and a bad idea to leave redundant features. Now if one was a lot faster (e.g. like image quality vs disk space) I get it, but if they both do the same function in (almost) the same way....

Just a thought.

[manupap1]

@Linwood-F, yes it could replace it. In fact this could replace both options (frame skip and motion frame skip) as the filtering is performed upstream.

[manupap1]

I am not sure that the issue with pre-event frames can be solved easily because the problem is deeper, we have to deal with the buffer design.

The buffer size is fixed and based on a number of captured frames (default 50).
If we need 25 pre-event frames at 1fps, we need to keep 25s of captured frames in the buffer.
If the frames are captured at 30fps, we need 750 pre-event frames in the buffer...
But most of these frames are useless as we need only 25 of them.

Maybe the best option would be to implement a separated pre-event buffer with a proper sampling.

[Linwood-F]

I have only briefly glanced at that code but not sure I understand. The buffer is used to save images for (potential) save to disk, not analysis, right? So if the non-alarm skip is big (whether by time or frames) and the alarm skip is zero or small, you are indeed going to discard most frames going into the buffer -- which is a good thing. But the analysis rate would be independent of both of those two, and be decided upon as images are being captured. not sure if the buffer is used to pass to the analysis, but even if so, it would be passed off at the current head of the list, not drawn from deep in it?

[manupap1]

@Linwood-F:
Analysis is performed against images stored in the buffer. This is a good thing when analysis is performed at full framerate because the analysis algorithm may be late and pending images are not lost.
Also, when a new event is created, pre-event images are taken out from the buffer. The problem comes from this part because the buffer may not be properly sized to keep enough images.

[Linwood-F]

Ah, thanks, sorry for the tangent.

[connortechnology]

I am going to have to write a test case and prove it, but I have solved several seg faults by not passing NULL to atoi.

[connortechnology]

yeah, according to docs, do not pass NULL to atoi or atol

[connortechnology]

Sigh, there is also this.

According to the atoi man page, it has been deprecated by strtol.

IMPLEMENTATION NOTES
The atoi() and atoi_l() functions have been deprecated by strtol() and strtol_l()
and should not be used in new code.

[manupap1]

@connortechnology
PR updated
String conversion is modified.
I implemented a separate buffer to fix the issue with pre-event frames. This buffer is not used if the analysis fps parameter is not set.
I also fixed the calculation of the analysis rate for usleep function. With current algorithm, the capturing rate framerate has to be deducted.

[connortechnology]

I'm happy to merge this, but I'll leave it to you to resolve the conflicts from other things that we have merged.

[Linwood-F]

I'm still confused whether this should eliminate the motion frame skip parameter. If this is functionally a replacement and/or better, should we not get rid of that?

[knight-of-ni]

Wow, seems like we've got several changes in the works that are modifying the dB schema, which are causing the merge conflicts.

Everything up to 1.28.103 is currently spoken for. Please bump the revision in the PR to 1.28.104.

[knight-of-ni]

Ended up bumping the version to 1.28.104 with this merge