Re: [PULL] http://linuxtv.org/hg/~awalls/cx18

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 06 March 2009 04:39:34 Andy Walls wrote:
> On Tue, 2009-03-03 at 21:50 -0300, Mauro Carvalho Chehab wrote:
> > On Sun, 01 Mar 2009 22:04:55 -0500
> >
> > Andy Walls <awalls@xxxxxxxxx> wrote:
> > > Mauro,
> > >
> > > Please pull from
> > >
> > > http://linuxtv.org/hg/~awalls/cx18
> > >
> > > for the following
> > >
> > > cx18: Add interlock so sliced VBI insertion only happens for an MPEG
> > > PS
> > >
> > >
> > > cx18: Fix VPS service register code and ensure VBI consistentcy with
> > > ivtv
> >
> > Argh! This is really ugly!
> >
> > +
> > +#include <linux/ivtv.h> /* For IVTV_SLICED_TYPE_* */
> > +
>
> Yes.  It is.
>
> > Why do you need to include a header for a device that has nothing to do
> > with cx18?
>
> I don't.
>
> My rationale was that the cx18 driver supports a VBI data format type
> enumerated in the V4L2 spec and in linux/include/linux/videodev2.h,
> namely:
>
> enum v4l2_mpeg_stream_vbi_fmt {
> 	[...]
>         V4L2_MPEG_STREAM_VBI_FMT_IVTV = 1,  /* VBI in private packets,
> IVTV format */ };
>
> And some values used in that data format are in no other internal nor
> exported header than linux/include/linux/ivtv.h.  The data format itself
> isn't delared at all in any kernel header AFAIK, but only in
> linux/Documentation/video4linux/README.vbi in prose instead of C
> declarations.
>
> >  This doesn't belong here...
>
> OK.
>
> > If those VBI parameters should be global to all devices, then it should
> > be, instead, at some subsystem header, and your patch should also touch
> > on the other drivers.
>
> Yes, they should be exported to userspace as well, so apps like MythTV
> don't have to keep their own copies as well.  I'm going to work on a
> V4L2 spec change to add structures and defines for the packets indicated
> by V4L2_MPEG_STREAM_VBI_FMT_IVTV, and then add it to some API header.
> Maybe in linux/include/linux/videodev2.h with all the other VBI data
> formats.
>
> Unless someone really disagrees.

These VBI defines should be moved to videodev2.h. In hindsight this should 
never have been added to ivtv.h. Originally only ivtv used this, but now 
cx18 does as well, and in theory any MPEG encoder device can use it.

Due to the popularity of the ivtv driver it has become a de-facto standard. 
There are other standards for embedding e.g. closed captions in an MPEG 
stream. But the advantage of this IVTV standard is that it allows any VBI 
type to be embedded and supports the maximum number of VBI lines.

The somewhat awkward encoding is a result of a limitation of the PVR-350 
MPEG decoder. That decoder can read back the embedded VBI data, but it only 
reads back a limited number of bytes, any data beyond that maximum is 
ignored. So the maximum size of the embedded VBI data had to be limited to 
what the PVR-350 decoder supports.

It was always my intention to add support for other, more official, methods 
of including closed captions into an MPEG stream, but by that time all the 
main applications already supported this IVTV-specific format :-)

Looking back on this I wish I used a standard method of embedding this data, 
even though that might have meant that the VBI packet size could have been 
larger than the maximum supported by the PVR-350. It has been my experience 
that in practice you never see PAL transmissions that use the maximum 
number of VBI lines.

Oh well, you live and learn...

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux