BUG: When setting low_cpu to non zero CPU load increases

Main.KennethLavrsen has entered this bug on behalf of WillCooke because the bug is so nasty that it is worth having proper traceability on. Originally reported on IRC: http://koala.ilog.fr/twikiirc/bin/irclogger_log/motion?date=2005-04-24,Sun&sel=12#l8

When you activate low_cpu the CPU load increases significantly.

Fx. from a load of 10% to 50%. Or from 50% to 99% on a slow machine.

Further observations shows that this gets worse if low_cpu value is equal to framerate.

Test case

Environment

Motion version:	3.2.1_snap14
ffmpeg version:	N/A
Shared libraries:	ffmpeg, mysql, postgresql
Server OS:

-- KennethLavrsen - 24 Apr 2005 for WillCooke

Follow up

Fix record

There were 3 errors:

This was the code in motion.c

/* This will limit the framerate to 1 frame while not detecting
   motion. Using a different motion flag to allow for multiple frames per second
 */

if (cnt->conf.low_cpu && !detecting_motion) {
    /* Recalculate remaining time to delay for a total of 1/low_cpu seconds */
    if (required_frame_time+elapsedtime<(1000000/cnt->conf.low_cpu)) {
        frame_delay=(1000000/cnt->conf.low_cpu)-required_frame_time-elapsedtime;
        
        /* sleep using nanosleep. If a signal such as SIG_CHLD interrupts the
           sleep we just continue sleeping */         
        delay_time.tv_sec = 0;
        delay_time.tv_nsec = frame_delay * 1000;
        
        if (delay_time.tv_nsec > 999999999)
            delay_time.tv_nsec = 999999999;
        
        while ( nanosleep(&delay_time,&remaining_time) == -1 )
        {
            delay_time.tv_sec  = remaining_time.tv_sec;
            delay_time.tv_nsec = remaining_time.tv_nsec;
        }
    }
    /* Correct frame times to ensure required_frame_time is maintained */
    gettimeofday(&tv1, NULL);
    timenow=(tv1.tv_usec+(1000000L/cnt->conf.low_cpu)*tv1.tv_sec)-required_frame_time;
}

The issues

• if (required_frame_time+elapsedtime<(1000000/cnt->conf.low_cpu)) is too long. We should compare the elapsed time since timenow was set and that is the elapsed time + the frame_delay we just added in the code above.
  The corrected code becomes if (frame_delay+elapsedtime<(1000000L/cnt->conf.low_cpu))
• From this the calculation of the low_cpu frame_delay also needs to be corrected.
  frame_delay=(1000000/cnt->conf.low_cpu)-required_frame_time-elapsedtime; becomes
  frame_delay=(1000000L/cnt->conf.low_cpu)-frame_delay-elapsedtime;
• We test for weather we should sleep but no matter if we did or not we adjust the timenow. The gettimeofday() and the setting of timenow must be moved inside the if statement so it only gets adjusted when there is a reason to do so.
• Finally the major bug was the statement setting timenow:
  timenow=(tv1.tv_usec+(1000000L/cnt->conf.low_cpu)*tv1.tv_sec)-required_frame_time;
  This error was introduced when the feature of being able to set low_cpu to any value was added in 3.0.1. The right formular should be like this
  timenow=tv1.tv_usec+1000000L*tv1.tv_sec-required_frame_time;

This gives this new code.

/* This will limit the framerate to 1 frame while not detecting
   motion. Using a different motion flag to allow for multiple frames per second
 */

if (cnt->conf.low_cpu && !detecting_motion) {
    /* Recalculate remaining time to delay for a total of 1/low_cpu seconds */
    if (frame_delay+elapsedtime<(1000000L/cnt->conf.low_cpu)) {
        frame_delay=(1000000L/cnt->conf.low_cpu)-frame_delay-elapsedtime;
        
        /* sleep using nanosleep. If a signal such as SIG_CHLD interrupts the
           sleep we just continue sleeping */         
        delay_time.tv_sec = 0;
        delay_time.tv_nsec = frame_delay * 1000;
        
        if (delay_time.tv_nsec > 999999999)
            delay_time.tv_nsec = 999999999;
        
        while ( nanosleep(&delay_time,&remaining_time) == -1 )
        {
            delay_time.tv_sec  = remaining_time.tv_sec;
            delay_time.tv_nsec = remaining_time.tv_nsec;
        }
        /* Correct frame times to ensure required_frame_time is maintained */
        gettimeofday(&tv1, NULL);
        timenow=tv1.tv_usec+1000000L*tv1.tv_sec-required_frame_time;
    }
}

The fix will be in motion-3.2.1_snap15 and 3.1.20_snap7.

-- KennethLavrsen - 24 Apr 2005