Hi Mauro,
> Hi Sakari,
>
> Sakari Ailus wrote:
>> Hi Mauro,
>>
>> The latest event patchset is available in my Gitorious tree now. The
>> file handles documentation patch changed due to unrelated changes in
>> Documentation/video4linux/v4l2-framework.txt. There are no other changes
>> compared to the patches sent to the list earlier.
>>
>> Please pull.
>>
>> The following changes since commit
>> 9178a7c062ff0c43e95d826419f9e9454c52ef15:
>> Mauro Carvalho Chehab (1):
>> V4L/DVB: Fix bad whitespacing
>>
>> are available in the git repository at:
>>
>> git://gitorious.org/omap3camera/mainline.git v4l-dvb-event
>>
>> Sakari Ailus (6):
>> V4L: File handles
>
> 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.
I guess that was an unfortunate phrase in the comment. It should have
been: 'If drivers need the events interface, then they should use
v4l2_fh.'
So this is not required for all drivers. Although it is my intention to
move into that direction, but that will take much more time.
>
> 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.
Drivers will still have their own fh structure, but it embed the v4l2_fh
struct. Just as is done with e.g. struct v4l2_subdev or struct v4l2_device
(or pretty much everywhere in the kernel these days).
>
> 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.
I plan on converting ivtv to this (ivtv already has events using the
poorly defined dvb/video.h events interface). But it looks like that will
take two weeks before I get the time. Although if this would block you
from merging this patch series, then I can try to get it done earlier.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
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
