Mailing List Archive: [PULL] http://linuxtv.org/hg/~awalls/cx18

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

Feb 21, 2009, 6:22 PM

Post #1 of 9 (4832 views)

Mauro,

Please pull from:

http://linuxtv.org/hg/~awalls/cx18

for the following:

cx18: Disable AC3 controls as the firmware doesn't support AC3
cx18: Increment version number due to significant changes for v4l2_subdevs
cx18: Get rid of unused variables related to video output
cx18: Change log lines for internal subdevs and fix tveeprom reads
cx18: Fix a memory leak of buffers used for sliced VBI insertion
cx18: Convert GPIO connected functions to act as v4l2_subdevices
cx18: Convert I2C devices to v4l2_subdevices
cx18, v4l2-chip-ident: Finish conversion of AV decoder core to v4l2_subdev
cx18: Slim down instance handling, build names from v4l2_device.name
cx18: Convert the integrated A/V decoder core interface to a v4l2_subdev

This is the conversion of cx18 to the new v4l_device/v4l2_subdev
framework, along with a few minor changes. (Note, I added a new value
in v4l2-chip-ident, outside of the cx18 directory.)

This set of changes will conflict with Hans' recent pull request that
affects cx18-av-core.c. If your merge tools can't detect that the code
Hans intends to patch simply differs in position by about 20 lines, then
Hans' changes to cx18-av-core.c still should be easy enough to merge by
hand.

Regards,
Andy

diffstat:

drivers/media/video/cx18/cx18-audio.c | 50 -
drivers/media/video/cx18/cx18-audio.h | 2
drivers/media/video/cx18/cx18-av-core.c | 753 ++++++++++++++++------------
drivers/media/video/cx18/cx18-av-core.h | 16
drivers/media/video/cx18/cx18-av-firmware.c | 7
drivers/media/video/cx18/cx18-cards.c | 42 -
drivers/media/video/cx18/cx18-cards.h | 15
drivers/media/video/cx18/cx18-controls.c | 25
drivers/media/video/cx18/cx18-driver.c | 238 ++++----
drivers/media/video/cx18/cx18-driver.h | 100 +++
drivers/media/video/cx18/cx18-fileops.c | 41 -
drivers/media/video/cx18/cx18-firmware.c | 18
drivers/media/video/cx18/cx18-gpio.c | 347 ++++++++----
drivers/media/video/cx18/cx18-gpio.h | 10
drivers/media/video/cx18/cx18-i2c.c | 315 ++---------
drivers/media/video/cx18/cx18-i2c.h | 5
drivers/media/video/cx18/cx18-ioctl.c | 113 +++-
drivers/media/video/cx18/cx18-streams.c | 12
drivers/media/video/cx18/cx18-vbi.c | 3
drivers/media/video/cx18/cx18-version.h | 4
drivers/media/video/cx18/cx18-video.c | 3
include/media/v4l2-chip-ident.h | 3
22 files changed, 1134 insertions(+), 988 deletions(-)

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

mchehab at infradead

Feb 26, 2009, 5:25 PM

Post #2 of 9 (4606 views)

Permalink

On Sat, 21 Feb 2009 21:22:22 -0500
Andy Walls <awalls@radix.net> wrote:

> Mauro,
>
> Please pull from:
>
> http://linuxtv.org/hug/~awalls/cx18
>
> for the following:
>
> cx18: Disable AC3 controls as the firmware doesn't support AC3
> cx18: Increment version number due to significant changes for v4l2_subdevs
> cx18: Get rid of unused variables related to video output
> cx18: Change log lines for internal subdevs and fix tveeprom reads

+ strncpy(c.name, "cx18 tveeprom tmp", sizeof(c.name));
+ c.name[sizeof(c.name)-1] = '\0';

Please use strlcpy() instead.

> cx18: Fix a memory leak of buffers used for sliced VBI insertion
> cx18: Convert GPIO connected functions to act as v4l2_subdevices
> cx18: Convert I2C devices to v4l2_subdevices
> cx18, v4l2-chip-ident: Finish conversion of AV decoder core to v4l2_subdev
> cx18: Slim down instance handling, build names from v4l2_device.name
> cx18: Convert the integrated A/V decoder core interface to a v4l2_subdev
>
> This is the conversion of cx18 to the new v4l_device/v4l2_subdev
> framework, along with a few minor changes. (Note, I added a new value
> in v4l2-chip-ident, outside of the cx18 directory.)
>
> This set of changes will conflict with Hans' recent pull request that
> affects cx18-av-core.c. If your merge tools can't detect that the code
> Hans intends to patch simply differs in position by about 20 lines, then
> Hans' changes to cx18-av-core.c still should be easy enough to merge by
> hand.

There were some merge conflicts. I've fixed it by hand, but I suspect that
you'll need to properly tune the default values for v4l2_ctrl_query_fill().
Please review my last patch and fix it accordingly with the limits of each
V4L2_CID.

Cheers,
Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

mchehab at infradead

Feb 26, 2009, 7:14 PM

Post #3 of 9 (4584 views)

Permalink

Andy and Hans,

On Sat, 21 Feb 2009 21:22:22 -0500
Andy Walls <awalls@radix.net> wrote:

> This set of changes will conflict with Hans' recent pull request that
> affects cx18-av-core.c. If your merge tools can't detect that the code
> Hans intends to patch simply differs in position by about 20 lines, then
> Hans' changes to cx18-av-core.c still should be easy enough to merge by
> hand.

