BUG: auto_brightness function cycles between black and white, causing false triggers

I am using a P2-450 running kernel 2.6.5 on a smp machine. The capture card is a generic Videologic card using the Brooktree 848 capture chip and a single b/w nightvision camera. I have run motion 3.0.7 and 3.0.7 on this equipment wth no problems but when I enable the auto_brightness routine the capturing is immediateley triggered. This is due to the auto_brightness constantly cycling the capture card between minimum black and maximum white levels.

The problem seems to be that my card takes several frames before the brightness adjustment takes effect. Motion does not allow for this and erroneously keeps adjusting the level until it cycles around again. The change from earlier versions to make the adjustment in one go seems to have showed the problem with my bt848 card.

I have modified the program so that the adjustment is made in small changes rather than in one go. This has completely fixed the problem and only takes a few seconds to settle to the new range, even from completely black settings ( set brightness to 1 with no auto_brightness and then restart with auto_brightness to test ).

At the same time I have added two settings to the conference file to allow the target range minimum and maximum values to be specified in the config file. I also write the average picture level and card brightness levels to the image as a check.

NB: I will post a patch for the changes I have made once I am happy with it. ( has been running for over 48 hours without issue so seems okay )

Test case

enable auto_brightness and a normal threshold value: ( only needed values given, most other values as distributed config file - all values as used for motion 3.1.7 )

auto_brightness on

threshold 1000

Environment

Motion version:	3.1.18
ffmpeg version:	0.4.9-pre1, build 4718
Shared libraries:	ffmpeg
Server OS:	Suse Enterprise 9.2, kernel 2.6.5-7.97-smp

-- GearboxGraeme - 26 Feb 2005

Follow up

Looking forward to see the patch.

I need to evaluate if I want to introduce too many new options for this. It is always a balance between keeping the configuration simple and making things work well.

Often I have found that there is a good compromise where you have some part adjustable and some part fixed.

So I would like to also have your experience on what values that works well.

Post your patch as you like to see it yourself and we will discuss it at night (European nights) on IRC. That is where many of these decisions are made. You are always welcome to drop by.

-- KennethLavrsen - 04 Mar 2005

---

I just by accident found your patch file attached to the patch index topic. This is not how it is supposed to be done. You are supposed to create a new patch topic using the form at the bottom of the topic like it has been done with all the others including a description and how to install it and use it.

I looked at the patch file and it does not look correct.

It contains a lot of files like configure, Makefile etc etc.

Can you make a clean patch file?

I have moved your attachment to this bug topic instead of deleting it.

-- KennethLavrsen - 10 Mar 2005

Fix record

I decided that the proposed patch would not be included. I did not feel like adding two new options that on top of it needs some experimenting to set right. The old code was not quite functional either. So I decided to re-write the whole thing.

The code is shown below.

• When auto_brightness is disabled the brightness setting is multiplied by 256 and simply fed to the device driver with an ioctl.
• When auto_brightness is enabled the brightness setting becomes a target value for brightness.
• The regulation loop has been made so that it fast enough to react pretty quickly but slow enough so at least when trying at 30fps on pwc and a bttv driven capture card, the regulation does not overshoot. The chosen stepsize: (target-average)/5+1 is a good compromize. (target-average) overshoots and is almost unstable with 5-8 ripples on a bttv card. (target-average)/2 makes one ripple. But other tested with this fix and found that the system was unstable until they devided by 5. So I chose /5. The +1 is made to avoid the value 0 making the adjustment stop one step too early.
• The average brightness is now measured by stepping 101 instead of 100. This makes the test point pattern not vertically alligned with normal picture sizes such as 320x240 and 640x480.
• As this is written I am still not sure about how to make a better compromise that also prevents too bright images at night. Suggestions welcome.

#define MAX2(x, y) ((x) > (y) ? (x) : (y))
#define MIN2(x, y) ((x) < (y) ? (x) : (y))

static void v4l_picture_controls(struct video_dev *viddev, struct context *cnt)
{
    int dev=viddev->fd;
    unsigned char *image = cnt->imgs.new;
    struct video_picture vid_pic;
    int make_change = 0;

SNIP-SNAP code for other picture controls not shown and not changed!
    
    if (cnt->conf.autobright) {
        
        int brightness_window_high;
        int brightness_window_low;
        int brightness_target;
        int i, j = 0, avg = 0, step = 0;
        
        if (cnt->conf.brightness)
            brightness_target = cnt->conf.brightness;
        else
            brightness_target = 128;
        
        brightness_window_high = MIN2(brightness_target + 10, 255);
        brightness_window_low = MAX2(brightness_target - 10, 1);
        
        for (i = 0; i < cnt->imgs.motionsize; i += 101) {
            avg += *image++;
            j++;
        }
        avg = avg / j;
        
        if (avg > brightness_window_high || avg < brightness_window_low) {
            /* If we already read the VIDIOGPICT - we should not do it again */
            if (!make_change) {
                if (ioctl(dev, VIDIOCGPICT, &vid_pic)==-1) {
                    printf("[%d] ioctl (VIDIOCGPICT): %m\n", cnt->threadnr);
                    syslog(LOG_ERR, "[%d] ioctl (VIDIOCGPICT): %m", cnt->threadnr);
                }
            }
            /* average is above window - turn down brigtness - go for the target */
            if (avg > brightness_window_high) {
                step = MIN2((avg - brightness_target)/5+1, viddev->brightness - 1);
                if (viddev->brightness > 1 ) {
                    viddev->brightness -= step;
                    vid_pic.brightness = viddev->brightness * 256;
                    make_change = 1;
                }
            }
            /* average is below window - turn up brigtness - go for the target */
            if (avg < brightness_window_low) {
                step = MIN2((brightness_target - avg)/5+1, 255 - viddev->brightness);
                if (viddev->brightness < 255 ) {
                    viddev->brightness += step;
                    vid_pic.brightness = viddev->brightness * 256;
                    make_change = 1;
                }
            }
        }
    } else {
        if (cnt->conf.brightness && cnt->conf.brightness != viddev->brightness) {
            if (!make_change) {
                if (ioctl(dev, VIDIOCGPICT, &vid_pic)==-1) {
                    printf("[%d] ioctl (VIDIOCGPICT): %m\n", cnt->threadnr);
                    syslog(LOG_ERR, "[%d] ioctl (VIDIOCGPICT): %m", cnt->threadnr);
                }
            }
        
            make_change = 1;
            vid_pic.brightness = cnt->conf.brightness * 256;
            viddev->brightness = cnt->conf.brightness;
        }
    }

SNIP-SNAP rest of function not shown

Code will be included in motion-3.2.1_snap15