-
Notifications
You must be signed in to change notification settings - Fork 204
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
crash fix, format width, power line frequency merge request #6
base: master
Are you sure you want to change the base?
Conversation
I corrected the power_line_frequency because I needed to set the default value in the conf_template. |
"allow compiling ffmpeg on non x86 platforms" |
ffmpeg.c "and" at the end and start of the line motion.c bad spelling, to "an image" jpegutils.c hsf was set but not used netcam_ftp.c type was converted to upper case but not used, the function says it takes an I or A, as since only I is passed the end result is the same webhttpd.c consolidate the timeout to the top of the file as I needed to change it for testing
I identified this while debugging, the thread was created, but hadn't yet set its state to running before the main thread checked the running variable and started another thread for the same device. That resulted in a crash. Set running in the main thread, to avoid this race condition.
On a SIGHUP restart the main thread waits to restart until all worker threads are finished executing, except webcontrol wasn't included. If it was still running (such as reading a web request), it would have a dangling context pointer after cnt_list was freed leading to a crash. This counts that thread in the running threads, as a termination webcontrol_finish variable to terminate it indepdent of the device thread, and creates a webcontrol_running to identify when it is running.
I've added some more crash fixes I found with a flaky USB cable that exercised that part of the program. This also has an option to let the user specify if frames should be duplicated or not, in my case it looks horrible duplicating frames. |
I apparently have some marginal USB hardware and motion has been crashing every couple of days in the memcpy at the end of alg.c as both cnt->imgs.ref and cnt->imgs.image_virgin were NULL pointers. This was just after the main thread watchdog timer called pthread_cancel on that thread. The problem is pthread_cancel can't possibly kill a thread running on another CPU atomically, although in this case it's a single core processor and I think pthread_cancel was releasing it from the ioctl and it would then run to completition. This modifies the code to not cleanup that content's resources until that thread is no longer running. If it runs to completition a normal restart will happen, if it doesn't the main thread will check once a second until it no longer is running, cleanup and restart, but it can't cleanup with that thread running.
As long as errno is EINTR (and that must be the case when the USB device is still there, but the transfers are failing), the thread keeps looping running the ioctl when it isn't going to succeed, so give it access to the finish variable to only loop if the thread isn't supposed to be going away, and then keep sendig SIGVTALRM to break out of the ioctl when the thread is supposed to be exiting. With this fix the webcam was no longer crashing, which let the webcam without a problem continue to stream.
cnt->current_image because a dangling pointer after image_ring_resize because it is pointing to cnt->imgs.image_ring which is reallocated in that routine. motion_loop will then store cnt->current_image in old_image which it can then read from. Reallocations are rare, once in init to size 1, then once to the final size. I apparently have a bad USB link and I was seeing a crash pointing to bad data, after that camera started, then had an error and crashed in process_image_ring(cnt, IMAGE_BUFFER_FLUSH); it hadn't yet resized to the normal ring buffer size. That got me trying valgrind with a ring buffer size limit of 1 which found this bug.
This is especially useful when picking a threshold to see how close to the threshold it was.
Allow the user to specify field widths to keep the strings from jumping around as the width changes. This makes the values easier to read.
The USB webcam I have defaults to the wrong setting causing visible flickering degrading the image quality. This allows setting the value to any of the currently available options or -1 to by default not modify the value.
This lets the user decide which video problems they would rather see. In my case a Raspberry Pi 2 with a 640x480 webcam sometimes can keep up with 10fps so I don't want to set the framerate any lower, but then sometimes it can't and with duplicate frames it looks like the video output freezes every second.
This is a five commit pull request.
bugs:
The first has some typo, fixes, and such.
The next one set the device thread running from the main thread fix I ran into while debugging, there is a potential to happen in normal execution, but not likely so I didn't add it to the changelog.
The webcontrol fix I was running into on an older slower thread, the fix is to count that thread along with the other threads so it doesn't cleanup too early.
features:
I was printing the motion location and size, but it was jumping around, so I added width support so %3K would work, and I was surprised how little code it took to do so.
The USB webcam I have defaulted to 50Hz power line frequency compensation, but I needed 60Hz to avoid scrolling bright bands, so I added that setting.