PreCapture buffer redesign and enhanced minimum_motion_frames

Description of Patch

This patch implements two major things.

1. minimum_motion_frames have now also a way of specifying of how hany frames. It is done with a new conf. option named minimum_motion_base. e.g trigger an event if there is 5 motion frames in the latest 10 frames. This helps if you have a high framerate and slow motion, before there might be so that it wasn't any motion between two frames.

2. redesign of how the precap buffer works. Now only 2 pictures are saved/sent to ffmpeg every capture loop (can be more at end of postcap as all buffered pictures must be saved/sent). This is discussed in http://www.lavrsen.dk/twiki/bin/view/Motion/PreCaptureThrottleLoad

My implementation of 2 also changes the behavior when precap buffer is used. Before it was only used in the beginning of a new event, but now it is used before all motion frames, e.g. in middle of a movie also.

How the preview shoot first is implemented is also changed. Now best and first is working in the same way.

Also added into the v4l2 code that the capabilities, palettes etc. isn't outputed at every frame capture, just at startup.

The patch also moves the alg_locate_center_size call to before overlay_fixed_mask. If done in the other way the "motion box" is always in the masked area. This can be fixed in an other way f.ex. using despeckle label function, in the motion image masked area is white, same as motion.

This release of the patch adds some text into the picture in the upper left corner saying if this is a precap, postcap, motion or motion but we haven't got minimum_motion_frames of minimum_motion_base.

There have been quite many changes, so I would be very pleased if this is tested by someone more. Just to see that everything still works as expected.

Installation of Patch

Download the patch file. If it is packed as a gz or tar.gz unpack it first. Then copy it to the motion source directory and issue the command (assuming the patch file is called filename_of_patch_file.diff)

patch < filename_of_patch_file.diff

Then re-build Motion and test the patch.

Change History of Patch

• 1.0 Initial revision
  1. 1 Fixed Preview first added an other patch file to add center motion preview selection and preview frame logging into database

Discussion and Comments

---

I only quickly looked at the code.

Impressive work and in areas that we have wanted for some time.

Can you explain a little more how the minimum_motion_frames vs minimum_motion_base works? You did not add any help text for the minimum_motion_base.

One of our real issues also with tracking may actually more lie in the fact that the reference frame is based on frames only split seconds ago. Maybe we should address both the problem you are trying to resolve here and the tracking issue by rethinking reference frame a bit. Not saying that your solution is not good. It is worth trying also in practical.

Imagine a person that walks passing a wall. In the previouos frame he was in a specific position covering a part of the wall. In the next frame he moved a bit, making a piece of wall visible while covering another part.

To Motion pixels where is now is have changed compared to the reference frame. But so has the pixels of the piese of wall that he covered before.

And to the tracking it sees the entire area as Motion.

If the reference frame had been just a few seconds older the Motion detected would be the actual person moving around. But how much older should this frame be? Should it still be an average frame calculated based on many old frames? Will false detections go up or down with this? A slow mover should be easier to detect because the reference frame is older so more pixels have changed.

I look forward to study the pre-capture throttle code.

"Now best and first is working in the same way" - can you explain what you mean with this?

"Also added into the v4l2 code that the capabilities, palettes etc. isn't outputed at every frame capture, just at startup". As I have not studied the code and am still not fully up to speed with Angels great work on V4L2, forgive this basic question - but did you consider the round robin case for this?

"moves the alg_locate_center_size call to before overlay_fixed_mask" - does this mean that Motion pixels in the masked off area are included in the locate center? That would work badly for some of my cameras where it is only and always unwanted pixels in the masked area. The idea of the mask is to ignore all Motion in that area.

The text you add in upper left may be good for debugging but not for production. Is it only visible in debug mode?

Many questions. But do not get it wrong. I am very happy to see this great contribution.

-- KennethLavrsen - 07 Aug 2007

---

If you sepcify a minimum_motion_base, then there must be motion in minumim_motion_frames of the lates minimum_motion_base frames. See it as a average. For example if you sepcify minimum_motion_base 4 and minimum_motion_frames 3 and get motion, motion, no motion, motion it tiggers an event. If you don't specify any minimum_motion_base it acts as before.

