[10/11] ov534: Add Powerline Frequency control
Commit Message
From: Mosalam Ebrahimi <m.ebrahimi@ieee.org>
Note that setting this options to 50Hz can reduce the framerate, so the
default is still 60Hz.
Signed-off-by: Mosalam Ebrahimi <m.ebrahimi@ieee.org>
Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
linux/drivers/media/video/gspca/ov534.c | 71 +++++++++++++++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Comments
On Sat, 27 Feb 2010 21:20:27 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:
> +static int sd_querymenu(struct gspca_dev *gspca_dev,
> + struct v4l2_querymenu *menu)
> +{
> + switch (menu->id) {
> + case V4L2_CID_POWER_LINE_FREQUENCY:
> + switch (menu->index) {
> + case 0: /*
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
> + strcpy((char *) menu->name, "50 Hz");
> + return 0;
> + case 1: /*
> V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
> + strcpy((char *) menu->name, "60 Hz");
> + return 0;
> + }
> + break;
> + }
> +
> + return -EINVAL;
> +}
In videodev2.h, there is:
V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
V4L2_CID_POWER_LINE_FREQUENCY_60HZ = 2,
On Sun, 28 Feb 2010 19:49:51 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:
> On Sat, 27 Feb 2010 21:20:27 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
>
> > +static int sd_querymenu(struct gspca_dev *gspca_dev,
> > + struct v4l2_querymenu *menu)
> > +{
> > + switch (menu->id) {
> > + case V4L2_CID_POWER_LINE_FREQUENCY:
> > + switch (menu->index) {
> > + case 0: /*
> > V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
> > + strcpy((char *) menu->name, "50 Hz");
> > + return 0;
> > + case 1: /*
> > V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
> > + strcpy((char *) menu->name, "60 Hz");
> > + return 0;
> > + }
> > + break;
> > + }
> > +
> > + return -EINVAL;
> > +}
>
> In videodev2.h, there is:
>
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
> V4L2_CID_POWER_LINE_FREQUENCY_60HZ = 2,
>
Maybe we could just use
V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
It looks like the code matches the DISABLED state (writing 0 to the
register). Mosalam?
Regards,
Antonio
On Sun, 28 Feb 2010 20:18:50 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:
> Maybe we could just use
> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
>
> It looks like the code matches the DISABLED state (writing 0 to the
> register). Mosalam?
I don't know the ov772x sensor. I think it should look like the ov7670
where there are 3 registers to control the light frequency: one
register tells if light frequency filter must be used, and which
frequency 50Hz or 60Hz; the two other ones give the filter values for
each frequency.
On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Sun, 28 Feb 2010 20:18:50 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
>
>> Maybe we could just use
>> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
>> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
>>
>> It looks like the code matches the DISABLED state (writing 0 to the
>> register). Mosalam?
>
> I don't know the ov772x sensor. I think it should look like the ov7670
> where there are 3 registers to control the light frequency: one
> register tells if light frequency filter must be used, and which
> frequency 50Hz or 60Hz; the two other ones give the filter values for
> each frequency.
>
I think it's safe to go with disabled/50hz. Perhaps later if needed
can patch it to control the filter values. Since it seems there is no
flickering in the 60hz regions at available frame rates, and this
register almost perfectly removes light flickers in the 50hz regions
(by modifying exposure/frame rate).
Mosalam
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2 Mar 2010 11:26:15 +0000
"M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
> On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
> > On Sun, 28 Feb 2010 20:18:50 +0100
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> >
> >> Maybe we could just use
> >> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
> >> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
> >>
> >> It looks like the code matches the DISABLED state (writing 0 to the
> >> register). Mosalam?
> >
> > I don't know the ov772x sensor. I think it should look like the ov7670
> > where there are 3 registers to control the light frequency: one
> > register tells if light frequency filter must be used, and which
> > frequency 50Hz or 60Hz; the two other ones give the filter values for
> > each frequency.
> >
>
> I think it's safe to go with disabled/50hz. Perhaps later if needed
> can patch it to control the filter values. Since it seems there is no
> flickering in the 60hz regions at available frame rates, and this
> register almost perfectly removes light flickers in the 50hz regions
> (by modifying exposure/frame rate).
>
> Mosalam
>
Mosalam did you spot the register from a PS3 usb dump or by looking at
the sensor datasheet?
Thanks,
Antonio
On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
>
>
> On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> wrote:
>>
>> On Tue, 2 Mar 2010 11:26:15 +0000
>> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
>>
>> > On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr> wrote:
>> > > On Sun, 28 Feb 2010 20:18:50 +0100
>> > > Antonio Ospite <ospite@studenti.unina.it> wrote:
>> > >
>> > >> Maybe we could just use
>> > >> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
>> > >> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
>> > >>
>> > >> It looks like the code matches the DISABLED state (writing 0 to the
>> > >> register). Mosalam?
>> > >
>> > > I don't know the ov772x sensor. I think it should look like the ov7670
>> > > where there are 3 registers to control the light frequency: one
>> > > register tells if light frequency filter must be used, and which
>> > > frequency 50Hz or 60Hz; the two other ones give the filter values for
>> > > each frequency.
>> > >
>> >
>> > I think it's safe to go with disabled/50hz. Perhaps later if needed
>> > can patch it to control the filter values. Since it seems there is no
>> > flickering in the 60hz regions at available frame rates, and this
>> > register almost perfectly removes light flickers in the 50hz regions
>> > (by modifying exposure/frame rate).
>> >
>> > Mosalam
>> >
>>
>> Mosalam did you spot the register from a PS3 usb dump or by looking at
>> the sensor datasheet?
None, I got that register from sniffing a Windows driver for another
camera that turned out to be using ov7620 or something similar, though
I thought it has the same sensor. I double checked, this register is
for frame rate adjustment (decreasing frame rate / increasing
exposure) . And this has been used in some other drivers (e.g.
gspca_sonixb) to remove light flicker as well.
>>
>> --
>> Antonio Ospite
>> http://ao2.it
>>
>> PGP public key ID: 0x4553B001
>>
>> A: Because it messes up the order in which people normally read text.
>> See http://en.wikipedia.org/wiki/Posting_style
>> Q: Why is top-posting such a bad thing?
>> A: Top-posting.
>> Q: What is the most annoying thing in e-mail?
>
> I'd also like to know where you got the 2b register from, cause someone else
> also said 2b was filtering but the datasheet says it LSB of dummy pixel...
>
>- Max Thrun
Definitely it is adjusting the frame rate (see the ov7620 DS for the
description how the register value is used, for instance). I have no
idea why the ov7720 datasheet says otherwise.
Since this patch does not use the banding filter registers mentioned
in the datasheet maybe should be discarded. I am working on 75 FPS at
VGA, when I get that working well I can get back to this again.
Thanks for the comments.
Mosalam
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3 March 2010 02:30, Max Thrun <bear24rw@gmail.com> wrote:
>
>
> On Tue, Mar 2, 2010 at 9:27 PM, M.Ebrahimi <m.ebrahimi@ieee.org> wrote:
>>
>> On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
>> >
>> >
>> > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite
>> > <ospite@studenti.unina.it>
>> > wrote:
>> >>
>> >> On Tue, 2 Mar 2010 11:26:15 +0000
>> >> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
>> >>
>> >> > On 28 February 2010 19:55, Jean-Francois Moine <moinejf@free.fr>
>> >> > wrote:
>> >> > > On Sun, 28 Feb 2010 20:18:50 +0100
>> >> > > Antonio Ospite <ospite@studenti.unina.it> wrote:
>> >> > >
>> >> > >> Maybe we could just use
>> >> > >> V4L2_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
>> >> > >> V4L2_CID_POWER_LINE_FREQUENCY_50HZ = 1,
>> >> > >>
>> >> > >> It looks like the code matches the DISABLED state (writing 0 to
>> >> > >> the
>> >> > >> register). Mosalam?
>> >> > >
>> >> > > I don't know the ov772x sensor. I think it should look like the
>> >> > > ov7670
>> >> > > where there are 3 registers to control the light frequency: one
>> >> > > register tells if light frequency filter must be used, and which
>> >> > > frequency 50Hz or 60Hz; the two other ones give the filter values
>> >> > > for
>> >> > > each frequency.
>> >> > >
>> >> >
>> >> > I think it's safe to go with disabled/50hz. Perhaps later if needed
>> >> > can patch it to control the filter values. Since it seems there is no
>> >> > flickering in the 60hz regions at available frame rates, and this
>> >> > register almost perfectly removes light flickers in the 50hz regions
>> >> > (by modifying exposure/frame rate).
>> >> >
>> >> > Mosalam
>> >> >
>> >>
>> >> Mosalam did you spot the register from a PS3 usb dump or by looking at
>> >> the sensor datasheet?
>>
>> None, I got that register from sniffing a Windows driver for another
>> camera that turned out to be using ov7620 or something similar, though
>> I thought it has the same sensor. I double checked, this register is
>> for frame rate adjustment (decreasing frame rate / increasing
>> exposure) . And this has been used in some other drivers (e.g.
>> gspca_sonixb) to remove light flicker as well.
>>
>> >>
>> >> --
>> >> Antonio Ospite
>> >> http://ao2.it
>> >>
>> >> PGP public key ID: 0x4553B001
>> >>
>> >> A: Because it messes up the order in which people normally read text.
>> >> See http://en.wikipedia.org/wiki/Posting_style
>> >> Q: Why is top-posting such a bad thing?
>> >> A: Top-posting.
>> >> Q: What is the most annoying thing in e-mail?
>> >
>> > I'd also like to know where you got the 2b register from, cause someone
>> > else
>> > also said 2b was filtering but the datasheet says it LSB of dummy
>> > pixel...
>> >
>> >- Max Thrun
>>
>> Definitely it is adjusting the frame rate (see the ov7620 DS for the
>> description how the register value is used, for instance). I have no
>> idea why the ov7720 datasheet says otherwise.
>>
>> Since this patch does not use the banding filter registers mentioned
>> in the datasheet maybe should be discarded. I am working on 75 FPS at
>> VGA, when I get that working well I can get back to this again.
>>
>> Thanks for the comments.
>> Mosalam
>
> Do you have a link by any chance to ov7620 DS? Also 75 FPS at VGA would be
> awesome (if its possible?). I managed to get QVGA up to 200 the other day..
>
> -- Max Thrun
>
http://mxhaard.free.fr/spca50x/Doc/Omnivision/OV7620.pdf
Mosalam
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 3 Mar 2010 02:27:38 +0000
"M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
> On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
> >
> >
> > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> > wrote:
[...]
> >> Mosalam did you spot the register from a PS3 usb dump or by looking at
> >> the sensor datasheet?
>
> None, I got that register from sniffing a Windows driver for another
> camera that turned out to be using ov7620 or something similar, though
> I thought it has the same sensor. I double checked, this register is
> for frame rate adjustment (decreasing frame rate / increasing
> exposure) . And this has been used in some other drivers (e.g.
> gspca_sonixb) to remove light flicker as well.
>
I see. It would be interesting to see how Powerline Frequency filtering
is done on PS3. I added Jim Paris on CC.
> >
> > I'd also like to know where you got the 2b register from, cause someone else
> > also said 2b was filtering but the datasheet says it LSB of dummy pixel...
> >
> >- Max Thrun
>
> Definitely it is adjusting the frame rate (see the ov7620 DS for the
> description how the register value is used, for instance). I have no
> idea why the ov7720 datasheet says otherwise.
>
> Since this patch does not use the banding filter registers mentioned
> in the datasheet maybe should be discarded. I am working on 75 FPS at
> VGA, when I get that working well I can get back to this again.
>
> Thanks for the comments.
> Mosalam
>
Ok, so Jean-Francois can you apply the patches except 10/11, please?
We are keeping this one for another round.
Thanks,
Antonio
On Wed, 3 Mar 2010 09:00:08 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:
> Ok, so Jean-Francois can you apply the patches except 10/11, please?
> We are keeping this one for another round.
Hello ov534 team,
Actually, I have problems with the mercurial tree. I will apply your
changes as soon as everything will be resolved..
Cheers.
On Wed, 3 Mar 2010 09:37:44 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:
> On Wed, 3 Mar 2010 09:00:08 +0100
> Antonio Ospite <ospite@studenti.unina.it> wrote:
>
> > Ok, so Jean-Francois can you apply the patches except 10/11, please?
> > We are keeping this one for another round.
>
> Hello ov534 team,
>
> Actually, I have problems with the mercurial tree. I will apply your
> changes as soon as everything will be resolved..
>
No problem. Just a question, will you switch to git anytime soon?
Regards,
Antonio
On Wed, 3 Mar 2010 11:57:18 +0100
Antonio Ospite <ospite@studenti.unina.it> wrote:
> No problem. Just a question, will you switch to git anytime soon?
I don't look at this yet, but I think I will have to do it soon...
Antonio Ospite wrote:
> On Wed, 3 Mar 2010 02:27:38 +0000
> "M.Ebrahimi" <m.ebrahimi@ieee.org> wrote:
>
> > On 2 March 2010 16:06, Max Thrun <bear24rw@gmail.com> wrote:
> > >
> > >
> > > On Tue, Mar 2, 2010 at 10:39 AM, Antonio Ospite <ospite@studenti.unina.it>
> > > wrote:
> [...]
> > >> Mosalam did you spot the register from a PS3 usb dump or by looking at
> > >> the sensor datasheet?
> >
> > None, I got that register from sniffing a Windows driver for another
> > camera that turned out to be using ov7620 or something similar, though
> > I thought it has the same sensor. I double checked, this register is
> > for frame rate adjustment (decreasing frame rate / increasing
> > exposure) . And this has been used in some other drivers (e.g.
> > gspca_sonixb) to remove light flicker as well.
> >
>
> I see. It would be interesting to see how Powerline Frequency filtering
> is done on PS3. I added Jim Paris on CC.
Hi Antonio and Mosalam,
I tried, but I can't capture that. My USB logger only does USB 1.1,
which is too slow for the camera to run normally, but good enough to
see the initialization sequence. However, the 50/60Hz option only
appears later, once the PS3 is receiving good frame data.
I can open up the camera and sniff the I2C bus instead. It'll take
a little longer.
Interesting side note, the only change in the initialization sequence
between PS3 firmware 1.93 and 3.15 is 0x0C bit 6 -- horizontal flip :)
So they haven't made any improvements that we can borrow.
-jim
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 3 Mar 2010 23:55:33 -0500
Jim Paris <jim@jtan.com> wrote:
> Antonio Ospite wrote:
[...]
> >
> > I see. It would be interesting to see how Powerline Frequency filtering
> > is done on PS3. I added Jim Paris on CC.
>
> Hi Antonio and Mosalam,
>
> I tried, but I can't capture that. My USB logger only does USB 1.1,
> which is too slow for the camera to run normally, but good enough to
> see the initialization sequence. However, the 50/60Hz option only
> appears later, once the PS3 is receiving good frame data.
>
> I can open up the camera and sniff the I2C bus instead. It'll take
> a little longer.
>
Thanks for your time Jim.
> Interesting side note, the only change in the initialization sequence
> between PS3 firmware 1.93 and 3.15 is 0x0C bit 6 -- horizontal flip :)
> So they haven't made any improvements that we can borrow.
>
> -jim
>
Regards,
Antonio
===================================================================
@@ -66,7 +66,7 @@
s8 sharpness;
u8 hflip;
u8 vflip;
-
+ u8 freqfltr;
};
/* V4L2 controls supported by the driver */
@@ -90,6 +90,10 @@
static int sd_getbrightness(struct gspca_dev *gspca_dev, __s32 *val);
static int sd_setcontrast(struct gspca_dev *gspca_dev, __s32 val);
static int sd_getcontrast(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val);
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val);
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+ struct v4l2_querymenu *menu);
static const struct ctrl sd_ctrls[] = {
{ /* 0 */
@@ -233,6 +237,20 @@
.set = sd_setvflip,
.get = sd_getvflip,
},
+{ /* 10 */
+ {
+ .id = V4L2_CID_POWER_LINE_FREQUENCY,
+ .type = V4L2_CTRL_TYPE_MENU,
+ .name = "Light Frequency Filter",
+ .minimum = 0,
+ .maximum = 1,
+ .step = 1,
+#define FREQFLTR_DEF 1
+ .default_value = FREQFLTR_DEF,
+ },
+ .set = sd_setfreqfltr,
+ .get = sd_getfreqfltr,
+},
};
static const struct v4l2_pix_format ov772x_mode[] = {
@@ -779,6 +797,17 @@
sccb_reg_read(gspca_dev, 0x0c) & ~0x80);
}
+static void setfreqfltr(struct gspca_dev *gspca_dev)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ if (sd->freqfltr == 0)
+ sccb_reg_write(gspca_dev, 0x2b, 0x9e);
+ else
+ sccb_reg_write(gspca_dev, 0x2b, 0x00);
+}
+
+
/* this function is called at probe time */
static int sd_config(struct gspca_dev *gspca_dev,
const struct usb_device_id *id)
@@ -812,6 +841,7 @@
sd->sharpness = SHARPNESS_DEF;
sd->hflip = HFLIP_DEF;
sd->vflip = VFLIP_DEF;
+ sd->freqfltr = FREQFLTR_DEF;
return 0;
}
@@ -881,6 +911,7 @@
setsharpness(gspca_dev);
setvflip(gspca_dev);
sethflip(gspca_dev);
+ setfreqfltr(gspca_dev);
ov534_set_led(gspca_dev, 1);
ov534_reg_write(gspca_dev, 0xe0, 0x00);
@@ -1174,6 +1205,43 @@
return 0;
}
+static int sd_setfreqfltr(struct gspca_dev *gspca_dev, __s32 val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ sd->freqfltr = val;
+ if (gspca_dev->streaming)
+ setfreqfltr(gspca_dev);
+ return 0;
+}
+
+static int sd_getfreqfltr(struct gspca_dev *gspca_dev, __s32 *val)
+{
+ struct sd *sd = (struct sd *) gspca_dev;
+
+ *val = sd->freqfltr;
+ return 0;
+}
+
+static int sd_querymenu(struct gspca_dev *gspca_dev,
+ struct v4l2_querymenu *menu)
+{
+ switch (menu->id) {
+ case V4L2_CID_POWER_LINE_FREQUENCY:
+ switch (menu->index) {
+ case 0: /* V4L2_CID_POWER_LINE_FREQUENCY_50HZ */
+ strcpy((char *) menu->name, "50 Hz");
+ return 0;
+ case 1: /* V4L2_CID_POWER_LINE_FREQUENCY_60HZ */
+ strcpy((char *) menu->name, "60 Hz");
+ return 0;
+ }
+ break;
+ }
+
+ return -EINVAL;
+}
+
/* get stream parameters (framerate) */
static int sd_get_streamparm(struct gspca_dev *gspca_dev,
struct v4l2_streamparm *parm)
@@ -1225,6 +1293,7 @@
.start = sd_start,
.stopN = sd_stopN,
.pkt_scan = sd_pkt_scan,
+ .querymenu = sd_querymenu,
.get_streamparm = sd_get_streamparm,
.set_streamparm = sd_set_streamparm,
};