bttv 2.6.16: wrong VBI_OFFSET?

bttv 2.6.16: wrong VBI_OFFSET?

[Edgar Toernig]

Hi,

in 2.6.16 the code in driver/media/video/bttv-vbi.c was changed
a little bit.  Beside other things, the constant 244 for the vbi
offset was replaced by a #define VBI_OFFSET 128.

Afaics, the old value 244 was correct - was the change to 128
intentionally?

The reason I ask is because it broke the teletext decoder AleVT.
The teletext standard defines where the clock run-in pulses
have to lie (13th bit at 12us/-1us/+0.4us).  For the complete
16 bits of clock run-in that comes down to 9.2us for the start
of the first bit and 12.9us for the end of the last bit.

Here are two oscilloscope shots showing the difference between
offset 244 and 128 - the red line is the interval 9.2us-12.9us:

	http://goron.de/~froese/vbi-offset244.gif
	http://goron.de/~froese/vbi-offset128.gif

As one can see, offset 244 fits very well.  The clock run-in
lies where it's expected.  With offset 128 it's completely
off - no way to find it at that position.

One can also see that the sampling starts somewhere within
the color burst, not at the trailing edge of hsync as some
comment in the code may imply.

Ciao, ET.


PS: In bttv_vb_try_fmt code was changed from 32 to 64 bit
arithmetic:

|        /* s64 to prevent overflow. */
|        count0 = (s64) f->fmt.vbi.start[0] + f->fmt.vbi.count[0]
|                - tvnorm->vbistart[0];
|        ...

What should that change fix?  These values will never overflow -
they fit into 16 bits each.

Re: bttv 2.6.16: wrong VBI_OFFSET?

[matthieu castet]

Hi,

Edgar Toernig wrote:
> Hi,
> 
> in 2.6.16 the code in driver/media/video/bttv-vbi.c was changed
> a little bit.  Beside other things, the constant 244 for the vbi
> offset was replaced by a #define VBI_OFFSET 128.
> 
> Afaics, the old value 244 was correct - was the change to 128
> intentionally?

You can have some comments about that in the git log : 
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=67f1570a0659abba5efbf55cc986187af61bdd52;hp=7e57819169d4f9a1d7af55fb645ece3fb981e2e3


Matthieu

Re: bttv 2.6.16: wrong VBI_OFFSET?

[Edgar Toernig]

matthieu castet wrote:
>
> > in 2.6.16 the code in driver/media/video/bttv-vbi.c was changed
> > a little bit.  Beside other things, the constant 244 for the vbi
> > offset was replaced by a #define VBI_OFFSET 128.
> > 
> > Afaics, the old value 244 was correct - was the change to 128
> > intentionally?
> 
> You can have some comments about that in the git log : 
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=67f1570a0659abba5efbf55cc986187af61bdd52;hp=7e57819169d4f9a1d7af55fb645ece3fb981e2e3

Hmmm... regarding VBI_OFFSET there's this:

| - V4L2_(G|S|TRY)_FMT returned incorrect VBI start lines for PAL-M,
|   NTSC-JP, and PAL-60. They also returned an inaccurate VBI offset.

I remember that I tried to figure out how to calculate the VBI offset
from the Bt8xx specs a couple years ago but resigned - somehow the
specs are wrong.  But given a teletext signal and the teletext specs
you can measure its value[1] and 244 is pretty accurate for PAL.


Regarding the 64 bit arithmetic there's this:

| - V4L2_(S|TRY)_FMT did not expect very large or small VBI start or
|   count values, returning wrong (but safe) counts due to an overflow.

Wow, previously the driver produced (safe) garbage when given garbage
and now it produces more accurate (safe) garbage???  I don't get it.
[My suspicion is that it was only inserted to shut up warnings...]

Ciao, ET.


[1] You know that the 7th peak of the teletext clock run-in is at
12us (+0.4us/-1.0us) from the falling hsync.  You look at which
sample the 7th peak is at its max.  The difference between this
number and 12e-6 * Fs (426 for Fs=35468950) is the VBI_OFFSET.

Btw, hsync ends at 166 (PAL/Fs=35468950) respectively 134 (NTSC/
Fs=28636363).  So any VBI_OFFSET lower than that would show the
hsync in the samples...