Event Close Mode Time not working on RECORD

[SirLouen]

Describe Your Environment

• Zoneminder: 1.35.21 (current master)
• From Source
• Ubuntu Focal

If the issue concerns a camera
N/A

Describe the bug
If I set the Function on RECORD, and set a Length, it doesn't stop the event on the given Event in case the EVENT_CLOSE_MODE is set to time. This only works on MOCORD

To Reproduce
Steps to reproduce the behaviour:

1. Go to Options, set EVENT_CLOSE_MODE to time
2. Create a Monitor with function RECORD
3. Set any time in Misc -> Event Length, for example 3600s. First event will be longer than 3600 because it rounds up to the first 3600 modulus from the timestamp (for example: at 9:00:00am 10:00:00am 11:00:00am, etc...

Expected behavior
Similarly to MOCORD, it should stop on event length if Event Close Mode is set to time.

I will be providing a patch for this

[welcome]

Thanks for opening your first issue here! Just a reminder, this forum is for Bug Reports only. Be sure to follow the issue template!

[SirLouen]

One of my solutions has been merged, but I think it should be fully acting according to the docs and I've been thinking a little forward about my solution:

https://zoneminder.readthedocs.io/en/stable/userguide/options/options_config.html

EVENT_CLOSE_MODE - When a monitor is running in a continuous recording mode (Record or Mocord) events are usually closed after a fixed period of time (the section length). However in Mocord mode it is possible that motion detection may occur near the end of a section. This option controls what happens when an alarm occurs in Mocord mode. The ‘time’ setting means that the event will be closed at the end of the section regardless of alarm activity. The ‘idle’ setting means that the event will be closed at the end of the section if there is no alarm activity occurring at the time otherwise it will be closed once the alarm is over meaning the event may end up being longer than the normal section length. The ‘alarm’ setting means that if an alarm occurs during the event, the event will be closed once the alarm is over regardless of when this occurs. This has the effect of limiting the number of alarms to one per event and the events will be shorter than the section length if an alarm has occurred.

In fact the new conditions regarding to the MOCORD option, I'm not 100% sure if they are well coded.

Theorically the 3 scenarios are

CLOSE_TIME = regardless if RECORD or MOCORD, on TIME, close. This doesn't affect the MOCORD scenario with the current conditional
CLOSE_IDLE = for this conditional, this is equivalent to TIME, for RECORD it should remind unaffected, but for MOCORD, its identical to RECORD
CLOSE_ALARM = this obviously shall not be affecting ever RECORD, but exclusively MOCORD which is based on motion/alarms

So I still believe that even the current adaption which is better, shall be improved to the scenarios given in the docs

Brief solution:

RECORD = close always on time since it's not a mode contingent to an ALARM
MOCORD = close on IDLE and on ALARM

In fact, CLOSE_IDLE is not something being currently used.
I think that the logic should be improved in the line 2070 where it only accepts the parameter CLOSE_ALARM but should be accepting all but CLOSE_TIME

This is the diagram for the MOCORD scenario

Still, I don't understand the meaning of this conditional, it's neither reflected on the docs, or at least for me, makes any logic.

timestamp->tv_sec % section_length

Maybe some organized cam admins, want to have their recordings in perfect rounded time sets, but with this logic perspective, this will apply on CLOSE_ALARM. Personally, I would recommend removing it because it only offers an unexpected behaviour

[connortechnology]

So I'm trying to wrap my head around this still. Let me address your final concerns about timestamp->tv_sec % section_length. You are correct, MANY people want their events to start and stop on nice neat lined up intervals. it is a BAD practice but they want it. Why is it a bad practice? Because ending an mp4 is a lot of work and hammers the IO to the disk, which is ok, but if ALL of your monitors hammer the disk at exactly the same time it will not work out so well. It really is much better to stagger the start/stop times of events.

[connortechnology]

Looking at the current code, we don't seem to be taking into account the current alarm status. Only whether we are recording. I think I will start by breaking the code up to make it more readable.

We may need another setting. If in record mode, we don't care about mode=ALARM. We only care about whether we are starting on a round timestamp or not. So I am going to move this into the monitor (because we might want different settings for each monitor. And I am going to turn it into two settings. One for having events start/stop on even timestamps and another for when in mocord if we will end an event if we are alarmed

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SirLouen]

What happened with this topic?
I have an installation stucked in that old version (1.24 AFAIK) with my own mod because this never progressed so I feared of moving forward on updates (and having to perma-patch)

[connortechnology]

Where we are is that all of this code has changed drastically. I have one more branch to fit in before getting back to this.
I have split the motion detecting state engine out of the code that handles recording, so it's a little easier to read and hopefully we can now make easier to understand decisions about when to start recording and when to stop. We need to move the existing fields from the general config to per-monitor, and add fields to determine if recording should start on an even second, and also whether to stop on an even second.

[connortechnology]

Wow, almost 2 years. I just recently finally got around to refactoring the code that makes the event open/close choice. The new code is much more simple and easy to understand. So I feel like we are now in a position to address this issue.