There were several merge conflicts and bisect breakages that happened during
the merge of your trees. Most of the troubles were due the removal of a control
function helper from core.

It took me a large amount of time to fix those conflicts and to be sure that
bisect were not broken during the -git export.

Since I had to do some magic during this merge, it would be really interesting
if you could double check if everything is ok. I've already backported the -git
changes into -hg.

If you find any issues, please send me a patch fixing they.

Cheers,
Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

awalls at radix

Feb 28, 2009, 7:56 AM

Post #4 of 9 (4570 views)

Permalink

On Fri, 2009-02-27 at 00:14 -0300, Mauro Carvalho Chehab wrote:
> Andy and Hans,
>
> On Sat, 21 Feb 2009 21:22:22 -0500
> Andy Walls <awalls@radix.net> wrote:
>
> > This set of changes will conflict with Hans' recent pull request that
> > affects cx18-av-core.c. If your merge tools can't detect that the code
> > Hans intends to patch simply differs in position by about 20 lines, then
> > Hans' changes to cx18-av-core.c still should be easy enough to merge by
> > hand.
>
> There were several merge conflicts and bisect breakages that happened during
> the merge of your trees. Most of the troubles were due the removal of a control
> function helper from core.
>
> It took me a large amount of time to fix those conflicts and to be sure that
> bisect were not broken during the -git export.
>
> Since I had to do some magic during this merge, it would be really interesting
> if you could double check if everything is ok. I've already backported the -git
> changes into -hg.
>
> If you find any issues, please send me a patch fixing they.

Mauro,

I've checked the latest v4l-dvb against all of my intended changes and
Hans' intended change to cx18-av-core.c. My changes are all correct and
your merge of Hans' changes to cx18-av-core.c with mine appears to be
exactly what Hans' intended.

After your merge work, cx18 is OK. No patches from me. :)

Regards,
Andy

> Cheers,
> Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

awalls at radix

Feb 28, 2009, 8:07 AM

Post #5 of 9 (4571 views)

Permalink

On Thu, 2009-02-26 at 22:25 -0300, Mauro Carvalho Chehab wrote:
> On Sat, 21 Feb 2009 21:22:22 -0500
> Andy Walls <awalls@radix.net> wrote:

> > cx18: Change log lines for internal subdevs and fix tveeprom reads
>
> + strncpy(c.name, "cx18 tveeprom tmp", sizeof(c.name));
> + c.name[sizeof(c.name)-1] = '\0';
>
> Please use strlcpy() instead.

Sure. I'm checking the change now. I'll issue a pull request later
this weekend.

> > cx18: Convert the integrated A/V decoder core interface to a v4l2_subdev

> There were some merge conflicts. I've fixed it by hand, but I suspect that
> you'll need to properly tune the default values for v4l2_ctrl_query_fill().
> Please review my last patch and fix it accordingly with the limits of each
> V4L2_CID.

Sorry I didn't get to this sooner (I was traveling).
I see you've already fixed the values. Your fixes are as Hans had
intended them for cx18.

Thanks.

Regards,
Andy

> Cheers,
> Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

[PULL] http://linuxtv.org/hg/~awalls/cx18 [ In reply to ]

awalls at radix

Mar 1, 2009, 7:04 PM

Post #6 of 9 (4557 views)

Permalink

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
cx18: Correct comments about vertical and horizontal blanking timings
cx18: Fix s-parse warnings and a logic error about extracting the VBI PTS
cx18: Include cx18-audio.h in cx18-audio.c to eliminate s-parse warning
cx18: Fix a video scaling check problem introduced by sliced VBI changes
cx18: Use strlcpy() instead of strncpy() for temp eeprom i2c_client setup

Regards,
Andy

diffstat:
cx18-audio.c | 1 +
cx18-av-core.c | 42 +++++++++++++++++++++++++++++++-----------
cx18-av-core.h | 19 ++++++++++++-------
cx18-av-vbi.c | 6 +++---
cx18-controls.c | 40 +++++++++++++++++++++++++++++++---------
cx18-driver.c | 3 +--
cx18-driver.h | 17 ++++++++++++-----
cx18-streams.c | 5 ++---
cx18-vbi.c | 9 +++++----
9 files changed, 98 insertions(+), 44 deletions(-)

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

mchehab at infradead

Mar 3, 2009, 4:50 PM

Post #7 of 9 (4508 views)

Permalink

On Sun, 01 Mar 2009 22:04:55 -0500
Andy Walls <awalls@radix.net> 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_* */
+

Why do you need to include a header for a device that has nothing to do with
cx18? This doesn't belong here...

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.

Cheers,
Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

awalls at radix

Mar 5, 2009, 7:39 PM

Post #8 of 9 (4464 views)

Permalink

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@radix.net> 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.

In the mean time I will issue a new PULL request omitting the portion of
the change you don't like.

Regards,
Andy

>
> Cheers,
> Mauro

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

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

hverkuil at xs4all

Mar 6, 2009, 6:41 AM

Post #9 of 9 (4459 views)

Permalink

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@radix.net> 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

_______________________________________________
ivtv-devel mailing list
ivtv-devel@ivtvdriver.org
http://ivtvdriver.org/mailman/listinfo/ivtv-devel