* [RFC] Restructure video_device
@ 2009-10-23 14:25 Laurent Pinchart
2009-11-05 14:19 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2009-10-23 14:25 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Sakari Ailus
Hi everybody,
while working on device node support for subdevs I ran into an issue with the
way v4l2 objects are structured.
We currently have the following structure:
- video_device represents a device that complies with the V4L1 or V4L2 API.
Every video_device has a corresponding device node.
- v4l2_device represents a high-level media device that handles sub-devices.
With the new media controller infrastructure a v4l2_device will have a device
node as well.
- v4l2_subdev represents a sub-device. As for v4l2_device's, the new media
controller infrastructure will give a device node for every sub-device.
- v4l2_entity is the structure that both v4l2_subdev and video_device derive
from. Most of the media controller code will deal with entities rather than
sub-devices or video devices, as most operations (such as discovering the
topology and create links) do not depend on the exact nature of the entity.
New types of entities could be introduced later.
Both the video_device and v4l2_subdev structure inherit from v4l2_entity, so
both of them have a v4l2_entity field. With v4l2_device and v4l2_subdev now
needing to devices to have device nodes created, the v4l2_device and
v4l2_subdev structure both have a video_device field.
This isn't clean for two reasons:
- v4l2_device isn't a v4l2_entity, so it should inherit from a structure
(video_device) that itself inherits from v4l2_entity.
- v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and once
through video_device.
To fix this I would like to refactor the video_device structure and cut it in
two pieces. One of them will deal with device node related tasks, being mostly
V4L1/V4L2 agnostic, and the other will inherit from the first and add
V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields from the current
video_device structure), as well as media controller support (inheriting from
v4l2_entity).
My plan was to create a video_devnode structure for the low-level device node
related structure, and keeping the video_device name for the higher level
structure. v4l2_device, v4l2_subdev and video_device would then all have a
video_devnode field.
While this isn't exactly difficult, it would require changing a lot of
drivers, as some field will be moved from video_device to
video_device::video_devnode. Some of those fields are internal, some of them
are accessed by drivers while they shouldn't in most cases (the minor field
for instance), and some are public (name, parent).
I would like to have your opinion on whether you think this proposal is
acceptable or whether you see a better and cleaner way to restructure the
video device code structures.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2009-10-23 14:25 [RFC] Restructure video_device Laurent Pinchart
@ 2009-11-05 14:19 ` Hans Verkuil
2009-11-06 10:23 ` Laurent Pinchart
2011-09-26 11:17 ` Guennadi Liakhovetski
0 siblings, 2 replies; 7+ messages in thread
From: Hans Verkuil @ 2009-11-05 14:19 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Sakari Ailus
On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> Hi everybody,
>
> while working on device node support for subdevs I ran into an issue with the
> way v4l2 objects are structured.
>
> We currently have the following structure:
>
> - video_device represents a device that complies with the V4L1 or V4L2 API.
> Every video_device has a corresponding device node.
>
> - v4l2_device represents a high-level media device that handles sub-devices.
> With the new media controller infrastructure a v4l2_device will have a device
> node as well.
>
> - v4l2_subdev represents a sub-device. As for v4l2_device's, the new media
> controller infrastructure will give a device node for every sub-device.
>
> - v4l2_entity is the structure that both v4l2_subdev and video_device derive
> from. Most of the media controller code will deal with entities rather than
> sub-devices or video devices, as most operations (such as discovering the
> topology and create links) do not depend on the exact nature of the entity.
> New types of entities could be introduced later.
>
> Both the video_device and v4l2_subdev structure inherit from v4l2_entity, so
> both of them have a v4l2_entity field. With v4l2_device and v4l2_subdev now
> needing to devices to have device nodes created, the v4l2_device and
> v4l2_subdev structure both have a video_device field.
>
> This isn't clean for two reasons:
>
> - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> (video_device) that itself inherits from v4l2_entity.
>
> - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and once
> through video_device.
I agree.
> To fix this I would like to refactor the video_device structure and cut it in
> two pieces. One of them will deal with device node related tasks, being mostly
> V4L1/V4L2 agnostic, and the other will inherit from the first and add
> V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields from the current
> video_device structure), as well as media controller support (inheriting from
> v4l2_entity).
>
> My plan was to create a video_devnode structure for the low-level device node
Let's call it v4l2_devnode to be consistent with the current naming convention.
> related structure, and keeping the video_device name for the higher level
> structure. v4l2_device, v4l2_subdev and video_device would then all have a
> video_devnode field.
>
> While this isn't exactly difficult, it would require changing a lot of
> drivers, as some field will be moved from video_device to
> video_device::video_devnode. Some of those fields are internal, some of them
> are accessed by drivers while they shouldn't in most cases (the minor field
> for instance), and some are public (name, parent).
>
> I would like to have your opinion on whether you think this proposal is
> acceptable or whether you see a better and cleaner way to restructure the
> video device code structures.
>
I have two issues with this:
1) Is it really necessary to do this now? We are still in the prototyping
phase and I think it is probably more efficient right now to hack around this
and postpone the real fix (as described above) until we are sure that the mc
concept is working correctly.
2) I'm not sure whether the final media controller will and should be part
of the v4l framework at all. I think that this is something that can be used
separately from the v4l subsystem. So we should be very careful about
integrating this too closely in v4l. Again, this is not much of an issue
while prototyping, but it definitely will need some careful thinking when we
do the final implementation.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2009-11-05 14:19 ` Hans Verkuil
@ 2009-11-06 10:23 ` Laurent Pinchart
2009-11-07 12:36 ` Hans Verkuil
2011-09-26 11:17 ` Guennadi Liakhovetski
1 sibling, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2009-11-06 10:23 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Sakari Ailus
Hi Hans,
On Thursday 05 November 2009 15:19:06 Hans Verkuil wrote:
> On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > Hi everybody,
> >
> > while working on device node support for subdevs I ran into an issue with
> > the way v4l2 objects are structured.
> >
> > We currently have the following structure:
> >
> > - video_device represents a device that complies with the V4L1 or V4L2
> > API. Every video_device has a corresponding device node.
> >
> > - v4l2_device represents a high-level media device that handles
> > sub-devices. With the new media controller infrastructure a v4l2_device
> > will have a device node as well.
> >
> > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new
> > media controller infrastructure will give a device node for every
> > sub-device.
> >
> > - v4l2_entity is the structure that both v4l2_subdev and video_device
> > derive from. Most of the media controller code will deal with entities
> > rather than sub-devices or video devices, as most operations (such as
> > discovering the topology and create links) do not depend on the exact
> > nature of the entity. New types of entities could be introduced later.
> >
> > Both the video_device and v4l2_subdev structure inherit from v4l2_entity,
> > so both of them have a v4l2_entity field. With v4l2_device and
> > v4l2_subdev now needing to devices to have device nodes created, the
> > v4l2_device and v4l2_subdev structure both have a video_device field.
> >
> > This isn't clean for two reasons:
> >
> > - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> > (video_device) that itself inherits from v4l2_entity.
> >
> > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and
> > once through video_device.
>
> I agree.
>
> > To fix this I would like to refactor the video_device structure and cut
> > it in two pieces. One of them will deal with device node related tasks,
> > being mostly V4L1/V4L2 agnostic, and the other will inherit from the
> > first and add V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields
> > from the current video_device structure), as well as media controller
> > support (inheriting from v4l2_entity).
> >
> > My plan was to create a video_devnode structure for the low-level device
> > node
>
> Let's call it v4l2_devnode to be consistent with the current naming
> convention.
Ok.
> > related structure, and keeping the video_device name for the higher level
> > structure. v4l2_device, v4l2_subdev and video_device would then all have
> > a video_devnode field.
> >
> > While this isn't exactly difficult, it would require changing a lot of
> > drivers, as some field will be moved from video_device to
> > video_device::video_devnode. Some of those fields are internal, some of
> > them are accessed by drivers while they shouldn't in most cases (the
> > minor field for instance), and some are public (name, parent).
> >
> > I would like to have your opinion on whether you think this proposal is
> > acceptable or whether you see a better and cleaner way to restructure the
> > video device code structures.
>
> I have two issues with this:
>
> 1) Is it really necessary to do this now? We are still in the prototyping
> phase and I think it is probably more efficient right now to hack around
> this and postpone the real fix (as described above) until we are sure that
> the mc concept is working correctly.
The media controller prototyping code is, as usual with prototyping codes, a
bit messy. Splitting the device node management part from video_device into
v4l2_devnode will make the media controller code easier to understand for
outsiders (by outsider I mean every person who haven't been actively working
on the code, so that includes pretty much everybody). I think it's worth it,
especially given that I've already written the patches. They can live in the
media controller tree of course, we don't have to apply them to mainline at
the moment.
> 2) I'm not sure whether the final media controller will and should be part
> of the v4l framework at all. I think that this is something that can be
> used separately from the v4l subsystem.
I think it should not be part of the v4l subsystem. ALSA will benefit from the
media controller, and so might other subsystems such as GPU. A media_ prefix
would be much nicer.
> So we should be very careful about integrating this too closely in v4l.
> Again, this is not much of an issue while prototyping, but it definitely
> will need some careful thinking when we do the final implementation.
Agreed. Let's rename v4l2_devnode to media_devnode in the future then :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2009-11-06 10:23 ` Laurent Pinchart
@ 2009-11-07 12:36 ` Hans Verkuil
2009-11-09 9:36 ` Laurent Pinchart
0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2009-11-07 12:36 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Sakari Ailus
On Friday 06 November 2009 11:23:59 Laurent Pinchart wrote:
> Hi Hans,
>
> On Thursday 05 November 2009 15:19:06 Hans Verkuil wrote:
> > On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > > Hi everybody,
> > >
> > > while working on device node support for subdevs I ran into an issue with
> > > the way v4l2 objects are structured.
> > >
> > > We currently have the following structure:
> > >
> > > - video_device represents a device that complies with the V4L1 or V4L2
> > > API. Every video_device has a corresponding device node.
> > >
> > > - v4l2_device represents a high-level media device that handles
> > > sub-devices. With the new media controller infrastructure a v4l2_device
> > > will have a device node as well.
> > >
> > > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new
> > > media controller infrastructure will give a device node for every
> > > sub-device.
> > >
> > > - v4l2_entity is the structure that both v4l2_subdev and video_device
> > > derive from. Most of the media controller code will deal with entities
> > > rather than sub-devices or video devices, as most operations (such as
> > > discovering the topology and create links) do not depend on the exact
> > > nature of the entity. New types of entities could be introduced later.
> > >
> > > Both the video_device and v4l2_subdev structure inherit from v4l2_entity,
> > > so both of them have a v4l2_entity field. With v4l2_device and
> > > v4l2_subdev now needing to devices to have device nodes created, the
> > > v4l2_device and v4l2_subdev structure both have a video_device field.
> > >
> > > This isn't clean for two reasons:
> > >
> > > - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> > > (video_device) that itself inherits from v4l2_entity.
> > >
> > > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and
> > > once through video_device.
> >
> > I agree.
> >
> > > To fix this I would like to refactor the video_device structure and cut
> > > it in two pieces. One of them will deal with device node related tasks,
> > > being mostly V4L1/V4L2 agnostic, and the other will inherit from the
> > > first and add V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields
> > > from the current video_device structure), as well as media controller
> > > support (inheriting from v4l2_entity).
> > >
> > > My plan was to create a video_devnode structure for the low-level device
> > > node
> >
> > Let's call it v4l2_devnode to be consistent with the current naming
> > convention.
>
> Ok.
>
> > > related structure, and keeping the video_device name for the higher level
> > > structure. v4l2_device, v4l2_subdev and video_device would then all have
> > > a video_devnode field.
> > >
> > > While this isn't exactly difficult, it would require changing a lot of
> > > drivers, as some field will be moved from video_device to
> > > video_device::video_devnode. Some of those fields are internal, some of
> > > them are accessed by drivers while they shouldn't in most cases (the
> > > minor field for instance), and some are public (name, parent).
> > >
> > > I would like to have your opinion on whether you think this proposal is
> > > acceptable or whether you see a better and cleaner way to restructure the
> > > video device code structures.
> >
> > I have two issues with this:
> >
> > 1) Is it really necessary to do this now? We are still in the prototyping
> > phase and I think it is probably more efficient right now to hack around
> > this and postpone the real fix (as described above) until we are sure that
> > the mc concept is working correctly.
>
> The media controller prototyping code is, as usual with prototyping codes, a
> bit messy. Splitting the device node management part from video_device into
> v4l2_devnode will make the media controller code easier to understand for
> outsiders (by outsider I mean every person who haven't been actively working
> on the code, so that includes pretty much everybody). I think it's worth it,
> especially given that I've already written the patches. They can live in the
> media controller tree of course, we don't have to apply them to mainline at
> the moment.
Ah, it's only for the mc tree. I was getting the impression that you wanted to
do this for the mainline tree as well. But if it is just for the mc tree, then
go ahead. You can just do it in your own tree; as far as I am concerned your
tree is leading for now.
> > 2) I'm not sure whether the final media controller will and should be part
> > of the v4l framework at all. I think that this is something that can be
> > used separately from the v4l subsystem.
>
> I think it should not be part of the v4l subsystem. ALSA will benefit from the
> media controller, and so might other subsystems such as GPU. A media_ prefix
> would be much nicer.
I agree, but let's postpone such decisions until later.
> > So we should be very careful about integrating this too closely in v4l.
> > Again, this is not much of an issue while prototyping, but it definitely
> > will need some careful thinking when we do the final implementation.
>
> Agreed. Let's rename v4l2_devnode to media_devnode in the future then :-)
>
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2009-11-07 12:36 ` Hans Verkuil
@ 2009-11-09 9:36 ` Laurent Pinchart
0 siblings, 0 replies; 7+ messages in thread
From: Laurent Pinchart @ 2009-11-09 9:36 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Sakari Ailus
Hi Hans,
On Saturday 07 November 2009 13:36:33 Hans Verkuil wrote:
> On Friday 06 November 2009 11:23:59 Laurent Pinchart wrote:
> > Hi Hans,
> >
> > On Thursday 05 November 2009 15:19:06 Hans Verkuil wrote:
> > > On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > > > Hi everybody,
> > > >
> > > > while working on device node support for subdevs I ran into an issue
> > > > with the way v4l2 objects are structured.
> > > >
> > > > We currently have the following structure:
> > > >
> > > > - video_device represents a device that complies with the V4L1 or
> > > > V4L2 API. Every video_device has a corresponding device node.
> > > >
> > > > - v4l2_device represents a high-level media device that handles
> > > > sub-devices. With the new media controller infrastructure a
> > > > v4l2_device will have a device node as well.
> > > >
> > > > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new
> > > > media controller infrastructure will give a device node for every
> > > > sub-device.
> > > >
> > > > - v4l2_entity is the structure that both v4l2_subdev and video_device
> > > > derive from. Most of the media controller code will deal with
> > > > entities rather than sub-devices or video devices, as most operations
> > > > (such as discovering the topology and create links) do not depend on
> > > > the exact nature of the entity. New types of entities could be
> > > > introduced later.
> > > >
> > > > Both the video_device and v4l2_subdev structure inherit from
> > > > v4l2_entity, so both of them have a v4l2_entity field. With
> > > > v4l2_device and v4l2_subdev now needing to devices to have device
> > > > nodes created, the v4l2_device and v4l2_subdev structure both have a
> > > > video_device field.
> > > >
> > > > This isn't clean for two reasons:
> > > >
> > > > - v4l2_device isn't a v4l2_entity, so it should inherit from a
> > > > structure (video_device) that itself inherits from v4l2_entity.
> > > >
> > > > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly
> > > > and once through video_device.
> > >
> > > I agree.
> > >
> > > > To fix this I would like to refactor the video_device structure and
> > > > cut it in two pieces. One of them will deal with device node related
> > > > tasks, being mostly V4L1/V4L2 agnostic, and the other will inherit
> > > > from the first and add V4L1/V4L2 support
> > > > (tvnorms/current_norm/ioctl_ops fields from the current video_device
> > > > structure), as well as media controller support (inheriting from
> > > > v4l2_entity).
> > > >
> > > > My plan was to create a video_devnode structure for the low-level
> > > > device node
> > >
> > > Let's call it v4l2_devnode to be consistent with the current naming
> > > convention.
> >
> > Ok.
> >
> > > > related structure, and keeping the video_device name for the higher
> > > > level structure. v4l2_device, v4l2_subdev and video_device would then
> > > > all have a video_devnode field.
> > > >
> > > > While this isn't exactly difficult, it would require changing a lot
> > > > of drivers, as some field will be moved from video_device to
> > > > video_device::video_devnode. Some of those fields are internal, some
> > > > of them are accessed by drivers while they shouldn't in most cases
> > > > (the minor field for instance), and some are public (name, parent).
> > > >
> > > > I would like to have your opinion on whether you think this proposal
> > > > is acceptable or whether you see a better and cleaner way to
> > > > restructure the video device code structures.
> > >
> > > I have two issues with this:
> > >
> > > 1) Is it really necessary to do this now? We are still in the
> > > prototyping phase and I think it is probably more efficient right now
> > > to hack around this and postpone the real fix (as described above)
> > > until we are sure that the mc concept is working correctly.
> >
> > The media controller prototyping code is, as usual with prototyping
> > codes, a bit messy. Splitting the device node management part from
> > video_device into v4l2_devnode will make the media controller code easier
> > to understand for outsiders (by outsider I mean every person who haven't
> > been actively working on the code, so that includes pretty much
> > everybody). I think it's worth it, especially given that I've already
> > written the patches. They can live in the media controller tree of
> > course, we don't have to apply them to mainline at the moment.
>
> Ah, it's only for the mc tree. I was getting the impression that you wanted
> to do this for the mainline tree as well. But if it is just for the mc
> tree, then go ahead. You can just do it in your own tree; as far as I am
> concerned your tree is leading for now.
Ok. I just wanted to make sure there was no huge issue with the proposed
change. I want to avoid writing code that I'll have to completely redesign
later.
> > > 2) I'm not sure whether the final media controller will and should be
> > > part of the v4l framework at all. I think that this is something that
> > > can be used separately from the v4l subsystem.
> >
> > I think it should not be part of the v4l subsystem. ALSA will benefit
> > from the media controller, and so might other subsystems such as GPU. A
> > media_ prefix would be much nicer.
>
> I agree, but let's postpone such decisions until later.
>
> > > So we should be very careful about integrating this too closely in
> > > v4l. Again, this is not much of an issue while prototyping, but it
> > > definitely will need some careful thinking when we do the final
> > > implementation.
> >
> > Agreed. Let's rename v4l2_devnode to media_devnode in the future then :-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2009-11-05 14:19 ` Hans Verkuil
2009-11-06 10:23 ` Laurent Pinchart
@ 2011-09-26 11:17 ` Guennadi Liakhovetski
2011-09-28 9:34 ` Hans Verkuil
1 sibling, 1 reply; 7+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-26 11:17 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Laurent Pinchart, linux-media, Sakari Ailus
Hi Hans
Sorry for reviving an almost 2 year old thread, but the topic, discussed
back then is still relevant (I'll include a complete quote to refresh the
old discussion):
On Thu, 5 Nov 2009, Hans Verkuil wrote:
> On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > Hi everybody,
> >
> > while working on device node support for subdevs I ran into an issue with the
> > way v4l2 objects are structured.
> >
> > We currently have the following structure:
> >
> > - video_device represents a device that complies with the V4L1 or V4L2 API.
> > Every video_device has a corresponding device node.
> >
> > - v4l2_device represents a high-level media device that handles sub-devices.
> > With the new media controller infrastructure a v4l2_device will have a device
> > node as well.
> >
> > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new media
> > controller infrastructure will give a device node for every sub-device.
> >
> > - v4l2_entity is the structure that both v4l2_subdev and video_device derive
> > from. Most of the media controller code will deal with entities rather than
> > sub-devices or video devices, as most operations (such as discovering the
> > topology and create links) do not depend on the exact nature of the entity.
> > New types of entities could be introduced later.
> >
> > Both the video_device and v4l2_subdev structure inherit from v4l2_entity, so
> > both of them have a v4l2_entity field. With v4l2_device and v4l2_subdev now
> > needing to devices to have device nodes created, the v4l2_device and
> > v4l2_subdev structure both have a video_device field.
> >
> > This isn't clean for two reasons:
> >
> > - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> > (video_device) that itself inherits from v4l2_entity.
> >
> > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and once
> > through video_device.
>
> I agree.
>
> > To fix this I would like to refactor the video_device structure and cut it in
> > two pieces. One of them will deal with device node related tasks, being mostly
> > V4L1/V4L2 agnostic, and the other will inherit from the first and add
> > V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields from the current
> > video_device structure), as well as media controller support (inheriting from
> > v4l2_entity).
> >
> > My plan was to create a video_devnode structure for the low-level device node
>
> Let's call it v4l2_devnode to be consistent with the current naming convention.
>
> > related structure, and keeping the video_device name for the higher level
> > structure. v4l2_device, v4l2_subdev and video_device would then all have a
> > video_devnode field.
> >
> > While this isn't exactly difficult, it would require changing a lot of
> > drivers, as some field will be moved from video_device to
> > video_device::video_devnode. Some of those fields are internal, some of them
> > are accessed by drivers while they shouldn't in most cases (the minor field
> > for instance), and some are public (name, parent).
> >
> > I would like to have your opinion on whether you think this proposal is
> > acceptable or whether you see a better and cleaner way to restructure the
> > video device code structures.
> >
>
> I have two issues with this:
>
> 1) Is it really necessary to do this now? We are still in the prototyping
> phase and I think it is probably more efficient right now to hack around this
> and postpone the real fix (as described above) until we are sure that the mc
> concept is working correctly.
Here comes my question: is it the right time for this now?;-) I've relaxed
the problem a bit with this my patch:
http://patchwork.linuxtv.org/patch/7817/
But the problem, described above, when MC _is_ used - that of double
inheritance - still remains. I really think it should be fixed now.
Thanks
Guennadi
>
> 2) I'm not sure whether the final media controller will and should be part
> of the v4l framework at all. I think that this is something that can be used
> separately from the v4l subsystem. So we should be very careful about
> integrating this too closely in v4l. Again, this is not much of an issue
> while prototyping, but it definitely will need some careful thinking when we
> do the final implementation.
>
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC] Restructure video_device
2011-09-26 11:17 ` Guennadi Liakhovetski
@ 2011-09-28 9:34 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2011-09-28 9:34 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Laurent Pinchart, linux-media, Sakari Ailus
On Monday, September 26, 2011 13:17:00 Guennadi Liakhovetski wrote:
> Hi Hans
>
> Sorry for reviving an almost 2 year old thread, but the topic, discussed
> back then is still relevant (I'll include a complete quote to refresh the
> old discussion):
>
> On Thu, 5 Nov 2009, Hans Verkuil wrote:
>
> > On Friday 23 October 2009 16:25:40 Laurent Pinchart wrote:
> > > Hi everybody,
> > >
> > > while working on device node support for subdevs I ran into an issue with the
> > > way v4l2 objects are structured.
> > >
> > > We currently have the following structure:
> > >
> > > - video_device represents a device that complies with the V4L1 or V4L2 API.
> > > Every video_device has a corresponding device node.
> > >
> > > - v4l2_device represents a high-level media device that handles sub-devices.
> > > With the new media controller infrastructure a v4l2_device will have a device
> > > node as well.
> > >
> > > - v4l2_subdev represents a sub-device. As for v4l2_device's, the new media
> > > controller infrastructure will give a device node for every sub-device.
> > >
> > > - v4l2_entity is the structure that both v4l2_subdev and video_device derive
> > > from. Most of the media controller code will deal with entities rather than
> > > sub-devices or video devices, as most operations (such as discovering the
> > > topology and create links) do not depend on the exact nature of the entity.
> > > New types of entities could be introduced later.
> > >
> > > Both the video_device and v4l2_subdev structure inherit from v4l2_entity, so
> > > both of them have a v4l2_entity field. With v4l2_device and v4l2_subdev now
> > > needing to devices to have device nodes created, the v4l2_device and
> > > v4l2_subdev structure both have a video_device field.
> > >
> > > This isn't clean for two reasons:
> > >
> > > - v4l2_device isn't a v4l2_entity, so it should inherit from a structure
> > > (video_device) that itself inherits from v4l2_entity.
> > >
> > > - v4l2_subdev shouldn't inherit twice from v4l2_entity, once directly and once
> > > through video_device.
> >
> > I agree.
> >
> > > To fix this I would like to refactor the video_device structure and cut it in
> > > two pieces. One of them will deal with device node related tasks, being mostly
> > > V4L1/V4L2 agnostic, and the other will inherit from the first and add
> > > V4L1/V4L2 support (tvnorms/current_norm/ioctl_ops fields from the current
> > > video_device structure), as well as media controller support (inheriting from
> > > v4l2_entity).
> > >
> > > My plan was to create a video_devnode structure for the low-level device node
> >
> > Let's call it v4l2_devnode to be consistent with the current naming convention.
> >
> > > related structure, and keeping the video_device name for the higher level
> > > structure. v4l2_device, v4l2_subdev and video_device would then all have a
> > > video_devnode field.
> > >
> > > While this isn't exactly difficult, it would require changing a lot of
> > > drivers, as some field will be moved from video_device to
> > > video_device::video_devnode. Some of those fields are internal, some of them
> > > are accessed by drivers while they shouldn't in most cases (the minor field
> > > for instance), and some are public (name, parent).
> > >
> > > I would like to have your opinion on whether you think this proposal is
> > > acceptable or whether you see a better and cleaner way to restructure the
> > > video device code structures.
> > >
> >
> > I have two issues with this:
> >
> > 1) Is it really necessary to do this now? We are still in the prototyping
> > phase and I think it is probably more efficient right now to hack around this
> > and postpone the real fix (as described above) until we are sure that the mc
> > concept is working correctly.
>
> Here comes my question: is it the right time for this now?;-) I've relaxed
> the problem a bit with this my patch:
>
> http://patchwork.linuxtv.org/patch/7817/
>
> But the problem, described above, when MC _is_ used - that of double
> inheritance - still remains. I really think it should be fixed now.
I'd say, make a proposal for this. I'm not against it and we have enough
experience now to have a good feel for this issue.
I think I would like to put this perhaps in a slightly bigger picture
as well. There are some naming problems as well that I would like to address:
1) v4l2_device is very poorly named. It really represents a root structure from
which to find all other pieces. It does not really represent a device. My proposal
would be to rename it v4l2_root (alternatives are: v4l2_core, v4l2_top, v4l2_sys).
Comments?
2) If v4l2_device becomes available as a name, then I would like to rename
video_device to v4l2_device. Originally I attempted to rename video_device to
v4l2_devnode, but that was opposed by Mauro. I gathered from him that he thinks
v4l2_device would be a proper name for this, but of course it is currently in
use.
Comments?
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-09-28 9:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-23 14:25 [RFC] Restructure video_device Laurent Pinchart
2009-11-05 14:19 ` Hans Verkuil
2009-11-06 10:23 ` Laurent Pinchart
2009-11-07 12:36 ` Hans Verkuil
2009-11-09 9:36 ` Laurent Pinchart
2011-09-26 11:17 ` Guennadi Liakhovetski
2011-09-28 9:34 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.