The reson why now and best was changed to work in the code in same way was that before the first was saved in the beginning of the event, and the best was copied to a temporary buffer and saved at the end of the event. Whith the changed precap code, it is not possible to save the frame when a event start. The result is the same as before, the only difference is that the frame is always saved at the end of the event.

In the v4l2 code this was always outputed into the logs:

 [7] - CAMERA [7] - video standard PAL [7] - video standard PAL-BG [7] - video standard PAL-I [7] - video standard PAL-DK [7] Supported palettes: [7] 0: GREY (8 bpp gray) [7] 1: RGBO (15 bpp RGB, le) [7] 2: RGBQ (15 bpp RGB, be) [7] 3: RGBP (16 bpp RGB, le) [7] 4: RGBR (16 bpp RGB, be) [7] 5: BGR3 (24 bpp RGB, le) [7] 6: RGB3 (24 bpp RGB, be) [7] 7: BGR4 (32 bpp RGB, le) [7] 8: RGB4 (32 bpp RGB, be) [7] 9: YUYV (4:2:2 packed, YUYV) [7] 10: UYVY (4:2:2 packed, UYVY) [7] 11: 422P (4:2:2 planar, Y-Cb-Cr) [7] 12: YU12 (4:2:0 planar, Y-Cb-Cr) [7] 13: YV12 (4:2:0 planar, Y-Cb-Cr) [7] Test palette YU12 (384x288) 

now this is only at startup.

The reason why I moved alg_locate_center is that I have configured a static mask and not using despeckle label function, the motion rectangle was always around my mask and not only the moving parts. Moved up this it works better, but as you say the rectangle can be in the masked area. I haven't studied exactly why it is in this way but looking at a motion image whith a static mask the masked area are white and also motion.

The text is visible all time just now, when/if this patch is more I can remove it and make a new patch.

-- DagErlandsson - 08 Aug 2007

---

Thank you Dag for following up this topic. I'll test your patch soon and will follow this development closely, because this is the most critical part of motion that was really due to be fixed. I guess a lot of people were waiting for this performance improvement for a long time - including me of course

-- JoergWeber - 09 Aug 2007

---

Fixed a small problem with preview frame first. Memory wasn't allocated correctly. Also added some new functionallity, I know that it isn't so good to have all this changes in one patch but this is canges I have made to work as I want in mu setup. If there is interrest in one feature in it I might be able to extract just that part.

In this patch I also added: Save preview image and log it to sql database and save all frames as jpegs in the same time. In my setup I dosn't save it to a movie, just saves all images as jpegs. I still wanted to have a preview image. To be able to do this I have added a new conf. setting, sql_log_preview. Can set to three values, on, off and old_type. on adds it to the database using a new image_type 64, old_type saves it as before using 8 (same as jpegs). e.g. to work in the same way as before this have to be specified. Off dosn't logs the preview frames into the database. I also had to add some values to the output_normal configuration, on_and_best and on_and_first to be able to tell motion that I want to have both all images and the preview image saved.

I also added a new preview select option, center. This saves the image where the motion is nearest the center of the camera and saves it as the preview.

-- DagErlandsson - 10 Aug 2007

---

Dag, I see that you have some good ideas, but please let's concentrate on the key feature first: the pre_capture redesign. I would like to see this work perfectly, before we start implementing new features. Today there is not much activity in the SVN, so that this point in time is pefect to implement such a huge change. Let's take the time to finish this one and afterwards we can add new functionalty, ok?

-- JoergWeber - 10 Aug 2007

---

In my setup I have the pre_capture design running. After that I wanted to have some other features so I added them also. I'm first working on this for my own setup to get it work as I want. After that, if someone else want to have any of my changes I can try to extract just that changes from the diff file. The problem I have is that some of the changes is in the same parts of the code so I can't have diff files that can be apllied to the svn version for just one feature. But as I wrote before if someone want to have just one feature I can try to extract just that changes. Don't take my wrong here, it isn't so i'm not willing to contribute the greate effort all of you have made to get this project this far, it is just so that I just now must concentrate me on get my own setup working as I want. An I have setup this patch just to show what I'm working on. After that we can see what parts other wants to have into the main svn code and maybe setup other patches to it so it will be easier to find it.

-- DagErlandsson - 11 Aug 2007

