| Message ID | 1236389501.3068.10.camel@palomino.walls.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Saturday 07 March 2009 02:31:41 Andy Walls wrote: > On Fri, 2009-03-06 at 15:41 +0100, Hans Verkuil wrote: > > On Friday 06 March 2009 04:39:34 Andy Walls wrote: > > > 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. > > Hans, Mauro, and whoever: > > Before I get too far down the road of writing the spec modifications and > perhaps modifying drivers, in the diff below: > > 1. Are the modifications to the headers acceptable? > > 2. Are they correct? (I *think* they are.) Acked-by: Hans Verkuil <hverkuil@xs4all.nl> Very nice. I was also toying with the idea to rename 'IVTV' in these defines to something different, but that makes too much of a mess. I think it is sufficient to add a sentence to the spec along the lines of: "This format was first introduced in the ivtv driver, hence the use of IVTV in these defines. It is however not limited to the ivtv driver, any MPEG encoder can use it." And I think that it also doesn't hurt if some of my explanations from my earlier email are added to the spec as well. The format looks really weird if you do not understand the PVR350 (cx23415) limitation. Regards, Hans > > Regards, > Andy > > > diff -r 5361470b10f4 linux/include/linux/ivtv.h > --- a/linux/include/linux/ivtv.h Sun Mar 01 21:10:07 2009 -0500 > +++ b/linux/include/linux/ivtv.h Fri Mar 06 20:27:20 2009 -0500 > @@ -60,10 +60,10 @@ > > #define IVTV_IOC_DMA_FRAME _IOW ('V', BASE_VIDIOC_PRIVATE+0, struct > ivtv_dma_frame) > > -/* These are the VBI types as they appear in the embedded VBI private > packets. */ -#define IVTV_SLICED_TYPE_TELETEXT_B (1) > -#define IVTV_SLICED_TYPE_CAPTION_525 (4) > -#define IVTV_SLICED_TYPE_WSS_625 (5) > -#define IVTV_SLICED_TYPE_VPS (7) > +/* Deprecated defines: applications should use the defines from > videodev2.h */ +#define IVTV_SLICED_TYPE_TELETEXT_B > V4L2_MPEG_VBI_IVTV_TELETEXT_B +#define IVTV_SLICED_TYPE_CAPTION_525 > V4L2_MPEG_VBI_IVTV_CAPTION_525 +#define IVTV_SLICED_TYPE_WSS_625 > V4L2_MPEG_VBI_IVTV_WSS_625 +#define IVTV_SLICED_TYPE_VPS > V4L2_MPEG_VBI_IVTV_VPS > > #endif /* _LINUX_IVTV_H */ > diff -r 5361470b10f4 linux/include/linux/videodev2.h > --- a/linux/include/linux/videodev2.h Sun Mar 01 21:10:07 2009 -0500 > +++ b/linux/include/linux/videodev2.h Fri Mar 06 20:27:20 2009 -0500 > @@ -1348,6 +1348,53 @@ > }; > > /* > + * Sliced VBI data inserted into MPEG Streams > + */ > + > +/* > + * V4L2_MPEG_STREAM_VBI_FMT_IVTV: > + * > + * Structure of payload contained in an MPEG 2 Private Stream 1 PES > Packet in an + * MPEG-2 Program Pack that contains > V4L2_MPEG_STREAM_VBI_FMT_IVTV Sliced VBI + * data > + * > + * Note, the MPEG-2 Program Pack and Private Stream 1 PES packet header > + * definitions are not included here. See the MPEG-2 specifications for > details + * on these headers. > + */ > + > +/* Line type IDs */ > +#define V4L2_MPEG_VBI_IVTV_TELETEXT_B (1) > +#define V4L2_MPEG_VBI_IVTV_CAPTION_525 (4) > +#define V4L2_MPEG_VBI_IVTV_WSS_625 (5) > +#define V4L2_MPEG_VBI_IVTV_VPS (7) > + > +struct v4l2_mpeg_vbi_itv0_line { > + __u8 id; /* One of V4L2_MPEG_VBI_IVTV_* above */ > + __u8 data[42]; /* Sliced VBI data for the line */ > +} __attribute__ ((packed)); > + > +struct v4l2_mpeg_vbi_itv0 { > + __u32 linemask[2]; /* Bitmasks of which VBI service lines are present > */ + struct v4l2_mpeg_vbi_itv0_line line[35]; > +} __attribute__ ((packed)); > + > +struct v4l2_mpeg_vbi_ITV0 { > + struct v4l2_mpeg_vbi_itv0_line line[36]; > +} __attribute__ ((packed)); > + > +#define V4L2_MPEG_VBI_IVTV_MAGIC0 "itv0" > +#define V4L2_MPEG_VBI_IVTV_MAGIC1 "ITV0" > + > +struct v4l2_mpeg_vbi_fmt_ivtv { > + __u8 magic[4]; > + union { > + struct v4l2_mpeg_vbi_itv0 itv0; > + struct v4l2_mpeg_vbi_ITV0 ITV0; > + }; > +} __attribute__ ((packed)); > + > +/* > * A G G R E G A T E S T R U C T U R E S > */ > > > > > > > -- > 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 Sat, 7 Mar 2009 09:49:30 +0100 Hans Verkuil <hverkuil@xs4all.nl> wrote: > On Saturday 07 March 2009 02:31:41 Andy Walls wrote: > > On Fri, 2009-03-06 at 15:41 +0100, Hans Verkuil wrote: > > > On Friday 06 March 2009 04:39:34 Andy Walls wrote: > > > > 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. > > > > Hans, Mauro, and whoever: > > > > Before I get too far down the road of writing the spec modifications and > > perhaps modifying drivers, in the diff below: > > > > 1. Are the modifications to the headers acceptable? > > > > 2. Are they correct? (I *think* they are.) > > Acked-by: Hans Verkuil <hverkuil@xs4all.nl> > > Very nice. I was also toying with the idea to rename 'IVTV' in these defines > to something different, but that makes too much of a mess. I think it is > sufficient to add a sentence to the spec along the lines of: > > "This format was first introduced in the ivtv driver, hence the use of IVTV > in these defines. It is however not limited to the ivtv driver, any MPEG > encoder can use it." > > And I think that it also doesn't hurt if some of my explanations from my > earlier email are added to the spec as well. The format looks really weird > if you do not understand the PVR350 (cx23415) limitation. IMO, it is better to remove the IVTV from the name or to replace to something else, since it is meant to be used by other drivers. > +#define V4L2_MPEG_VBI_IVTV_MAGIC0 "itv0" > +#define V4L2_MPEG_VBI_IVTV_MAGIC1 "ITV0" Hmm... maybe we could just name it as format ITV0, as marked at the magic values above. What do you think? Cheers, Mauro -- 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 Monday 09 March 2009 05:30:54 Mauro Carvalho Chehab wrote: > On Sat, 7 Mar 2009 09:49:30 +0100 > > Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On Saturday 07 March 2009 02:31:41 Andy Walls wrote: > > > On Fri, 2009-03-06 at 15:41 +0100, Hans Verkuil wrote: > > > > On Friday 06 March 2009 04:39:34 Andy Walls wrote: > > > > > 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. > > > > > > Hans, Mauro, and whoever: > > > > > > Before I get too far down the road of writing the spec modifications > > > and perhaps modifying drivers, in the diff below: > > > > > > 1. Are the modifications to the headers acceptable? > > > > > > 2. Are they correct? (I *think* they are.) > > > > Acked-by: Hans Verkuil <hverkuil@xs4all.nl> > > > > Very nice. I was also toying with the idea to rename 'IVTV' in these > > defines to something different, but that makes too much of a mess. I > > think it is sufficient to add a sentence to the spec along the lines > > of: > > > > "This format was first introduced in the ivtv driver, hence the use of > > IVTV in these defines. It is however not limited to the ivtv driver, > > any MPEG encoder can use it." > > > > And I think that it also doesn't hurt if some of my explanations from > > my earlier email are added to the spec as well. The format looks really > > weird if you do not understand the PVR350 (cx23415) limitation. > > IMO, it is better to remove the IVTV from the name or to replace to > something else, since it is meant to be used by other drivers. > > > +#define V4L2_MPEG_VBI_IVTV_MAGIC0 "itv0" > > +#define V4L2_MPEG_VBI_IVTV_MAGIC1 "ITV0" > > Hmm... maybe we could just name it as format ITV0, as marked at the magic > values above. What do you think? I don't really see much of an improvement here. I think it is better to put a note in the spec (and perhaps in videodev2.h) that while it originated with ivtv it is not specific to that driver. But I do not have a really strong opinion here. Regards, Hans
diff -r 5361470b10f4 linux/include/linux/ivtv.h --- a/linux/include/linux/ivtv.h Sun Mar 01 21:10:07 2009 -0500 +++ b/linux/include/linux/ivtv.h Fri Mar 06 20:27:20 2009 -0500 @@ -60,10 +60,10 @@ #define IVTV_IOC_DMA_FRAME _IOW ('V', BASE_VIDIOC_PRIVATE+0, struct ivtv_dma_frame) -/* These are the VBI types as they appear in the embedded VBI private packets. */ -#define IVTV_SLICED_TYPE_TELETEXT_B (1) -#define IVTV_SLICED_TYPE_CAPTION_525 (4) -#define IVTV_SLICED_TYPE_WSS_625 (5) -#define IVTV_SLICED_TYPE_VPS (7) +/* Deprecated defines: applications should use the defines from videodev2.h */ +#define IVTV_SLICED_TYPE_TELETEXT_B V4L2_MPEG_VBI_IVTV_TELETEXT_B +#define IVTV_SLICED_TYPE_CAPTION_525 V4L2_MPEG_VBI_IVTV_CAPTION_525 +#define IVTV_SLICED_TYPE_WSS_625 V4L2_MPEG_VBI_IVTV_WSS_625 +#define IVTV_SLICED_TYPE_VPS V4L2_MPEG_VBI_IVTV_VPS #endif /* _LINUX_IVTV_H */ diff -r 5361470b10f4 linux/include/linux/videodev2.h --- a/linux/include/linux/videodev2.h Sun Mar 01 21:10:07 2009 -0500 +++ b/linux/include/linux/videodev2.h Fri Mar 06 20:27:20 2009 -0500 @@ -1348,6 +1348,53 @@ }; /* + * Sliced VBI data inserted into MPEG Streams + */ + +/* + * V4L2_MPEG_STREAM_VBI_FMT_IVTV: + * + * Structure of payload contained in an MPEG 2 Private Stream 1 PES Packet in an + * MPEG-2 Program Pack that contains V4L2_MPEG_STREAM_VBI_FMT_IVTV Sliced VBI + * data + * + * Note, the MPEG-2 Program Pack and Private Stream 1 PES packet header + * definitions are not included here. See the MPEG-2 specifications for details + * on these headers. + */ + +/* Line type IDs */ +#define V4L2_MPEG_VBI_IVTV_TELETEXT_B (1) +#define V4L2_MPEG_VBI_IVTV_CAPTION_525 (4) +#define V4L2_MPEG_VBI_IVTV_WSS_625 (5) +#define V4L2_MPEG_VBI_IVTV_VPS (7) + +struct v4l2_mpeg_vbi_itv0_line { + __u8 id; /* One of V4L2_MPEG_VBI_IVTV_* above */ + __u8 data[42]; /* Sliced VBI data for the line */ +} __attribute__ ((packed)); + +struct v4l2_mpeg_vbi_itv0 { + __u32 linemask[2]; /* Bitmasks of which VBI service lines are present */ + struct v4l2_mpeg_vbi_itv0_line line[35]; +} __attribute__ ((packed)); + +struct v4l2_mpeg_vbi_ITV0 { + struct v4l2_mpeg_vbi_itv0_line line[36]; +} __attribute__ ((packed)); + +#define V4L2_MPEG_VBI_IVTV_MAGIC0 "itv0" +#define V4L2_MPEG_VBI_IVTV_MAGIC1 "ITV0" + +struct v4l2_mpeg_vbi_fmt_ivtv { + __u8 magic[4]; + union { + struct v4l2_mpeg_vbi_itv0 itv0; + struct v4l2_mpeg_vbi_ITV0 ITV0; + }; +} __attribute__ ((packed)); + +/* * A G G R E G A T E S T R U C T U R E S */