Updated Rotate Patch ( Speed Improvement )

Introduction

The updated rotate patch was created for two reasons:

1. Possible bugs in the original rotate patch.
2. Lazy/slow implementation of the original rotate patch.

The original patch was not able to handle greyscale input, and would probably fail if given such input. Moreover, it had a very straighforward but slow implementation, e.g. allocating memory for the rotate buffer in every call. I also felt that it was rather clumsily integrated into the rest of Motion.

The updated rotate patch has been created to overcome the limits and problems with the old patch.

Contents

These are the main changes made by the patch:

• A new structure, rotate_data, is added to the context structure. It contains data used by the updated rotate functions.
• Code comments have been updated/clarified.
• The interface to the rotate function has been made more consistent with the rest of Motion.
• The rotate function now supports both greyscale and YUV 4:2:0 palettes.
• 180 degrees rotation has been greatly improved in speed (see below).
• The configure script has been modified to support the updated rotate functions.
• Occurances of conf.width and conf.height have been substituted for imgs.width and imgs.height, respectively, where appropriate (and vice versa). The conf versions are capture dimensions, while the imgs versions are output dimensions. Before, they were mixed up in some places.

Speed

The following table illustrates the speed improvements made in the updated patch. The numbers are rotations per second. The test machine is a Pentium II running at 300MHz.

Size	Degrees	Old	New	Opt*
320x240	90	450	480
	180	540	660	2050
	270	450	480
640x480	90	40	50
	180	70	130	260
	270	40	50

*) The numbers in this column illustrate the speed achieved when having the bswap instruction (in my case, compiling with "-march=pentium2"). Note that this only affects 180 degrees rotation.

Things to consider

The updated rotate function relies on having a 32-bit data type. For that reason, configure.in (and configure.in.freebsd) has been updated to check for such a data type. Note that the configure script needs to be re-genererated, and that configure.in.linux should be a copy of configure.in. I didn't fix this as it would increase the size of the patch substantially.

180 degrees rotation is faster than 90 and 270, but gets really fast if it can use the bswap assembler instruction, which is available on Intel 486+ and compatible CPUs. I've updated configure.in with a check for this (it tries to link a program using the instruction). Hopefully it works for everyone, but it needs to be tested.

For the bswap instruction to be detected (and used), Motion must be compiled with the proper CPU/architecture flags. I've included in configure.in AngelCarpintero's code for detecting CPU and architecture.

The CPU/architecture auto-detection probably will cause some speed improvements in other parts of Motion too.

Attached patch

The (preliminary) patch is attached. Please try it out, because it needs some testing before it can be considered final. Also, Angel has talked about replacing the current CPU/architecture auto-detection code with something more robust.

To apply/test the patch, do the following:

1. Apply the patch (e.g., zcat <path-to-patch> | patch -p1 in the Motion directory)
2. Run autoconf (to re-create configure).
3. Configure and build Motion.

Note that you may get an offset warning in step 1. This is because the patch was created with a "baseline" slightly different from the fresh Motion source. You can safely ignore the warnings; they just mean that the patch tool had to look around a bit to find where to apply the patch.

Phase 2 patch

The phase 2 patch for Motion 3.1.18-snap2 is now available. See attached file. The phase 2 patch fixes the following:

• Use of width and height from cnt->conf: Motion now uses the conf values only for initialization. They can be changed via xmlrpc, but that won't break anything. Internally, width and height from cnt->imgs are used as output dimensions, and cap_width and cap_height from cnt->rotate_data are used as capture dimensions. See below for more information.
• Redundant function parameters in vid_start and vid_next: Previously, these functions were called with a context struct plus a number of values from inside the context struct. Now, only the struct gets passed, and the functions extract the appropriate values themselves.

I've added the following information about width/height to motion.h:

/* DIFFERENCES BETWEEN imgs.width, conf.width AND rotate_data.cap_width
 * (and the corresponding height values, of course)
 * ===========================================================================
 * Location      Purpose
 *
 * conf          The values in conf reflect width and height set in the
 *               configuration file. These can be set via xmlrpc, but they are
 *               not used internally by Motion, so it won't break anything.
 *               These values are transferred to imgs in vid_start.
 *
 * imgs          The values in imgs are the actual output dimensions. Normally
 *               the output dimensions are the same as the capture dimensions,
 *               but for 90 or 270 degrees rotation, they are not. E.g., if
 *               you capture at 320x240, and rotate 90 degrees, the output
 *               dimensions are 240x320.
 *               These values are set from the conf values in vid_start, or
 *               from the first JPEG image in netcam_start. For 90 or 270
 *               degrees rotation, they are swapped in rotate_init.
 *
 * rotate_data   The values in rotate_data are named cap_width and cap_height,
 *               and contain the capture dimensions. The difference between
 *               capture and output dimensions is explained above.
 *               These values are set in rotate_init.
 */

Also, I've run a number of test cases:

	DEGREES
TEST CASE	0	90 (270)	180
Idle before w/h change	OK	OK	OK
Detect motion before w/h change	OK	OK	OK
W/h change via xmlrpc	OK	OK	OK
Idle after w/h change	OK	OK	OK
Detect motion after w/h change	OK	OK	OK
Jpeg output	OK	OK	OK
Ppm output	OK	OK	OK
Ghost output	OK	OK	OK
Movie output	OK	OK	OK
Mask file	OK	OK	OK
Pipes	N/A	N/A	N/A
Netcam idle	OK	OK	OK
Netcam motion	N/A	N/A	N/A

