Mauro Carvalho Chehab wrote:
> Hi Sakari,
Hi Mauro,
...
> The idea of having a common file handle at the V4L2 core is interesting, but
> I see some troubles on it. As you said on this changeset's comment:
>
> V4L/DVB: V4L: File handles
>
> This patch adds a list of v4l2_fh structures to every video_device.
> It allows using file handle related information in V4L2. The event interface
> is one example of such use.
>
> Video device drivers should use the v4l2_fh pointer as their
> file->private_data.
>
> You're saying that the drivers should use struct v4l2_fh, but there
> aren't any patches on your series enforcing this change.
Oh, there should be an "if" there. It is described better in
Documentation/video4linux/v4l2-framework.txt .
> Also, you're defining it as just:
>
> +struct v4l2_fh {
> + struct list_head list;
> + struct video_device *vdev;
> +};
>
> while other drivers need something much more complex. For example, saa7134
> defines the file handler struct used on file-private_data as:
>
> struct saa7134_fh {
> struct saa7134_dev *dev;
> unsigned int radio;
> enum v4l2_buf_type type;
> unsigned int resources;
> enum v4l2_priority prio;
>
> /* video overlay */
> struct v4l2_window win;
> struct v4l2_clip clips[8];
> unsigned int nclips;
>
> /* video capture */
> struct saa7134_format *fmt;
> unsigned int width,height;
> struct videobuf_queue cap;
> struct saa7134_pgtable pt_cap;
>
> /* vbi capture */
> struct videobuf_queue vbi;
> struct saa7134_pgtable pt_vbi;
> };
>
> Even the simplest driver we have (vivi) require more than just
> the video device:
>
> struct vivi_fh {
> struct vivi_dev *dev;
>
> /* video capture */
> struct vivi_fmt *fmt;
> unsigned int width, height;
> struct videobuf_queue vb_vidq;
>
> enum v4l2_buf_type type;
> unsigned char bars[8][3];
> int input; /* Input Number on bars */
> };
>
>
> So, it is not clear for me how to convert any of the existing
> drivers to fit on your proposal.
The struct v4l2_fh is meant to be a part of the driver's own file handle
structure. It does not attempt to offer drivers anything directly but
generic functionality that depends on file handle specific data such as
V4L2 events. E.g.
struct my_video_fh {
blah;
struct v4l2_fh fh;
blah;
};
> Also, the better is if you could provide us a patchset with the needed
> conversion. It is interesting if you could add some notes there about why
> this change is needed.
There is no need to change any existing drivers, this change does not
affect them. V4L2 file handles are only for those drivers that want to
use them. Other drivers can safely ignore this. But if a driver wants to
use V4L2 events it also has to use V4L2 file handles since event
information is stored in V4L2 file handles.
Cheers,
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
--
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
