[PATCH] Auto-exposure for ov96xx sensors.
GWater
webcams with a sensor from the ov96xx series.
These are currently:
- 0c45:624e (SOI968 ≈ OV9628)
- 0c45:624f (OV9650)
- 0c45:6288 (OV9655)
Users with these webcams please test this patch.
( $ git apply 0001-Auto...
$ make
$ sudo make rmmod
$ sudo make insmod )
GWater
Vasily Khoruzhick
> Lets make microdia webcams plug'n play, (currently plug'n pray)
> To post to this group, send email to micr...@googlegroups.com
> Visit us online https://groups.google.com/group/microdia
> -~----------~----~----~----~------~----~------~--~---
Sorry, but it doesn't work for my 624f-cam (ov9650 sensor) :(
GWater
>
>> GWater schrieb:
>>
>>> This patch adds the auto-exposure setting/feature for all
>>> webcams with a sensor from the ov96xx series.
>>> These are currently:
>>> - 0c45:624e (SOI968 ≈ OV9628)
>>> - 0c45:624f (OV9650)
>>> - 0c45:6288 (OV9655)
>>>
>>> Users with these webcams please test this patch.
>>>
>>> ( $ git apply 0001-Auto...
>>> $ make
>>> $ sudo make rmmod
>>> $ sudo make insmod )
>>>
>>> GWater
>>>
>> Here is th missing patch.
>>
>> GWater
>>
>> >>
>
>
register 0x13 is the one. Nevertheless there may be other registers like
0x14 which are able to freeze or disable AEC.
Therefore I would really like to have a I2C register dump from you while
the cam is streaming. Unfortunatly I think we don't have this feature
right know. If you don't have the time to do it manually I think I could
write a script to do it though.
GWater
GWater
> On 10 August 2008 20:06:29 GWater wrote:
>
>> GWater schrieb:
>>
>>> This patch adds the auto-exposure setting/feature for all
>>> webcams with a sensor from the ov96xx series.
>>> These are currently:
>>> - 0c45:624f (OV9650)
>>> - 0c45:6288 (OV9655)
>>>
>>> Users with these webcams please test this patch.
>>>
>>> ( $ git apply 0001-Auto...
>>> $ make
>>> $ sudo make rmmod
>>> $ sudo make insmod )
>>>
>>> GWater
>>>
>> Here is th missing patch.
>>
>> GWater
>>
>> >>
>
>
Maybe that's why you don't see a difference.
GWater
GWater
>
>> GWater schrieb:
>>
>>> This patch adds the auto-exposure setting/feature for all
>>> webcams with a sensor from the ov96xx series.
>>> These are currently:
>>> - 0c45:624e (SOI968 ≈ OV9628)
>>> - 0c45:624f (OV9650)
>>> - 0c45:6288 (OV9655)
>>>
>>> Users with these webcams please test this patch.
>>>
>>> ( $ git apply 0001-Auto...
>>> $ make
>>> $ sudo make rmmod
>>> $ sudo make insmod )
>>>
>>> GWater
>>>
>> Here is th missing patch.
>>
>> GWater
>>
>> >>
>
>
to be able to turn autoexposurecontrol on and off - test it and give me
a "GO!".
Second: I want this feature and it works for 624e. Conventions state
that commits that only change stuff for one cam may be pushed afer
insmod, test, ...
I will exclude the feature for 624f and 6288 and push this only for 624e.
GWater
Kuzja
On Aug 15, 3:51 am, "JoJo jojo" <onetwoj...@gmail.com> wrote:
> Hi
>
> To help us with development please test the driver (latest git version),
> apply the patches, make, insmod etc and let us know if the patch works or not.
>
> -JoJo
>
> 4KViewDownload
JoJo jojo
This patch is just 1 of the patches that need testing, here is the
full list of patches that need testing
https://groups.google.com/group/microdia/browse_thread/thread/497a71f5bf2e2f14
(this patch is also included in the bigger list, so you just need to
test the above 9 patches.
To apply patches, download them into the latest microdia driver source
code directory, then
$ git apply ./<patchfile.patch>
OR
$ git am ./<patchfile.patch>
-JoJo
2008/8/16 Kuzja <kuznet...@gmail.com>:
Kuzja
and eventually driver refused to compile. I'm attaching the log so you
can have a look by yourself:
vitaly@ToshibaLaptop:~$ cd microdia
vitaly@ToshibaLaptop:~/microdia$ git apply 0001-Auto-exposure-for-
ov96xx-sensors.patch
vitaly@ToshibaLaptop:~/microdia$ git 0001-Seperate-bridge-and-sensor-
code-for-624f.patch
git: '0001-Seperate-bridge-and-sensor-code-for-624f.patch' is not a
git-command. See 'git --help'.
vitaly@ToshibaLaptop:~/microdia$ git apply 0001-Seperate-bridge-and-
sensor-code-for-624f.patch
error: patch failed: microdia-usb.c:285
error: microdia-usb.c: patch does not apply
vitaly@ToshibaLaptop:~/microdia$ git apply 0002-
Minimizing-624f_start_stream.patch
error: patch failed: microdia-dev.c:3911
error: microdia-dev.c: patch does not apply
vitaly@ToshibaLaptop:~/microdia$ git apply 0004-Readded-support-
for-627f-cameras.patch
vitaly@ToshibaLaptop:~/microdia$ git apply 0005-Added-support-for-
the-0c45-6253-camera.patch
error: patch failed: microdia-usb.c:296
error: microdia-usb.c: patch does not apply
vitaly@ToshibaLaptop:~/microdia$ git apply 0006-Added-support-
for-0c45-62bb.patch
vitaly@ToshibaLaptop:~/microdia$ git apply 0007-Fix-warnings-when-
compiled-under-rt-kernel.patch
vitaly@ToshibaLaptop:~/microdia$ git apply 0008-Support-for-
hardware-resolultion-change.patch
error: patch failed: microdia-dev.c:3974
error: microdia-dev.c: patch does not apply
vitaly@ToshibaLaptop:~/microdia$ 0009-Removed-start_stream-writes-
to-10fb-10ff-and-1189.patch
bash: 0009-Removed-start_stream-writes-to-10fb-10ff-and-1189.patch:
command not found
vitaly@ToshibaLaptop:~/microdia$ git apply 0009-Removed-start_stream-
writes-to-10fb-10ff-and-1189.patch
vitaly@ToshibaLaptop:~/microdia$ make
make -C /lib/modules/2.6.24-19-generic/build SUBDIRS=/home/vitaly/
microdia modules
make[1]: Entering directory `/usr/src/linux-headers-2.6.24-19-generic'
CC [M] /home/vitaly/microdia/microdia-usb.o
/home/vitaly/microdia/microdia-usb.c:338: error: unknown field
'sensor_init' specified in initializer
/home/vitaly/microdia/microdia-usb.c:338: error: 'ov965x_initialize'
undeclared here (not in a function)
/home/vitaly/microdia/microdia-usb.c:342: error: 'ov965x_set_exposure'
undeclared here (not in a function)
/home/vitaly/microdia/microdia-usb.c:343: error: 'ov965x_set_hvflip'
undeclared here (not in a function)
make[2]: *** [/home/vitaly/microdia/microdia-usb.o] Error 1
make[1]: *** [_module_/home/vitaly/microdia] Error 2
make[1]: Leaving directory `/usr/src/linux-headers-2.6.24-19-generic'
make: *** [driver] Error 2
vitaly@ToshibaLaptop:~/microdia$
On Aug 16, 1:32 am, "JoJo jojo" <onetwoj...@gmail.com> wrote:
> Hi Kuzja
>
> This patch is just 1 of the patches that need testing, here is the
> (this patch is also included in the bigger list, so you just need to
> test the above 9 patches.
>
> To apply patches, download them into the latest microdia driver source
> code directory, then
>
> $ git apply ./<patchfile.patch>
> OR
> $ git am ./<patchfile.patch>
>
> -JoJo
>
Brian Johnson
just git <patchname>. However i've just finsihed pushing all but the
ninth patch in the serires out to the main repo. The ninth was patch
is fir the moment apparently not working right for the 672b but you
should tst it and make sure its working right on your camera at least.
2008/8/21 Kuzja <kuznet...@gmail.com>:
Kuzja
and-1189.patch), tested the driver and everything seems to work fine.
On Aug 21, 9:32 am, "Brian Johnson" <brij...@gmail.com> wrote:
> Looks like for the second patch you dind't actually type git apply
> just git <patchname>. However i've just finsihed pushing all but the
> ninth patch in the serires out to the main repo. The ninth was patch
> is fir the moment apparently not working right for the 672b but you
> should tst it and make sure its working right on your camera at least.
>
Kuzja
picture has significantly decreased. It looks similar to you can see
on the recently uploaded skypebad.png. Camora seized to detect my cam.
I guess its not because of the patch 0009 cuz i tried the last version
of the driver and it gives the same result
GWater
skype-bad looks like a wrongly applied decoder to me.
Anyway, let's investigate.
Attached is a datasheet that should cover your sensor. Registers seem to
fit so far.
I can only see two possible reasons for a quality loss:
1. You tested in a very dark environment and the sensor set the exposure
up so far that everything looked blurry afterwards.
2. Some other settings that my cam already does during startstream are
missing for yours.
GWater
Brian Johnson
320x240 it may be an issue with the image scaling. can you use debugfs
to check the values of registers 1189 and 10fb-10ff? Also you can try
to systematically remove one commit at a time to see if you can find
which commit is causing problems.
2008/8/23 GWater <grew...@googlemail.com>:
GWater
I just noticed you changed the name of the AEC-function to
ov965x_set_autoexposure. I initially called it ov96xx_set_autoexposure
which was not a typo. My sensor is more similar to ov9625 than any other
known omnivision sensor adn there are some visible differences in the
registers. (That's also why I didn't move it to ov965x.c first - but
that's ok). I would like to rename the function back and remove the
author tag - most of this function is copied from mt9vx11.c anyway. Any
problems with that?
ov965x_set_exposure is a bit strange - none of the datasheets assigns
the registers 0x2d and 0x2e to an exposure value. What exactly does this
function do? Could you add doxygen comments?
I already stated that I am very impressed by the new 624f startstream
and I'll try to write something like that myself for 624e. Anyway, part
of this is the new ov965x_initialization. Is there a specific reason why
you set all the registers which already have the default value? If not -
I hope no one minds if I leave them out.
GWater
Brian Johnson
Yeah the problem is calling it ov96xx_set_autoexposure isn't really
any more accurate, since it will work for the ov76xx series as well.
So maybe it would be better named simply something like
ov_set_exposure, but then i'm not sure where exactly to put it, should
we make a new file simply called omnivision.c and combine the contents
of the various ov*.c files into it. Or leave them separate but put
only common functions into the new omnivision.c file? When i was
merging things i wasn't entirely the sure th best way to go about this
since while the register set is certainly different among the various
omnivision sensors there are certain things that are teh same for
every one i've looked at. what do you think?
About the ov965x_set_exposure function i didn't originally write that
one it used to be called 624f_set_exposure. And yeah its not really
adjusting the exposure value at all the registers it adjusts are
updated when enabling night mode on omnivision sensors. the effects it
has are basically to make then image brighter and lower the frame
rate. But it may be better to change to to actually update the
exposure value instead.
Yeah the default values should safely be able to be removed in the
ov965x_initialize function. I simply didn't bother to do more then
remove the duplicate writes that were originally being done. Also
once you separate your sensor init from the bridge init can i ask you
play around with using the bridge init set form my 624f_start_stream
there should be no real reason why all the devices can't end up using
the same bridge init and being able to eventually do so will reduce
the mount of code a lot as well.
2008/8/23 GWater <grew...@googlemail.com>:
JoJo jojo
> Does it look bad at both 640x480 and 320x240? If it is only bad at
> 320x240 it may be an issue with the image scaling. can you use debugfs
> to check the values of registers 1189 and 10fb-10ff? Also you can try
> to systematically remove one commit at a time to see if you can find
> which commit is causing problems.
>
Hi Brian
yes it would appear so...
the commit 8063bf44e8ffac breaks apps like skype/ekiga, while
mplayer/cheese work fine
(as you pointed out mplayer 640x420 & 320x240 both work fine, however)
and yes I tried working on your 9th pending patch, however stream
refuses to start w/o that write to 1189.
-JoJo
Kuzja
thing up a bit =) check out screenshot-ekiga.png
And camorama doesn't detect device anymore
On Aug 23, 3:03 pm, "JoJo jojo" <onetwoj...@gmail.com> wrote:
> 2008/8/24 Brian Johnson <brij...@gmail.com>:
Kuzja
thing up a bit =) check out screenshot-ekiga.png
And camorama doesn't detect device anymore
On Aug 23, 3:03 pm, "JoJo jojo" <onetwoj...@gmail.com> wrote:
> 2008/8/24 Brian Johnson <brij...@gmail.com>:
>
Brian Johnson
So you're saying using mplayer at both 640x480 and 320x240 works
properly but using skype/ekiga does not?
What format and resolution are the non working skype and ekiga running
at? and can people that are hacving problems with this post the output
from bridge.dump under the debugfs while runing skype or ekiga?
Also JoJo if it seems that the write to 1189 is necessary to get your
start_stream function working right can you try doing that write from
within the intialization fucntion instead that way it will get the
right value but won't overwrite the the scaling factor.
Kuzja
that is
On Aug 23, 3:34 pm, "Brian Johnson" <brij...@gmail.com> wrote:
> Jojo,
> So you're saying using mplayer at both 640x480 and 320x240 works
> properly but using skype/ekiga does not?
> What format and resolution are the non working skype and ekiga running
> at? and can people that are hacving problems with this post the output
> from bridge.dump under the debugfs while runing skype or ekiga?
>
> Also JoJo if it seems that the write to 1189 is necessary to get your
> start_stream function working right can you try doing that write from
> within the intialization fucntion instead that way it will get the
> right value but won't overwrite the the scaling factor.
>
JoJo jojo
take a look at
https://groups.google.com/group/microdia/web/testing-microdia-driver-draft
look under "The debugfs interface"
-JoJo
GWater
I wasn't aware this was such a large issue.
I'd say we make one big "omnivision.c". (It would follow our current way
of handling things in "microdia-dev.c".)
We could structure it a bit to show "first comes genereal stuff, next
ov7xxx, ...".
A resulting problem will be the ov*.h files since we will have different
defines for one general register like 0x13.
There are a two ways to solve that:
1. Use no defines in general functions but comments for explanation and
keep different "ov*.h" files fo each sensor.
2. Merge "ov*.h" files to "omnivision.h". Structure it like
"omnivision.c" but loose the connection to the datasheets.
I am in favour of the first one. One omvision.c, many ov*.h.
[set_exposure]
I agree.
[sensor_init]
I'll try that.
GWater
Brian Johnson schrieb:
JoJo jojo
>
> [omnivision-files]
> I wasn't aware this was such a large issue.
> I'd say we make one big "omnivision.c". (It would follow our current way
> of handling things in "microdia-dev.c".)
> We could structure it a bit to show "first comes genereal stuff, next
> ov7xxx, ...".
We're probably headed that way, first started with init/start/stop sequences,
which had both bridge r/w & sensor r/w interwined. Then we seperated
bridge & sensors,
>
> A resulting problem will be the ov*.h files since we will have different
> defines for one general register like 0x13.
> There are a two ways to solve that:
>
> 1. Use no defines in general functions but comments for explanation and
> keep different "ov*.h" files fo each sensor.
>
> 2. Merge "ov*.h" files to "omnivision.h". Structure it like
> "omnivision.c" but loose the connection to the datasheets.
>
> I am in favour of the first one. One omvision.c, many ov*.h.
>
Lets start with 1 set of functions for a unique sensor,
later we can decide which functionality can be merged together
if a register like 0x13 has a friendly/meaningful name, use it.
>
> [set_exposure]
> I agree.
>
This is a bit more complicated than that...
exposure & auto-exposure feature are available in both Sensor & Bridge,
so if we have the know-how to implement this feature in Bridge, do it there
while leaving the sensor at the best possible output value.
in case we don't know how to enable/disable or otherwise use this
feature in Bridge
we need to use the sensor documentation to implement it there.
so we either keep the sensor exposure values constant & vary the
bridge exposure values or
forget about the bridge & vary the sensor exposure values directly.
-JoJo
GWater
exposed to anything - it can only make things darker but never brighter.
If we have to implement the "best" setting on the sensor anyway we can
do it variably and spare the bridge stuff.
GWater
Vasily Khoruzhick
> ov965x_set_exposure is a bit strange - none of the datasheets assigns
> the registers 0x2d and 0x2e to an exposure value. What exactly does this
> function do? Could you add doxygen comments?
It seems that ov965x spec lies. Maybe you've noticed that initialize function
of ov9650 makes many writes to undocumented\reserved registers\bits.
I've tried to use exposure regs address from spec, but it doesn't work :(
maybe sensor in 624f cam is not a ov9650 sensor? Or some kind modified
ov9650?
Regards
Vasily
GWater
automatically reset the values. ov9625 spec works perfectly for my cam.
GWater
Vasily Khoruzhick
> Did you check whether AEC was enabled - if not maybe the sensor
> automatically reset the values. ov9625 spec works perfectly for my cam.
>
> GWater
Tried with AEC enabled and disabled... It seems that sensor in 624f do not
react on enabling\disabling AEC, or something wrong with stable range of AEC.
It's strange because AGC and AWB works (it's bits of same reg as for AEC)
Maybe we need to write our own sensor init func, without writes to
undocumented regs? :)
Regards
Vasily
JoJo jojo
Hi Vasily
OV7660 data sheet says
2D ADVFL 00 RW LSB of insert dummy lines in vertical direction (1 bit
equals 1 line)
2E ADVFH 00 RW MSB of insert dummy lines in vertical direction
there isn't just 1 register that we can tune and get the exposure that we want,
Last I checked there were a group of 3 or more registers being written
to / read from
the specs don't lie, it's just that Omnivision could be least bothered
to document them properly ;-0
-JoJo
JoJo jojo
> exposed to anything - it can only make things darker but never brighter.
> If we have to implement the "best" setting on the sensor anyway we can
> do it variably and spare the bridge stuff.
>
> GWater
Bridge is more capable to better calculate / self adjust exposure on-the-fly
Bridge has registers that moniter various parameters, take their average,
and we can set the gain etc as a value derived from these averages etc.
(There's a reason 'Dem EE guys put these nifty things in there ;-)
-JoJo
Vasily Khoruzhick
> Hi Vasily
>
> OV7660 data sheet says
> 2D ADVFL 00 RW LSB of insert dummy lines in vertical direction (1 bit
> equals 1 line)
> 2E ADVFH 00 RW MSB of insert dummy lines in vertical direction
But writes to changing values in these regs causes changing exposure... Maybe
sensor in 624f is not a OV9650.
> there isn't just 1 register that we can tune and get the exposure that we
> want, Last I checked there were a group of 3 or more registers being
> written to / read from
>
> the specs don't lie, it's just that Omnivision could be least bothered
> to document them properly ;-0
maybe, but why things that were wrote in spec do not fit things that really
happen? :)
> -JoJo
GWater
Exposure is AFAIK not something one calculates but something optical.
Anyway - since auto-exposure is currently the most comfortable method to
use in the sensor we don't need functions to set a fixed exposure in the
sensors. Let's find the bridge setting (it's 1 bit among ~3000).
GWater