---

Now when my setup is running as I want, I can split all my changes into more than one patch file. I start with the precap redesign. I don't know if we/I should clean this page up to just talk about the precap redesign or if we should keep it as is? For now I attach a new patchfile for just the precap redesign.

-- DagErlandsson - 12 Aug 2007

---

Dag, thank you for this 'pure' patchfile. I just initially thought it would be easier to do things separately instead of splitting it at the end. I will look at it as soon as I can. Just one remark: I had a quick look at the patch and noticed that the codestyle doesn't really match the desired motion format. If you have a minute, could you please look at Motion Coding Standard and try to keep this format for your patch.

-- JoergWeber - 12 Aug 2007

---

I did a first test today. The patch was compiled well and motion was running fine all day long. The result looks ok in my environment at first sight. I will do more tests with some extreme values for pre_capture to make sure the frames are in the right order. Also the 'retrigger pre_capture during post_capture'-behaviour has to be studied carefully. If it passes this second test as well, I don't see any reason why I shouldn't commit it to the SVN soon.

-- JoergWeber - 13 Aug 2007

---

Final test passed. I'll commit the patch when I have had some time to fix the format issues.

-- JoergWeber - 14 Aug 2007

---

I have added this patch to SVN - r207.

-- JoergWeber - 16 Aug 2007

---

Dag was right about the alg_locate_center problem in combination with fixed masks. Changing the order as suggested will fix this problem. I'll add that to SVN later today.

-- JoergWeber - 16 Aug 2007

---

