Re: [RFC PATCH 0/7] saa7134: improve v4l2-compliance

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

 



On Mon January 28 2013 23:40:59 Ondrej Zary wrote:
> On Monday 28 January 2013 11:56:59 Hans Verkuil wrote:
> > On Sun January 27 2013 20:45:05 Ondrej Zary wrote:
> > > Hello,
> > > this patch series improves v4l2-compliance of saa7134 driver. There are
> > > still some problems. Controls require conversion to control framework
> > > which I was unable to finish (because the driver accesses other controls
> > > and also the file handle from within s_ctrl).
> >
> > To convert to the control framework this driver needs quite a bit of work:
> > the saa6752hs driver should be done first (and moved to media/i2c as well
> > as it really doesn't belong here).
> >
> > The filehandle shouldn't be a problem, I think after the prio conversion
> > that's no longer needed at all.
> >
> > > Radio is now OK except for controls.
> > > Video has problems with controls, debugging, formats and buffers:
> > > Debug ioctls:
> > >         test VIDIOC_DBG_G_CHIP_IDENT: OK (Not Supported)
> > >                 fail: v4l2-test-debug.cpp(84): doioctl(node,
> > > VIDIOC_DBG_G_CHIP_IDENT, &chip) Format ioctls:
> > >         test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> > >                 fail: v4l2-test-formats.cpp(836): !cap->readbuffers
> >
> > That should be easy to fix. It's a pretty bogus field and I usually set it
> > to the minimum number of buffers (which is 2 for this driver).
> >
> > >         test VIDIOC_G/S_PARM: FAIL
> > >                 fail: v4l2-test-formats.cpp(335): !fmt.width ||
> > > !fmt.height test VIDIOC_G_FBUF: FAIL
> > >                 fail: v4l2-test-formats.cpp(382): !pix.colorspace
> >
> > That's easy enough to solve. Typically this should be set to
> > V4L2_COLORSPACE_SMPTE170M.
> >
> > But after solving this you'll probably get a bunch of other issues due to
> > a problem this driver shared with quite a few other related drivers: the
> > format state is stored in struct saa7134_fh instead of in the top-level
> > struct. These format states are all global and should never have been
> > placed in this struct.
> 
> Got this after setting colorspace:
>                fail: v4l2-test-formats.cpp(460): win.field == V4L2_FIELD_ANY
> 
> I don't know what win.field is supposed to be and where the value should came 
> from.

>From the spec:

"Applications set this field to determine which video field shall be overlaid,
typically one of V4L2_FIELD_ANY (0), V4L2_FIELD_TOP, V4L2_FIELD_BOTTOM or
V4L2_FIELD_INTERLACED. Drivers may have to choose a different field order
and return the actual setting here."

For this driver V4L2_FIELD_INTERLACED is almost certainly what should be
returned.

> It's now taken from fh->win which is probably all zeros because no 
> overlay is running?
> The same problem is with g_fbuf - what should fmt.width and fmt.height be?

The main problem here is that the win struct is in the filehandle. It should
be in a global struct and set to default values (usually the same size as the
current STD).

Regards,

	Hans
--
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