"W/h change" = change in width and height using motion-control. As you can see, I have neither tested with pipes nor with network camera motion. If you encounter a test case that fails, please update the table.

-- PerJonsson - 26 Oct 2004

Discussion and Comments

About the autoconf version : To know what version of autoconf you have by default type 'autoconf --version' . The preferable version is 2.50 or above. Note: autoconf 2.13 doesn't work.

-- AngelCarpintero - 08 Oct 2004

---

I've modified the patch a bit now. The configuration script tries to detect the bswap instruction and sets up CFLAGS accordingly. Note that this detection requires CPU/architecture flags to be set. Angel, if you move the CPU/architecture auto-detection code, make sure that it goes before bswap detection.

-- PerJonsson - 29 Sep 2004

---

The CPU/architecture auto-detection doesn't work on FreeBSD so i need to figure out how to implement it. Btw I got that after running configure ( patched with FreeBSDFixCompile ):

checking for short int... yes
checking size of short int... 2
checking for int... yes
checking size of int... 4
checking for gcc option to accept ANSI C... none needed
checking for an ANSI C-conforming const... yes
Detected CPU: unknown
CPU optimization: -march=i386
checking for bswap instruction... yes

So the macro AC_CHECK_SIZEOF and bswap detection works fine , Great Per

-- AngelCarpintero - 07 Oct 2004

---

Should I take special precautions when I build the RPM package of Motion? I only build RPM for 386 type CPUs. I use Pentium 4. I do not own an AMD based machine. Will an RPM that I build run on AMD?

-- KennethLavrsen - 06 Oct 2004

---

Kenneth you can build from your machine a RPM package for every CPU/Architecture, i'll give you some clues about.

There're some issue to work around :

• Autoconf version ( for those people who want to run autoconf after changing configure.in ), that can be managed adding to configure.in ( the requiered version of autoconf ):

AC_PREREQ(2.57)
AC_INIT

So running autoconf with a prior version will show this message :

configure.in:2: error: Autoconf version 2.57 or higher is required
configure.in:2: the top level
autom4te: /usr/bin/m4 failed with exit status: 1

• gcc version , to avoid problems like ( that happens with gcc 2.95 and 3.0 ) :

Detected CPU: Celeron (Covington)
cc1: Invalid option `fpmath=387'
cc1: Invalid option `mmx'
cc1: bad value (pentium2) for -march=

configure.in need to include a check for $CC variable that contains the compiler , AC_PROG_CC fill this variable so a easy way should be something like :

AC_PROG_CC
GCC_VERSION=`$CC --version`
echo "GCC is : $GCC_VERSION"

Obviously $GCC_VERSION will be parse instead of shown to check if gcc version is allowing the optimization values.

• FreeBSD CPU autodetection , fixed in motion-3.1.18_snap6 .

-- AngelCarpintero - 15 Nov 2004

---

Angel, I just realised something when I saw your configure output. I thought that the code for detecting bswap wouldn't link properly unless gcc was given the correct CPU/architecture flags. But it turns out that even though your only flag is -march=i386, bswap is detected anyway.

So, apparently the linker doesn't care about CPU/architecture flags in this case. In other words, the CPU/architecture detection code doesn't have to come before the bswap detection code.

Regarding completeness of the patch: Yes, I think it is complete. But apparently it needs more testing, as at least one guy on the ML has reported that it breaks motion detection. However, he ran automake instead of autoconf, so he used the old configure script, which should break compilation to start with. Perhaps he didn't do make clean first.

-- PerJonsson - 08 Oct 2004

---

I just tested the rotate feature with motion-control. Changing between 0 and 180 or between 90 and 270 is safe. Changing from 90 or 270 to 0 or 180 results in a tiny (but correctly rotated) image with colour disturbances inside a grey box. Changing from 0 or 180 to 90 or 270 crashes Motion with the messag "ioctl (VIDIOCGCHAN): Invalid argument" in syslog.

This of course has to do with the fact that for 90 or 270 degrees rotation, Motion internally switches width and height, and that these rotation levels require a temporary buffer to work with.

To me, it doesn't make sense to change rotation while Motion is running, but the behaviour if you do is not good. Is it possible to make the rotate parameter unchangeable by motion-control?

-- PerJonsson - 09 Oct 2004

---

You use Philips/Logitech camera? The pwc driver adds a gray box when width x height is not correct.

Didn't you change the height/width so that the settings now follows the output instead of the camera's native format? Then when you rotate the camera gets very wrong width and heigh.

-- KennethLavrsen - 10 Oct 2004

---

This page is getting big now

I've added info about the phase 2 patch above, and attached the patch file below.

-- PerJonsson - 26 Oct 2004

---

I have cut a little in the remarks with no information above. History is always preserved anyway.

The patch is applied in my sources. I am testing smartmask for the moment so I will test the updated rotate patch the day after. Inspected the code quickly and it looks good. This comment can be deleted soon

I just released snap3 including this patch. I also changed the state to released scheduled. Even if it can still receive a little more testing. It will anyway because of other patches.

-- KennethLavrsen - 26 Oct 2004

---