Re: [RFC] API changes for V4L2_MPEG_SLICED_VBI_FMT_IVTV definitions (Re: [PULL] http://linuxtv.org/hg/~awalls/cx18)

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

 



On Mon, 9 Mar 2009 22:53:44 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> wrote:

> On Monday 09 March 2009 21:47:55 Andy Walls wrote:
> > On Mon, 2009-03-09 at 08:25 +0100, Hans Verkuil wrote:
> > > On Monday 09 March 2009 05:30:54 Mauro Carvalho Chehab wrote:
> > > > On Sat, 7 Mar 2009 09:49:30 +0100
> > > >
> > > > Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxx>
> > > > >
> > > > > 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.
> >
> > I don't have a terribly strong opinion either.  I'm almost done the spec
> > changes (see the diff inline below), but I can easily change things.
> >
> > I don't particularly want to purge IVTV from the
> > V4L2_CID_MPEG_STREAM_VBI_FMT MPEG control enumeration:
> >
> > 	enum v4l2_mpeg_stream_vbi_fmt {
> > 		[...]
> > 		V4L2_MPEG_STREAM_VBI_FMT_IVTV = 1,
> > 	};
> >
> > as that would be a specification change vs. an addition.  Also adding a
> > new ...FMT_ITV0 symbol to the enumeration, that overlaps with a value of
> > 1, creates problems for the controls code. IMO, IVTV is there to stay in
> > that enumeration.
> 
> Right. Unless Mauro objects strongly I think we should just keep it as is. I 
> don't see any particular advantage in changing it.

Ok. Let's then keep the IVTV naming.

> 
> > I can change every other IVTV reference to ITV0 in a sensible way (I
> > think).  Let me know, if you really want this.  I won't be finishing it
> > and posting it for final draft review for a day or two.
> 
> Nice documentation! I think that when this goes in, then README.vbi can be 
> removed.
> 
> Regards,
> 
> 	Hans
> 




Cheers,
Mauro
--
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