From cf88e025ab2dc2db139f3276f5c2fb2a0620b54f Mon Sep 17 00:00:00 2001 From: David Fries Date: Sat, 3 Nov 2012 15:49:25 -0500 Subject: [PATCH 01/11] typo fix, unused variable cleanup, etc motion.c bad spelling, to "an image" webhttpd.c consolidate the timeout to the top of the file as I needed to change it for testing --- jpegutils.c | 2 +- motion.c | 4 ++-- webhttpd.c | 9 ++++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/jpegutils.c b/jpegutils.c index 89980346..7447c9dc 100644 --- a/jpegutils.c +++ b/jpegutils.c @@ -818,7 +818,7 @@ int decode_jpeg_gray_raw(unsigned char *jpeg_data, int len, guarantee_huff_tables(&dinfo); jpeg_start_decompress (&dinfo); - vsf[0]= 1; vsf[1] = 1; vsf[2] = 1; + vsf[0] = 1; vsf[1] = 1; vsf[2] = 1; /* Height match image height or be exact twice the image height. */ diff --git a/motion.c b/motion.c index 072a8350..9b88b431 100644 --- a/motion.c +++ b/motion.c @@ -174,7 +174,7 @@ static void image_ring_destroy(struct context *cnt) /** * image_save_as_preview * - * This routine is called when we detect motion and want to save a image in the preview buffer + * This routine is called when we detect motion and want to save an image in the preview buffer * * Parameters: * @@ -2728,7 +2728,7 @@ int main (int argc, char **argv) SLEEP(1, 0); /* - * Calculate how many threads runnig or wants to run + * Calculate how many threads are running or wants to run * if zero and we want to finish, break out */ int motion_threads_running = 0; diff --git a/webhttpd.c b/webhttpd.c index 2b3a1333..d228e124 100644 --- a/webhttpd.c +++ b/webhttpd.c @@ -16,6 +16,9 @@ #include #include +/* Timeout in seconds, used for read and write */ +const int NONBLOCK_TIMEOUT = 1; + pthread_mutex_t httpd_mutex; // This is a dummy variable use to kill warnings when not checking sscanf and similar functions @@ -197,7 +200,7 @@ static ssize_t write_nonblock(int fd, const void *buf, size_t size) struct timeval tm; fd_set fds; - tm.tv_sec = 1; /* Timeout in seconds */ + tm.tv_sec = NONBLOCK_TIMEOUT; tm.tv_usec = 0; FD_ZERO(&fds); FD_SET(fd, &fds); @@ -223,7 +226,7 @@ static ssize_t read_nonblock(int fd ,void *buf, ssize_t size) struct timeval tm; fd_set fds; - tm.tv_sec = 1; /* Timeout in seconds */ + tm.tv_sec = NONBLOCK_TIMEOUT; /* Timeout in seconds */ tm.tv_usec = 0; FD_ZERO(&fds); FD_SET(fd, &fds); @@ -2509,7 +2512,7 @@ void httpd_run(struct context **cnt) while ((client_sent_quit_message) && (!closehttpd)) { - client_socket_fd = acceptnonblocking(sd, 1); + client_socket_fd = acceptnonblocking(sd, NONBLOCK_TIMEOUT); if (client_socket_fd < 0) { if ((!cnt[0]) || (cnt[0]->finish)) { From 23d6354d53708cb9ecdde54a80c1bfbd715f539e Mon Sep 17 00:00:00 2001 From: David Fries Date: Sun, 23 Aug 2015 06:53:47 -0500 Subject: [PATCH 02/11] ftp_send_type send type like the function comment says The function comment says it will send 'I' or 'A', but it was only sending 'I', even though the function did everything, but put it in the snprintf (which could be skipped for a fixed string). --- netcam_ftp.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/netcam_ftp.c b/netcam_ftp.c index a6533c15..401ccfa2 100644 --- a/netcam_ftp.c +++ b/netcam_ftp.c @@ -784,12 +784,11 @@ int ftp_get_socket(ftp_context_pointer ctxt) */ int ftp_send_type(ftp_context_pointer ctxt, char type) { - char buf[100]; + char buf[100], utype; int len, res; - toupper(type); - /* Assure transfer will be in "image" mode. */ - snprintf(buf, sizeof(buf), "TYPE I\r\n"); + utype = toupper(type); + snprintf(buf, sizeof(buf), "TYPE %c\r\n", utype); len = strlen(buf); res = send(ctxt->control_file_desc, buf, len, 0); From 0629d9db616552b97e349402ef4ba504b9eef1be Mon Sep 17 00:00:00 2001 From: David Fries Date: Thu, 8 Nov 2012 22:26:17 -0600 Subject: [PATCH 03/11] set motion capture thread running in the main thread 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. --- motion.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/motion.c b/motion.c index 9b88b431..f655c729 100644 --- a/motion.c +++ b/motion.c @@ -1088,8 +1088,6 @@ static void *motion_loop(void *arg) */ unsigned long int time_last_frame = 1, time_current_frame; - cnt->running = 1; - if (motion_init(cnt) < 0) goto err; @@ -2595,11 +2593,22 @@ static void start_motion_thread(struct context *cnt, pthread_attr_t *thread_attr /* Give the thread WATCHDOG_TMO to start */ cnt->watchdog = WATCHDOG_TMO; + /* Flag it as running outside of the thread, otherwise if the main loop + * checked if it is was running before the thread set it to 1, it would + * start another thread for this device. */ + cnt->running = 1; + /* * Create the actual thread. Use 'motion_loop' as the thread * function. */ - pthread_create(&cnt->thread_id, thread_attr, &motion_loop, cnt); + if (pthread_create(&cnt->thread_id, thread_attr, &motion_loop, cnt)) { + /* thread create failed, undo running state */ + cnt->running = 0; + pthread_mutex_lock(&global_lock); + threads_running--; + pthread_mutex_unlock(&global_lock); + } } /** From bab15abfee844dd6eec33cb56eaa749fca06cd07 Mon Sep 17 00:00:00 2001 From: David Fries Date: Thu, 8 Nov 2012 23:14:29 -0600 Subject: [PATCH 04/11] count webcontrol as a thread to avoid a crash 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. --- CHANGELOG | 1 + CREDITS | 3 +++ motion.c | 23 ++++++++++++++++++++--- motion.h | 3 +++ webhttpd.c | 13 ++++++++++++- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 10d2955b..3cd88604 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -143,6 +143,7 @@ Bugfixes * Fixed leak in vloopback. * Fixed a build of motion for some kernel version with not good videodev.h * Netcam Modulo 8 + * Fix webhttpd race condition crash with SIGHUP, add it to running thread counter (David Fries) 3.2.12 Summary of Changes diff --git a/CREDITS b/CREDITS index 89390a73..d21549bc 100644 --- a/CREDITS +++ b/CREDITS @@ -473,6 +473,9 @@ Mark Feenstra Miguel Freitas * Came up with the round robing idea. +David Fries + * Fix webhttpd race condition crash with SIGHUP, add it to running thread counter + Aaron Gage * Pointed me to the vid_mmap/int problem when calling SYNC in video.c diff --git a/motion.c b/motion.c index f655c729..099e0dd8 100644 --- a/motion.c +++ b/motion.c @@ -341,6 +341,7 @@ static void sig_handler(int signo) while (cnt_list[++i]) { cnt_list[i]->makemovie = 1; cnt_list[i]->finish = 1; + cnt_list[i]->webcontrol_finish = 1; /* * Don't restart thread when it ends, * all threads restarts if global restart is set @@ -2723,8 +2724,21 @@ int main (int argc, char **argv) * Create a thread for the control interface if requested. Create it * detached and with 'motion_web_control' as the thread function. */ - if (cnt_list[0]->conf.webcontrol_port) - pthread_create(&thread_id, &thread_attr, &motion_web_control, cnt_list); + if (cnt_list[0]->conf.webcontrol_port) { + pthread_mutex_lock(&global_lock); + threads_running++; + /* set outside the loop to avoid thread set vs main thread check */ + cnt_list[0]->webcontrol_running = 1; + pthread_mutex_unlock(&global_lock); + if (pthread_create(&thread_id, &thread_attr, &motion_web_control, + cnt_list)) { + /* thread create failed, undo running state */ + pthread_mutex_lock(&global_lock); + threads_running--; + cnt_list[0]->webcontrol_running = 0; + pthread_mutex_unlock(&global_lock); + } + } MOTION_LOG(NTC, TYPE_ALL, NO_ERRNO, "%s: Waiting for threads to finish, pid: %d", getpid()); @@ -2746,6 +2760,9 @@ int main (int argc, char **argv) if (cnt_list[i]->running || cnt_list[i]->restart) motion_threads_running++; } + if (cnt_list[0]->conf.webcontrol_port && + cnt_list[0]->webcontrol_running) + motion_threads_running++; if (((motion_threads_running == 0) && finish) || ((motion_threads_running == 0) && (threads_running == 0))) { @@ -2805,7 +2822,7 @@ int main (int argc, char **argv) // Be sure that http control exits fine - cnt_list[0]->finish = 1; + cnt_list[0]->webcontrol_finish = 1; SLEEP(1, 0); MOTION_LOG(NTC, TYPE_ALL, NO_ERRNO, "%s: Motion terminating"); diff --git a/motion.h b/motion.h index c08d84f1..567feacd 100644 --- a/motion.h +++ b/motion.h @@ -370,6 +370,9 @@ struct context { volatile unsigned int restart; /* Restart the thread when it ends */ /* Is the motion thread running */ volatile unsigned int running; + /* Is the web control thread running */ + volatile unsigned int webcontrol_running; + volatile unsigned int webcontrol_finish; /* End the thread */ volatile int watchdog; pthread_t thread_id; diff --git a/webhttpd.c b/webhttpd.c index d228e124..3693ae8d 100644 --- a/webhttpd.c +++ b/webhttpd.c @@ -2515,7 +2515,7 @@ void httpd_run(struct context **cnt) client_socket_fd = acceptnonblocking(sd, NONBLOCK_TIMEOUT); if (client_socket_fd < 0) { - if ((!cnt[0]) || (cnt[0]->finish)) { + if ((!cnt[0]) || (cnt[0]->webcontrol_finish)) { MOTION_LOG(NTC, TYPE_STREAM, NO_ERRNO, "%s: motion-httpd - Finishing"); closehttpd = 1; } @@ -2545,6 +2545,17 @@ void *motion_web_control(void *arg) { struct context **cnt = arg; httpd_run(cnt); + + /* + * Update how many threads we have running. This is done within a + * mutex lock to prevent multiple simultaneous updates to + * 'threads_running'. + */ + pthread_mutex_lock(&global_lock); + threads_running--; + cnt[0]->webcontrol_running = 0; + pthread_mutex_unlock(&global_lock); + MOTION_LOG(NTC, TYPE_STREAM, NO_ERRNO, "%s: motion-httpd thread exit"); pthread_exit(NULL); } From bbd2f3f81393e5f8530c6b1e7c0302bfd3516d7b Mon Sep 17 00:00:00 2001 From: David Fries Date: Thu, 14 Aug 2014 20:43:19 -0500 Subject: [PATCH 05/11] redo watchdog termination logic, wait for the thread to cleanup 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. --- motion.c | 47 +++++++++++++++++++++++++++++++---------------- motion.h | 1 + 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/motion.c b/motion.c index 099e0dd8..a99e131b 100644 --- a/motion.c +++ b/motion.c @@ -2780,24 +2780,39 @@ int main (int argc, char **argv) } if (cnt_list[i]->watchdog > WATCHDOG_OFF) { - cnt_list[i]->watchdog--; + if (cnt_list[i]->watchdog == WATCHDOG_KILL) { + /* if 0 then it finally did clean up (and will restart without any further action here) + * kill(, 0) == ESRCH means the thread is no longer running + * if it is no longer running with running set, then cleanup here so it can restart + */ + if(cnt_list[i]->running && pthread_kill(cnt_list[i]->thread_id, 0) == ESRCH) { + MOTION_LOG(ERR, TYPE_ALL, NO_ERRNO, "%s: cleaning Thread %d", cnt_list[i]->threadnr); + pthread_mutex_lock(&global_lock); + threads_running--; + pthread_mutex_unlock(&global_lock); + motion_cleanup(cnt_list[i]); + cnt_list[i]->running = 0; + cnt_list[i]->finish = 0; + } + } else { + cnt_list[i]->watchdog--; - if (cnt_list[i]->watchdog == 0) { - MOTION_LOG(ERR, TYPE_ALL, NO_ERRNO, "%s: Thread %d - Watchdog timeout, trying to do " - "a graceful restart", cnt_list[i]->threadnr); - cnt_list[i]->finish = 1; - } - if (cnt_list[i]->watchdog == -60) { - MOTION_LOG(ERR, TYPE_ALL, NO_ERRNO, "%s: Thread %d - Watchdog timeout, did NOT restart graceful," - "killing it!", cnt_list[i]->threadnr); - pthread_cancel(cnt_list[i]->thread_id); - pthread_mutex_lock(&global_lock); - threads_running--; - pthread_mutex_unlock(&global_lock); - motion_cleanup(cnt_list[i]); - cnt_list[i]->running = 0; - cnt_list[i]->finish = 0; + if (cnt_list[i]->watchdog == 0) { + MOTION_LOG(ERR, TYPE_ALL, NO_ERRNO, "%s: Thread %d - Watchdog timeout, trying to do " + "a graceful restart", cnt_list[i]->threadnr); + cnt_list[i]->finish = 1; + } + + if (cnt_list[i]->watchdog == WATCHDOG_KILL) { + MOTION_LOG(ERR, TYPE_ALL, NO_ERRNO, "%s: Thread %d - Watchdog timeout, did NOT restart graceful," + "killing it!", cnt_list[i]->threadnr); + /* The problem is pthead_cancel might just wake up the thread so it runs to completition + * or it might not. In either case don't rip the carpet out under it by + * doing motion_cleanup until it no longer is running. + */ + pthread_cancel(cnt_list[i]->thread_id); + } } } } diff --git a/motion.h b/motion.h index 567feacd..65d26986 100644 --- a/motion.h +++ b/motion.h @@ -149,6 +149,7 @@ */ #define WATCHDOG_TMO 30 /* 30 sec max motion_loop interval */ +#define WATCHDOG_KILL -60 /* -60 sec grace period before calling thread cancel */ #define WATCHDOG_OFF -127 /* Turn off watchdog, used when we wants to quit a thread */ #define CONNECTION_KO "Lost connection" From ea7e94863a12f4820a4d409109c9ed9b1e37cbfe Mon Sep 17 00:00:00 2001 From: David Fries Date: Sat, 16 Aug 2014 00:27:28 -0500 Subject: [PATCH 06/11] get the thread out of the xioctl loop 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. --- motion.c | 10 ++++++++++ video2.c | 58 +++++++++++++++++++++++++++++--------------------------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/motion.c b/motion.c index a99e131b..e21f8bf6 100644 --- a/motion.c +++ b/motion.c @@ -357,6 +357,9 @@ static void sig_handler(int signo) break; case SIGSEGV: exit(0); + case SIGVTALRM: + printf("SIGVTALRM went off\n"); + break; } } @@ -2525,6 +2528,10 @@ static void setup_signals(struct sigaction *sig_handler_action, struct sigaction sigaction(SIGQUIT, sig_handler_action, NULL); sigaction(SIGTERM, sig_handler_action, NULL); sigaction(SIGUSR1, sig_handler_action, NULL); + + /* use SIGVTALRM as a way to break out of the ioctl, don't restart */ + sig_handler_action->sa_flags = 0; + sigaction(SIGVTALRM, sig_handler_action, NULL); } /** @@ -2793,6 +2800,9 @@ int main (int argc, char **argv) motion_cleanup(cnt_list[i]); cnt_list[i]->running = 0; cnt_list[i]->finish = 0; + } else { + /* keep sending signals so it doesn't get stuck in a blocking call */ + pthread_kill(cnt_list[i]->thread_id, SIGVTALRM); } } else { cnt_list[i]->watchdog--; diff --git a/video2.c b/video2.c index e1a617a7..714fcc85 100644 --- a/video2.c +++ b/video2.c @@ -173,6 +173,7 @@ typedef struct { u32 ctrl_flags; struct v4l2_queryctrl *controls; + volatile unsigned int *finish; /* End the thread */ } src_v4l2_t; @@ -180,16 +181,16 @@ typedef struct { * xioctl */ #ifdef __OpenBSD__ -static int xioctl(int fd, unsigned long request, void *arg) +static int xioctl(src_v4l2_t *vid_source, unsigned long request, void *arg) #else -static int xioctl(int fd, int request, void *arg) +static int xioctl(src_v4l2_t *vid_source, int request, void *arg) #endif { int ret; do - ret = ioctl(fd, request, arg); - while (-1 == ret && EINTR == errno); + ret = ioctl(vid_source->fd, request, arg); + while (-1 == ret && EINTR == errno && !vid_source->finish); return ret; } @@ -199,7 +200,7 @@ static int xioctl(int fd, int request, void *arg) */ static int v4l2_get_capability(src_v4l2_t * vid_source) { - if (xioctl(vid_source->fd, VIDIOC_QUERYCAP, &vid_source->cap) < 0) { + if (xioctl(vid_source, VIDIOC_QUERYCAP, &vid_source->cap) < 0) { MOTION_LOG(ERR, TYPE_VIDEO, NO_ERRNO, "%s: Not a V4L2 device?"); return -1; } @@ -262,7 +263,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, input.index = IN_TV; else input.index = in; - if (xioctl(vid_source->fd, VIDIOC_ENUMINPUT, &input) == -1) { + if (xioctl(vid_source, VIDIOC_ENUMINPUT, &input) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Unable to query input %d." " VIDIOC_ENUMINPUT, if you use a WEBCAM change input value in conf by -1", input.index); @@ -278,7 +279,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, if (input.type & V4L2_INPUT_TYPE_CAMERA) MOTION_LOG(NTC, TYPE_VIDEO, NO_ERRNO, "%s: - CAMERA"); - if (xioctl(vid_source->fd, VIDIOC_S_INPUT, &input.index) == -1) { + if (xioctl(vid_source, VIDIOC_S_INPUT, &input.index) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error selecting input %d" " VIDIOC_S_INPUT", input.index); return -1; @@ -290,7 +291,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, * Set video standard usually webcams doesn't support the ioctl or * return V4L2_STD_UNKNOWN */ - if (xioctl(vid_source->fd, VIDIOC_G_STD, &std_id) == -1) { + if (xioctl(vid_source, VIDIOC_G_STD, &std_id) == -1) { MOTION_LOG(WRN, TYPE_VIDEO, NO_ERRNO, "%s: Device doesn't support VIDIOC_G_STD"); norm = std_id = 0; // V4L2_STD_UNKNOWN = 0 } @@ -299,7 +300,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, memset(&standard, 0, sizeof(standard)); standard.index = 0; - while (xioctl(vid_source->fd, VIDIOC_ENUMSTD, &standard) == 0) { + while (xioctl(vid_source, VIDIOC_ENUMSTD, &standard) == 0) { if (standard.id & std_id) MOTION_LOG(NTC, TYPE_VIDEO, NO_ERRNO, "%s: - video standard %s", standard.name); @@ -318,7 +319,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, std_id = V4L2_STD_PAL; } - if (xioctl(vid_source->fd, VIDIOC_S_STD, &std_id) == -1) + if (xioctl(vid_source, VIDIOC_S_STD, &std_id) == -1) MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error selecting standard" " method %d VIDIOC_S_STD", (int)std_id); @@ -338,7 +339,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, memset(&tuner, 0, sizeof(struct v4l2_tuner)); tuner.index = input.tuner; - if (xioctl(vid_source->fd, VIDIOC_G_TUNER, &tuner) == -1) { + if (xioctl(vid_source, VIDIOC_G_TUNER, &tuner) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: tuner %d VIDIOC_G_TUNER", tuner.index); return 0; @@ -353,7 +354,7 @@ static int v4l2_select_input(struct config *conf, struct video_dev *viddev, freq.type = V4L2_TUNER_ANALOG_TV; freq.frequency = (freq_ / 1000) * 16; - if (xioctl(vid_source->fd, VIDIOC_S_FREQUENCY, &freq) == -1) { + if (xioctl(vid_source, VIDIOC_S_FREQUENCY, &freq) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: freq %ul VIDIOC_S_FREQUENCY", freq.frequency); return 0; @@ -405,7 +406,7 @@ static int v4l2_do_set_pix_format(u32 pixformat, src_v4l2_t * vid_source, vid_source->dst_fmt.fmt.pix.pixelformat = pixformat; vid_source->dst_fmt.fmt.pix.field = V4L2_FIELD_ANY; - if (xioctl(vid_source->fd, VIDIOC_TRY_FMT, &vid_source->dst_fmt) != -1 && + if (xioctl(vid_source, VIDIOC_TRY_FMT, &vid_source->dst_fmt) != -1 && vid_source->dst_fmt.fmt.pix.pixelformat == pixformat) { MOTION_LOG(NTC, TYPE_VIDEO, NO_ERRNO, "%s: Testing palette %c%c%c%c (%dx%d)", pixformat >> 0, pixformat >> 8, @@ -423,7 +424,7 @@ static int v4l2_do_set_pix_format(u32 pixformat, src_v4l2_t * vid_source, *height = vid_source->dst_fmt.fmt.pix.height; } - if (xioctl(vid_source->fd, VIDIOC_S_FMT, &vid_source->dst_fmt) == -1) { + if (xioctl(vid_source, VIDIOC_S_FMT, &vid_source->dst_fmt) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error setting pixel " "format.\nVIDIOC_S_FMT: "); return -1; @@ -502,7 +503,7 @@ static int v4l2_set_pix_format(struct context *cnt, src_v4l2_t * vid_source, MOTION_LOG(NTC, TYPE_VIDEO, NO_ERRNO, "%s: Supported palettes:"); - while (xioctl(vid_source->fd, VIDIOC_ENUM_FMT, &fmtd) != -1) { + while (xioctl(vid_source, VIDIOC_ENUM_FMT, &fmtd) != -1) { int i; @@ -556,7 +557,7 @@ static void v4l2_set_fps(src_v4l2_t * vid_source) { setfpvid_source->parm.capture.timeperframe.numerator = 1; setfpvid_source->parm.capture.timeperframe.denominator = vid_source->fps; - if (xioctl(vid_source->fd, VIDIOC_S_PARM, setfps) == -1) + if (xioctl(vid_source, VIDIOC_S_PARM, setfps) == -1) MOTION_LOG(ERR, 1, "%s: v4l2_set_fps VIDIOC_S_PARM"); @@ -581,7 +582,7 @@ static int v4l2_set_mmap(src_v4l2_t * vid_source) vid_source->req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; vid_source->req.memory = V4L2_MEMORY_MMAP; - if (xioctl(vid_source->fd, VIDIOC_REQBUFS, &vid_source->req) == -1) { + if (xioctl(vid_source, VIDIOC_REQBUFS, &vid_source->req) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error requesting buffers" " %d for memory map. VIDIOC_REQBUFS", vid_source->req.count); @@ -613,7 +614,7 @@ static int v4l2_set_mmap(src_v4l2_t * vid_source) buf.memory = V4L2_MEMORY_MMAP; buf.index = buffer_index; - if (xioctl(vid_source->fd, VIDIOC_QUERYBUF, &buf) == -1) { + if (xioctl(vid_source, VIDIOC_QUERYBUF, &buf) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error querying buffer" " %i\nVIDIOC_QUERYBUF: ", buffer_index); free(vid_source->buffers); @@ -642,7 +643,7 @@ static int v4l2_set_mmap(src_v4l2_t * vid_source) vid_source->buf.memory = V4L2_MEMORY_MMAP; vid_source->buf.index = buffer_index; - if (xioctl(vid_source->fd, VIDIOC_QBUF, &vid_source->buf) == -1) { + if (xioctl(vid_source, VIDIOC_QBUF, &vid_source->buf) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: VIDIOC_QBUF"); return -1; } @@ -650,7 +651,7 @@ static int v4l2_set_mmap(src_v4l2_t * vid_source) type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - if (xioctl(vid_source->fd, VIDIOC_STREAMON, &type) == -1) { + if (xioctl(vid_source, VIDIOC_STREAMON, &type) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: Error starting stream." " VIDIOC_STREAMON"); return -1; @@ -671,7 +672,7 @@ static int v4l2_scan_controls(src_v4l2_t * vid_source) for (i = 0, count = 0; queried_ctrls[i]; i++) { queryctrl.id = queried_ctrls[i]; - if (xioctl(vid_source->fd, VIDIOC_QUERYCTRL, &queryctrl)) + if (xioctl(vid_source, VIDIOC_QUERYCTRL, &queryctrl)) continue; count++; @@ -692,7 +693,7 @@ static int v4l2_scan_controls(src_v4l2_t * vid_source) struct v4l2_control control; queryctrl.id = queried_ctrls[i]; - if (xioctl(vid_source->fd, VIDIOC_QUERYCTRL, &queryctrl)) + if (xioctl(vid_source, VIDIOC_QUERYCTRL, &queryctrl)) continue; memcpy(ctrl, &queryctrl, sizeof(struct v4l2_queryctrl)); @@ -704,7 +705,7 @@ static int v4l2_scan_controls(src_v4l2_t * vid_source) memset(&control, 0, sizeof (control)); control.id = queried_ctrls[i]; - xioctl(vid_source->fd, VIDIOC_G_CTRL, &control); + xioctl(vid_source, VIDIOC_G_CTRL, &control); MOTION_LOG(NTC, TYPE_VIDEO, NO_ERRNO, "%s: \t\"%s\", default %d, current %d", ctrl->name, ctrl->default_value, control.value); @@ -740,12 +741,12 @@ static int v4l2_set_control(src_v4l2_t * vid_source, u32 cid, int value) case V4L2_CTRL_TYPE_INTEGER: value = control.value = (value * (ctrl->maximum - ctrl->minimum) / 256) + ctrl->minimum; - ret = xioctl(vid_source->fd, VIDIOC_S_CTRL, &control); + ret = xioctl(vid_source, VIDIOC_S_CTRL, &control); break; case V4L2_CTRL_TYPE_BOOLEAN: value = control.value = value ? 1 : 0; - ret = xioctl(vid_source->fd, VIDIOC_S_CTRL, &control); + ret = xioctl(vid_source, VIDIOC_S_CTRL, &control); break; default: @@ -822,6 +823,7 @@ unsigned char *v4l2_start(struct context *cnt, struct video_dev *viddev, int wid vid_source->fd = viddev->fd; vid_source->fps = cnt->conf.frame_limit; vid_source->pframe = -1; + vid_source->finish = &cnt->finish; struct config *conf = &cnt->conf; if (v4l2_get_capability(vid_source)) @@ -965,7 +967,7 @@ int v4l2_next(struct context *cnt, struct video_dev *viddev, unsigned char *map, vid_source->pframe); if (vid_source->pframe >= 0) { - if (xioctl(vid_source->fd, VIDIOC_QBUF, &vid_source->buf) == -1) { + if (xioctl(vid_source, VIDIOC_QBUF, &vid_source->buf) == -1) { MOTION_LOG(ERR, TYPE_VIDEO, SHOW_ERRNO, "%s: VIDIOC_QBUF"); pthread_sigmask(SIG_UNBLOCK, &old, NULL); return -1; @@ -977,7 +979,7 @@ int v4l2_next(struct context *cnt, struct video_dev *viddev, unsigned char *map, vid_source->buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; vid_source->buf.memory = V4L2_MEMORY_MMAP; - if (xioctl(vid_source->fd, VIDIOC_DQBUF, &vid_source->buf) == -1) { + if (xioctl(vid_source, VIDIOC_DQBUF, &vid_source->buf) == -1) { int ret; /* * Some drivers return EIO when there is no signal, @@ -1082,7 +1084,7 @@ void v4l2_close(struct video_dev *viddev) enum v4l2_buf_type type; type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - xioctl(vid_source->fd, VIDIOC_STREAMOFF, &type); + xioctl(vid_source, VIDIOC_STREAMOFF, &type); close(vid_source->fd); vid_source->fd = -1; } From 37360d858d629d0de398930639cbdd8a089bfafa Mon Sep 17 00:00:00 2001 From: David Fries Date: Sat, 23 Aug 2014 14:04:22 -0500 Subject: [PATCH 07/11] fix dangling pointer cnt->current_image after resize 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. --- motion.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/motion.c b/motion.c index e21f8bf6..852c5a97 100644 --- a/motion.c +++ b/motion.c @@ -134,6 +134,7 @@ static void image_ring_resize(struct context *cnt, int new_size) /* Point to the new ring */ cnt->imgs.image_ring = tmp; + cnt->current_image = NULL; cnt->imgs.image_ring_size = new_size; } @@ -168,6 +169,7 @@ static void image_ring_destroy(struct context *cnt) free(cnt->imgs.image_ring); cnt->imgs.image_ring = NULL; + cnt->current_image = NULL; cnt->imgs.image_ring_size = 0; } From 740f3cc73f919d3228dfbaf526084ec95f8a370f Mon Sep 17 00:00:00 2001 From: David Fries Date: Mon, 28 Jul 2014 23:27:53 -0500 Subject: [PATCH 08/11] alg_labeling give diffs feedback even when not over the threshold This is especially useful when picking a threshold to see how close to the threshold it was. --- alg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/alg.c b/alg.c index 343ff47e..226ebd79 100644 --- a/alg.c +++ b/alg.c @@ -526,6 +526,8 @@ static int alg_labeling(struct context *cnt) int height = imgs->height; int labelsize = 0; int current_label = 2; + /* Keep track of the area just under the threshold. */ + int max_under = 0; cnt->current_image->total_labels = 0; imgs->labelsize_max = 0; @@ -561,7 +563,8 @@ static int alg_labeling(struct context *cnt) labelsize = iflood(ix, iy, width, height, out, labels, current_label + 32768, current_label); imgs->labelgroup_max += labelsize; imgs->labels_above++; - } + } else if(max_under < labelsize) + max_under = labelsize; if (imgs->labelsize_max < labelsize) { imgs->labelsize_max = labelsize; @@ -579,8 +582,11 @@ static int alg_labeling(struct context *cnt) "Largest Label: %i", imgs->largest_label, imgs->labelsize_max, cnt->current_image->total_labels); - /* Return group of significant labels. */ - return imgs->labelgroup_max; + /* Return group of significant labels or if that's none, the next largest + * group (which is under the threshold, but especially for setup gives an + * idea how close it was). + */ + return imgs->labelgroup_max ? imgs->labelgroup_max : max_under; } /** From a3104af8d0deef28c7a97f152ddfb62b8256e9a1 Mon Sep 17 00:00:00 2001 From: David Fries Date: Thu, 8 Nov 2012 23:59:32 -0600 Subject: [PATCH 09/11] accept a width specifier to mystrftime 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. --- CHANGELOG | 1 + CREDITS | 1 + motion.c | 46 +++++++++++++++++++++++++++++----------------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 3cd88604..a898754e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -85,6 +85,7 @@ Features * Bug fix as part of warnings in webhttpd.c fixed(Mr-Dave) * Removed compiler warning regarding ffmpeg being newer than 0.4 version(Mr-Dave) * New configure script and identification of ffmpeg version and additional libs. (Mr-Dave) + * Allow text format specifiers to take a width like printf would. (David Fries) * Resolve additional compiler warnings in ffmpeg (Mr-Dave) * Revised INSTALL with samples(Mr-Dave) * Revisions for RTSP and code standard.(Mr-Dave) diff --git a/CREDITS b/CREDITS index d21549bc..a656adb0 100644 --- a/CREDITS +++ b/CREDITS @@ -475,6 +475,7 @@ Miguel Freitas David Fries * Fix webhttpd race condition crash with SIGHUP, add it to running thread counter + * Allow text format specifiers to take a width like printf would. Aaron Gage * Pointed me to the vid_mmap/int problem when calling SYNC in diff --git a/motion.c b/motion.c index 852c5a97..bc66e4f0 100644 --- a/motion.c +++ b/motion.c @@ -3151,6 +3151,7 @@ size_t mystrftime(const struct context *cnt, char *s, size_t max, const char *us char tempstring[PATH_MAX] = ""; char *format, *tempstr; const char *pos_userformat; + int width; format = formatstring; @@ -3170,6 +3171,12 @@ size_t mystrftime(const struct context *cnt, char *s, size_t max, const char *us */ tempstr = tempstring; tempstr[0] = '\0'; + width = 0; + while ('0' <= pos_userformat[1] && pos_userformat[1] <= '9') { + width *= 10; + width += pos_userformat[1] - '0'; + ++pos_userformat; + } switch (*++pos_userformat) { case '\0': // end of string @@ -3177,81 +3184,86 @@ size_t mystrftime(const struct context *cnt, char *s, size_t max, const char *us break; case 'v': // event - sprintf(tempstr, "%02d", cnt->event_nr); + sprintf(tempstr, "%0*d", width ? width : 2, cnt->event_nr); break; case 'q': // shots - sprintf(tempstr, "%02d", cnt->current_image->shot); + sprintf(tempstr, "%0*d", width ? width : 2, + cnt->current_image->shot); break; case 'D': // diffs - sprintf(tempstr, "%d", cnt->current_image->diffs); + sprintf(tempstr, "%*d", width, cnt->current_image->diffs); break; case 'N': // noise - sprintf(tempstr, "%d", cnt->noise); + sprintf(tempstr, "%*d", width, cnt->noise); break; case 'i': // motion width - sprintf(tempstr, "%d", cnt->current_image->location.width); + sprintf(tempstr, "%*d", width, + cnt->current_image->location.width); break; case 'J': // motion height - sprintf(tempstr, "%d", cnt->current_image->location.height); + sprintf(tempstr, "%*d", width, + cnt->current_image->location.height); break; case 'K': // motion center x - sprintf(tempstr, "%d", cnt->current_image->location.x); + sprintf(tempstr, "%*d", width, cnt->current_image->location.x); break; case 'L': // motion center y - sprintf(tempstr, "%d", cnt->current_image->location.y); + sprintf(tempstr, "%*d", width, cnt->current_image->location.y); break; case 'o': // threshold - sprintf(tempstr, "%d", cnt->threshold); + sprintf(tempstr, "%*d", width, cnt->threshold); break; case 'Q': // number of labels - sprintf(tempstr, "%d", cnt->current_image->total_labels); + sprintf(tempstr, "%*d", width, + cnt->current_image->total_labels); break; case 't': // thread number - sprintf(tempstr, "%d",(int)(unsigned long) + sprintf(tempstr, "%*d", width, (int)(unsigned long) pthread_getspecific(tls_key_threadnr)); break; case 'C': // text_event if (cnt->text_event_string && cnt->text_event_string[0]) - snprintf(tempstr, PATH_MAX, "%s", cnt->text_event_string); + snprintf(tempstr, PATH_MAX, "%*s", width, + cnt->text_event_string); else ++pos_userformat; break; case 'w': // picture width - sprintf(tempstr, "%d", cnt->imgs.width); + sprintf(tempstr, "%*d", width, cnt->imgs.width); break; case 'h': // picture height - sprintf(tempstr, "%d", cnt->imgs.height); + sprintf(tempstr, "%*d", width, cnt->imgs.height); break; case 'f': // filename -- or %fps if ((*(pos_userformat+1) == 'p') && (*(pos_userformat+2) == 's')) { - sprintf(tempstr, "%d", cnt->movie_fps); + sprintf(tempstr, "%*d", width, cnt->movie_fps); pos_userformat += 2; break; } if (filename) - snprintf(tempstr, PATH_MAX, "%s", filename); + snprintf(tempstr, PATH_MAX, "%*s", width, filename); else ++pos_userformat; break; case 'n': // sqltype if (sqltype) - sprintf(tempstr, "%d", sqltype); + sprintf(tempstr, "%*d", width, sqltype); else ++pos_userformat; break; From 63e585dc7699247bab857b7d9d0761fbdf73e0c5 Mon Sep 17 00:00:00 2001 From: David Fries Date: Fri, 9 Nov 2012 23:54:12 -0600 Subject: [PATCH 10/11] add power_line_frequency configuration item 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. --- CHANGELOG | 2 ++ CREDITS | 1 + conf.c | 17 +++++++++++++++++ conf.h | 1 + video.h | 1 + video2.c | 19 +++++++++++++++++++ video_common.c | 2 ++ 7 files changed, 43 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index a898754e..2f8a77f1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -86,6 +86,8 @@ Features * Removed compiler warning regarding ffmpeg being newer than 0.4 version(Mr-Dave) * New configure script and identification of ffmpeg version and additional libs. (Mr-Dave) * Allow text format specifiers to take a width like printf would. (David Fries) + * Allow text format specifiers to take a width like printf would. (David Fries) + * Add power_line_frequency configuration item to improve image quality. (David Fries) * Resolve additional compiler warnings in ffmpeg (Mr-Dave) * Revised INSTALL with samples(Mr-Dave) * Revisions for RTSP and code standard.(Mr-Dave) diff --git a/CREDITS b/CREDITS index a656adb0..02434b12 100644 --- a/CREDITS +++ b/CREDITS @@ -476,6 +476,7 @@ Miguel Freitas David Fries * Fix webhttpd race condition crash with SIGHUP, add it to running thread counter * Allow text format specifiers to take a width like printf would. + * Add power_line_frequency configuration item to improve image quality. Aaron Gage * Pointed me to the vid_mmap/int problem when calling SYNC in diff --git a/conf.c b/conf.c index ecac41be..c335bb2f 100644 --- a/conf.c +++ b/conf.c @@ -71,6 +71,7 @@ struct config conf_template = { contrast: 0, saturation: 0, hue: 0, + power_line_frequency: -1, roundrobin_frames: 1, roundrobin_skip: 1, pre_capture: 0, @@ -470,6 +471,22 @@ config_param config_params[] = { print_int }, { + "power_line_frequency", + "# Set the power line frequency to help cancel flicker by compensating\n" + "# for light intensity ripple. (default: -1).\n" + "# This can help reduce power line light flicker.\n" + "# Valuse :\n" + "# do not modify the device setting : -1\n" + "# V4L2_CID_POWER_LINE_FREQUENCY_DISABLED : 0\n" + "# V4L2_CID_POWER_LINE_FREQUENCY_50HZ : 1\n" + "# V4L2_CID_POWER_LINE_FREQUENCY_60HZ : 2\n" + "# V4L2_CID_POWER_LINE_FREQUENCY_AUTO : 3", + 0, + CONF_OFFSET(power_line_frequency), + copy_int, + print_int + }, + { "roundrobin_frames", "\n############################################################\n" "# Round Robin (multiple inputs on same video device name)\n" diff --git a/conf.h b/conf.h index c487b9e9..50462671 100644 --- a/conf.h +++ b/conf.h @@ -53,6 +53,7 @@ struct config { int contrast; int saturation; int hue; + int power_line_frequency; int roundrobin_frames; int roundrobin_skip; int pre_capture; diff --git a/video.h b/video.h index 193087bd..1ac45534 100644 --- a/video.h +++ b/video.h @@ -65,6 +65,7 @@ struct video_dev { int contrast; int saturation; int hue; + int power_line_frequency; unsigned long freq; int tuner_number; int fps; diff --git a/video2.c b/video2.c index 714fcc85..bca75356 100644 --- a/video2.c +++ b/video2.c @@ -144,6 +144,10 @@ static const u32 queried_ctrls[] = { V4L2_CID_CONTRAST, V4L2_CID_SATURATION, V4L2_CID_HUE, +/* first added in Linux kernel v2.6.26 */ +#ifdef V4L2_CID_POWER_LINE_FREQUENCY + V4L2_CID_POWER_LINE_FREQUENCY, +#endif V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE, @@ -749,6 +753,13 @@ static int v4l2_set_control(src_v4l2_t * vid_source, u32 cid, int value) ret = xioctl(vid_source, VIDIOC_S_CTRL, &control); break; + case V4L2_CTRL_TYPE_MENU: + /* set as is, no adjustments */ + control.value = value; + ret = xioctl(vid_source, VIDIOC_S_CTRL, &control); + break; + + default: MOTION_LOG(WRN, TYPE_VIDEO, NO_ERRNO, "%s: control type not supported yet"); return -1; @@ -789,6 +800,14 @@ static void v4l2_picture_controls(struct context *cnt, struct video_dev *viddev) v4l2_set_control(vid_source, V4L2_CID_HUE, viddev->hue); } +#ifdef V4L2_CID_POWER_LINE_FREQUENCY + /* -1 is don't modify as 0 is an option to disable the power line filter */ + if (cnt->conf.power_line_frequency != -1 && cnt->conf.power_line_frequency != viddev->power_line_frequency) { + viddev->power_line_frequency = cnt->conf.power_line_frequency; + v4l2_set_control(vid_source, V4L2_CID_POWER_LINE_FREQUENCY, viddev->power_line_frequency); + } +#endif + if (cnt->conf.autobright) { if (vid_do_autobright(cnt, viddev)) { if (v4l2_set_control(vid_source, V4L2_CID_BRIGHTNESS, viddev->brightness)) diff --git a/video_common.c b/video_common.c index 54569c6e..cabc5aa4 100644 --- a/video_common.c +++ b/video_common.c @@ -732,6 +732,8 @@ static int vid_v4lx_start(struct context *cnt) dev->contrast = 0; dev->saturation = 0; dev->hue = 0; + /* -1 is don't modify, (0 is a valid value) */ + dev->power_line_frequency = -1; dev->owner = -1; dev->v4l_fmt = VIDEO_PALETTE_YUV420P; dev->fps = 0; From 12433edb3f24456961e8188cafa89856e039d854 Mon Sep 17 00:00:00 2001 From: David Fries Date: Sat, 5 Jan 2013 16:42:13 -0600 Subject: [PATCH 11/11] add ffmpeg_duplicate_frames option 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. --- conf.c | 9 +++++++++ conf.h | 1 + motion.c | 7 ++++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index c335bb2f..e2f1275a 100644 --- a/conf.c +++ b/conf.c @@ -678,6 +678,15 @@ config_param config_params[] = { print_string }, { + "ffmpeg_duplicate_frames", + "# True to duplicate frames to achieve \"framerate\" fps, but enough\n" + "duplicated frames and the video appears to freeze once a second.", + 0, + CONF_OFFSET(ffmpeg_duplicate_frames), + copy_bool, + print_bool + }, + { "output_debug_pictures", "# Output pictures with only the pixels moving object (ghost images) (default: off)", 0, diff --git a/conf.h b/conf.h index 50462671..a6ad5c58 100644 --- a/conf.h +++ b/conf.h @@ -30,6 +30,7 @@ struct config { int max_changes; int threshold_tune; const char *output_pictures; + int ffmpeg_duplicate_frames; int motion_img; int emulate_motion; int event_gap; diff --git a/motion.c b/motion.c index bc66e4f0..b3cdc77e 100644 --- a/motion.c +++ b/motion.c @@ -571,8 +571,13 @@ static void process_image_ring(struct context *cnt, unsigned int max_images) /* * Check if we must add any "filler" frames into movie to keep up fps * Only if we are recording videos ( ffmpeg or extenal pipe ) + * While the overall elapsed time might be correct, if there are + * many duplicated frames, say 10 fps, 5 duplicated, the video will + * look like it is frozen every second for half a second. */ - if ((cnt->imgs.image_ring[cnt->imgs.image_ring_out].shot == 0) && + if (!cnt->conf.ffmpeg_duplicate_frames) { + /* don't duplicate frames */ + } else if ((cnt->imgs.image_ring[cnt->imgs.image_ring_out].shot == 0) && #ifdef HAVE_FFMPEG (cnt->ffmpeg_output || (cnt->conf.useextpipe && cnt->extpipe))) { #else