I have noticed a small differense when using this patch. It is the timestamp of the preview_first image. Before this have the timestamp at the beginning of the event, e.g. when the picture was taken. Now the timestamp is at the end of the event, e.g. when the picture are saved. This is same behavior as preview_best have, and had (it isn't changed). The timestamp can be used to name the jpeg file and is also, if uses, inserted into the database. I don't know if this is a issue? Is it?

-- DagErlandsson - 17 Aug 2007

---

I also added a new patch. PreviewCenter that adds just that part.

-- DagErlandsson - 17 Aug 2007

---

I think it is important that the preview image gets the same timestamp as the movie file. This should be the same for both 'first' and 'best' preview. If it was different before for 'first' preview images, we seem to have improved behaviour now. Did I get it right?

-- JoergWeber - 17 Aug 2007

---

Yes and No, now the timestamp of best and first is the same, but not the same as the movie.

The timestap of the movie is when the event is triggered. The timestamp of best preview is when gap ends. The old timestamp for first preview was when the event was triggered, now it is when gap ends.

I can think on somedifferent timestamp we want to have to the preview image. 1. The real time when the preview 2. Same as the movie file, e.g. when the event is triggered. Note: this time isn't the same as the first image in movie as we might also have a precapture buffer and/or a minimum_motion_frames. 3. When the event is ended. e.g. when the file is created.

I think it isn't so importent to have the same timestamp to both the movie and preview, as we also have the %C, event text, that can be used if we need someting to identify the correct preview image and movie file. This can be set so it is the time when the event is triggered.

Note: The timestamp here is what the %h, %m, %s etc. is translated to when saving a image, adding a record to the database etc. This is not the time drawn into the image, if that is enabled.

-- DagErlandsson - 17 Aug 2007

---

Ok, when I said 'timestamp' I was also talking about the data that makes the filename, when the files are saved. I am recording msmpeg4 files and best preview jpgs today. They both have the same time in their filename. This is absolutely necessary in order to easily access them from a web frontend. Both first and best preview files should always have the same timestamp as the movie. This is the only way that makes sense to me. Can we easily achieve that?

-- JoergWeber - 17 Aug 2007

---

Just looked into the code again: It's not critical, because we have the 'preview' option with the jpeg_filename parameter, that always forces the preview file to have the same (base-)name as the movie. No change is necessary.

-- JoergWeber - 17 Aug 2007

---

I have looked at the code and it is very easy to get the preview timestamp same as the movie timestamp, a one row change. I also fixed a problem when resizing pre_capture length in the config http interface. A attach a patch, precap_timestamp.diff.

-- DagErlandsson - 18 Aug 2007

---

Dag, there are some major problems with the redesign. Can you please join the motion channel on IRC for easier troubleshooting? Kenneth and Angel are also around most of the time. Motion is currently not saving any 'normal' jpegs. And maybe there is also something wrong with movies under certain config constellations.

-- JoergWeber - 20 Aug 2007

---

I have looked at this and tested some different config combinations. I found two problems.

output_all triggered one of my sanity checks.

The second one is when minimum_motion_frames=0 and pre_capture=0, this resulted in a ring buffer with one element. This wasn't handled correctly in the code. This can also be the problem that the movie isn't OK at all config constellations.

I attach a patch for this two problems. This patch also includes one old patch, that changes the pre_capture length in the http config interface didn't apply correctly.

Sorry for all problems this have made.

-- DagErlandsson - 21 Aug 2007

---

You don't have to apologize. I didn't expect such an operation to be bug-free from the first minute. We have to thank you for your great work and are ready to help you to troubleshoot any issues that may come up with the new code.

I have added your last patch to SVN. It was working for me at least. Let's see if it works for all others as well.

-- JoergWeber - 21 Aug 2007

---

I have talked to Kenneth about the preview image timestamp issue and we agreed on following: The preview images should always have the timestamp of the moment, they were captured - both, first and best (and center of course) preview shot. This may be important in the context of a surveillance system. If someone wants to have identical filenames for the picture and the movie, he can use 'preview' as jpeg_filename and motion will do exactly that.

Is that easy to implement?

-- JoergWeber - 21 Aug 2007

---

Another detail came to my mind while testing a bit: Why should we flush the buffer at the end of post_capture? Imagine a configuration, where we have a large number of pre_capture frames but no post_capture at all. When there is a smaller number of motion frames than Pre_capture frames, we have the same CPU load situation as before. Can't we just run the '2 frames per second' job until all pre_capture and motion frames are processed? The only situation, where we have to flush the buffer is, when motion has to terminate or when someone triggers a makemovie event. I don't know if this would break anything - for example when a new event is about to start during the processing of the 'old' buffer content. Anyway I would like to see the load throttling as much as it possible - without having to write another 2000 lines of code.

-- JoergWeber - 21 Aug 2007

---

I have attached a patch file that the timestamp of a preview image was set to the moment the frame was captured.

The reason I decided to flush the buffer at event end was to simplefy the code. It is not difficult to change so it continues after the event end but I can see some problems: If we don't flush the buffer in the event end, it have to be done at the beginning of an event (if there are any frames left then). It then have the same problems as today, that we might miss frames at the moment we needed them at most. We have just noticed any movment. If done at the event end, there isn't any movement just now, hopefulle it dosn't do anything if we miss some frames. The other thing is it makes the code a bit more complex as it have to handle the buffer at two different places in the code.

-- DagErlandsson - 21 Aug 2007

---

The description at the beginning of this topic says that the buffer is flushed at the end of post_capture. If it is done at the end of an event instead (= when gap ends), then we can absolutely live with it. Thanks for clarification.

-- JoergWeber - 21 Aug 2007

---

I have committed the preview timestamp patch (r213).

-- JoergWeber - 21 Aug 2007

---

Sorry again, you are right it is now at post_capture end. I will look if it possible to have it at end of event.

-- DagErlandsson - 22 Aug 2007

---

I don't know if it is right to change this much code just to try to structure the code some bit better, or if it should wait for v4. The changes I have made in the new patch, precapture_redesign_2_0_0.diff does two things. 1. Moves the purge of the buffer to end of event. 2. Moves some information out from context struct into a image_context struct. All images in the buffer have one of this e.g. the buffer is a linked list of it, and here is diffs etc. stored. This simplifies the code at some parts as the code dosn't have to put this info into the "main" context when saving images etc, so all %x is generated correctly in the on_event_xxx commands. This isn't the final patch for thi, I just wanted to show how it might look and have a discussion if this is the "right" way for the code.

Joerg: It is possible to move the purge to the event end without much changing in the "old" redesign. just split out the save/send parts out of motion_detect into an new function and add a "status" variable to the buffer. What way do you think is correct?

-- DagErlandsson - 24 Aug 2007

---

Dag, first of all: v4 does not exist today. It's just vision. Many people have contributed to the motion project and everytime I have added a new feature, I just tried to fit it into the existing code without changing too much of it. I did that with respect to the poeple who actually did it (which is quite silly) and because I'm a lazy guy of course. We are about to do a major redesign of the whole flow. Every single feature will remain (mostly) untouched, but the way they are executed and the way they interact is about to change a lot. I would suggest that you feel free to change as much code as it makes sense. Anything that makes things easier, more readable, faster or even just better looking makes sense during this project. I just think that you have to explain how things are working in detail now, when you are done.

The remark about flushing the buffer at the end of an event: Well, this is what we have agreed, right? Just go ahead. I'm not going to test the code yet, when it is not stable enough to run unattended. Take your time and drop a note, when it's worth testing again. And thanks a lot again for taking this much time to improve Motion so much.

-- JoergWeber - 24 Aug 2007

---

And I forgot to say that the struct makes sense. It looks ok the way you are doing this.

-- JoergWeber - 25 Aug 2007

---

Here is a new patch, precap_redesign_v2_0_0.diff. I'm running it in my setup of 8 cameras.

It was a lot of small changes in the code as I have moved the picture information from the global context struct into an own image struct. This results in code that reads f.ex. diffs have to be changed. The label data isn't saved with the image, it uses more memory than the image itself.

The main idea in this restructure is, to simplify the code and makes fewer misstakes, is to move picture dependent data (diffs, timestamp etc.) is moved out of the context structs. One pointer, current_image, to current picture is still in the context. This points to the newly captured image and is used in all calls that does any calculation / changes to the picture. When we are going to capture a new image, current_image is set to the oldest image in the precapture ringbuffer and all data is set directly using this pointer. The motion_loop then sets some flags in the image struct telling what type of image it is, precapture, postcapture if there is motion etc. It is also flagged if it is going to be saved / sent to move or not. In the motion_loop there is a call to process_image_ring() that takes the images from the ring buffer and saves / send to movie.

When process_image_ring() sends / saves the image it first sets the current_image to the "old" image that is in the buffer, and restores the original current_image when done. The reason for that is that some code uses the current_image pointer to get values, most often the timestamp. I first wanted to change the img or timestamp argument in the event function, but it was difficult to get that information into all places it needed in sub calls. There was also problems when we want to save the motion image as it dosn't have a image_data structure.

Some comments: motion_detected is now only called when there is a detected motion and we are "triggered" (had motion in at least minimum_motion_frames). Post capture is now handled by process_image_ring(), motion_loop marks the images to save them. Pre capture is done by motion_loop, when it triggers it loops all images in ring buffer and maks all to be saved. Motion frames / web cam is handled by motion_detected(). As I don't save label data with the image, the %i, j, k, l, Q shouldn't be used in the ON_XXX commands and in filenames. It is OK to be used to enter text into the image.

Questions: vid_do_autobright() uses the first image in the precapture ring to do auto brightness calculations on. Is that correct? Shouldn't it be the latest captured? Or the oldest? I think the first in ring isn't correct as you never know how old it is.

Should it be possible to use %i, j, k, l, Q in ON_xxx commands also? It is possible to add without a lot of memory as it is only simple values.

-- DagErlandsson - 27 Aug 2007

---

A lot of changes and a lot of questions... I'll have a look and maybe we can have a quick chat about it some afternoon this week, when I had the chance to look at the code.

-- JoergWeber - 27 Aug 2007

---

There is still a problem with the new code: Some frames are missing. I was using minimum_motion_frames 1, pre_capture 4 and post_capture 6. Gap was 10 and the framerate was 2. For better visibility, I have added a frame counter in the upper left corner. It is inserted into the virgin image, directly after it is captured - before anything else is processed. The sequence is right, but there are some frames missing in between and also the post_capture at the end is incomplete. Can you please have a look?

Regarding the questions: The autobright algo was there before pre_capture was introduced, so it's most likely an undetected error, that you have just discovered. Please change it to always use the latest frame. The conversion specifiers have to be stored with the images. We would loose functionality, if we don't store it. Of course it doesn't make sense to store all label data, but at least the result of all the label magic should be available at the time the image is saved.

-- JoergWeber - 28 Aug 2007

---

There is also a memory allocation problem, when terminating motion with ctrl-c from non-daemon mode:

[1] Thread exiting [0] httpd - Finishing: Success [0] httpd Closing [0] Motion terminating Speicherzugriffsfehler

-- JoergWeber - 28 Aug 2007

---

Found the fault, rewrote the ring buffer out code. I don't know what I have done there.... Also fixed the mem. problem and the autobright now uses virgin image. I haven't yet added the label information into the image buffer. It will come soon....

-- DagErlandsson - 29 Aug 2007

---

Here comes the label information fix. The location and total_labels into the image buffer so if %i, %J, %K, %L and %Q. So now all %xxx can be used in text, filenames and on_xxxxx commands e.g. in all places. The patchfile is precap_redesign_v2_0_2.diff.

-- DagErlandsson - 29 Aug 2007

---

That was quick. Your v2_0_2 passed my sequencing checks. I will check some more features tomorrow, but it looks pretty much ok so far. I have observed a small bug with 'locate'. It was there before. The first frame of an event that has motion (diffs above threshold) did not get a box. With this patch, this behaviour shows up everytime a continuous sequence of motion is interrupted by some non-motion frames. Maybe you can look into this if you have a minute. Noone knows the code better than you right now.

-- JoergWeber - 29 Aug 2007

---

Yes, I can look at it. Done some testing with v2_0_2 and have trouble to reproduce the problem. Is it missing in the preview, in the movie, in jpegs or all places? The reason for the question is, in the code, the labeling of first image is handled in a different way.

-- DagErlandsson - 30 Aug 2007

---

I have noticed it when watching a normal movie with locate and labeling turned on. The diff counter shows a number above threshold and the frame is counted as motion frame, but no locate box is drawn.

-- JoergWeber - 30 Aug 2007

---

Do you use minimum_motion_frames? motion_detected() isn't called until this number of frames with motion, and there is labeling done. So there is minimum_motion_frames-1 images that is above threshold and dosn't have a locate box. Is this your problem?

-- DagErlandsson - 30 Aug 2007

---

It seems to be related to that, yes. When I was missing one located frame, I had minimum_motion_frames set to 2. When I set it to 4, I am missing 3 frames with location box drawn. Can we easily change that?

I did some more test and there were no further problems. I suggest to commit the code and then finetune it.

-- JoergWeber - 30 Aug 2007

---

I canged so all images, if configured, have a locate box. The reason this was shows up every time now is that it saves the pre_capture buffer in an event also, and not only in the beginning of an event as before. the minimum_motion_frames-1 is handled in the code as a pre_cap buffer. So what happend before the redesign was that this images wasn't saved at all, except for the first ones in the event. I have commited the code, svn215, as Joerg suggested. Please feel free to test and come up with problems and I will fix them. I would guess there exists some as this was a quite big change in the code base...

-- DagErlandsson - 30 Aug 2007

---

The new code is running on my production system since this morning and I had no problems yet. Good job!

-- JoergWeber - 31 Aug 2007

---

Dag, I was playing a bit, when I noticed that we might want to initialize some more variables (like the location struct) in order not to circulate wrong information inside the buffer. Do you know an easy way to initialize the whole buffer in one shot? Maybe we can exclude the image data itself, because it will be filled anyway. Does it make sense to memcpy nulls into that area of memory in one shot?

-- JoergWeber - 08 Sep 2007

---

I have added clearing of location data in the ring buffer. It was correct, it could get "old" data when using the conversion specifiers in the pre- and postcapture images. I'm now clearing them all to 0. Checked into SVN rev. 122.

-- DagErlandsson - 09 Sep 2007

---

So what is the recommended maximum value to set for pre_capture after this redesign?

-- RomanGaufman - 06 Nov 2007

---

Since load is not a problem anymore, the amount of available ram is the next limit. In a typical environment, you may want to set pre_capture to a value that represents something like 2 or 3 seconds. (e.g. framerate = 5, pre_capture = 10, result: 2 seconds pre_capture).

-- JoergWeber - 06 Nov 2007

---

Dag, I have thought about the number of framebuffers again: You were right. It is correctly implemented right now. The delay does not shift the actual beginning of the event of course. It's just a delay that has to be buffered in addition to pre_capture.

-- JoergWeber - 07 Nov 2007

---