RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, September 27, 2011 11:36 PM To: Hiremath, Vaibhav Cc: Ravi, Deepthy; linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm- ker...@lists.infradead.org; linux-ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Vaibhav, On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? The media controller is an API used (among other things) to set the links. You still need to call it from userspace. That won't happen magically. The userspace component that sets up the links and configures the formats, be it a GStreamer element, a libv4l plugin, or something else, can as well setup the standard on the TVP5146 subdev. Please look at from analog device point of view which is interfaced to ISP. OMAP3 ISP = TVP5146 (video decoder) As a user I would want to expect the standard to be supported on streaming device node, since all standard streaming applications (for analog devices/interfaces) does this. Setting up the links and format is still something got added with MC framework, and I would consider it as a separate plug-in along with existing applications. Why do I need to write/use two different streaming application one for MC compatible device and another for non-MC? The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... At then end of the OMAP3 ISP pipeline video data has long lost its analog roots. I don't think standards make sense there. I don't agree with you here, I think we made it clear when we started with MC development activity, we will not break existing standard applications. Media-controller will play a roll to setup the links, connecting the pads and stuff. Thanks, Vaibhav I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Hiremath,
Hiremath, Vaibhav wrote:
-Original Message- From: Laurent Pinchart
[mailto:laurent.pinch...@ideasonboard.com] Sent: Tuesday, September
27, 2011 11:36 PM To: Hiremath, Vaibhav Cc: Ravi, Deepthy;
linux-media@vger.kernel.org; t...@atomide.com;
li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm-
ker...@lists.infradead.org; linux-ker...@vger.kernel.org;
mche...@infradead.org; g.liakhovet...@gmx.de Subject: Re: [PATCH
4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Vaibhav,
On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote:
On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote:
On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
On Thursday, September 08, 2011 10:51 PM Laurent Pinchart
wrote:
On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
From: Vaibhav Hiremath hvaib...@ti.com
In order to support TVP5146 (for that matter any video
decoder), it is important to support G/S/ENUM_STD ioctl
on /dev/videoX device node.
Why so ? Shouldn't it be queried on the subdev output pad
directly ?
Because standard v4l2 application for analog devices will
call these std ioctls on the streaming device node. So it's
done on /dev/video
to
make the existing apllication work.
Existing applications can't work with the OMAP3 ISP (and
similar
complex
embedded devices) without userspace support anyway, either in
the form
of
a GStreamer element or a libv4l plugin. I still believe that
analog
video
standard operations should be added to the subdev pad
operations and exposed through subdev device nodes, exactly as
done with formats.
I completely agree with your point that, existing application
will not
work
without setting links properly. But I believe the assumption here
is, media-controller should set the links (along with pad
formants) and all existing application should work as is. Isn't
it?
The media controller is an API used (among other things) to set the
links. You still need to call it from userspace. That won't happen
magically. The userspace component that sets up the links and
configures the formats, be it a GStreamer element, a libv4l plugin,
or something else, can as well setup the standard on the TVP5146
subdev.
Please look at from analog device point of view which is interfaced
to ISP.
OMAP3 ISP = TVP5146 (video decoder)
As a user I would want to expect the standard to be supported on
streaming device node, since all standard streaming applications (for
analog devices/interfaces) does this.
Setting up the links and format is still something got added with MC
framework, and I would consider it as a separate plug-in along with
existing applications.
Why do I need to write/use two different streaming application one
for MC compatible device and another for non-MC?
You musn't need to.
This is something that will have to be handled by the libv4l plugin, as
the rest of controlling the device. Controls related ioctls will be
passed from the source to downstream once they apply, and I don't see
why the same shouldn't be done to the {G,S,ENUM}_STD.
Regards,
--
Sakari Ailus
sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Em 27-09-2011 19:49, Gary Thomas escreveu: On 2011-09-27 16:31, Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, September 16, 2011 6:36 PM To: Ravi, Deepthy Cc: linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de; Hiremath, Vaibhav Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Deepthy, On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremathhvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. [Hiremath, Vaibhav] Laurent, I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. So how do you handle a part like the TVP5150 that is standard agnostic? That device can sense the standard from the input signal and sets the result appropriately. See the em28xx driver. It uses tvp5150 at the device node, and works properly. It should be noticed, however, that the implementation at tvp5150 doesn't implement standards detection properly, due to hardware limitation. A proper implementation of standards detection is to get the standards mask from userspace and let the driver detect between the standards that the userspace is expecting. So, userspace could, for example, do things like: v4l2_std_id std = V4L2_STD_PAL_M | V4L2_STD_NTSC_M | V4L2_STD_PAL_DK; ioctl (fd, VIDIOC_S_STD, std); msleep (100); ioctl (fd, VIDIOC_G_STD, std); if (std V4L2_STD_625_50) height = 576; else height = 480; The above code won't work with the current tvp5150 driver, due to two reasons: 1) The tvp5150 register 0x28 doesn't allow an arbitrary standards mask like the above. The driver does support standards detection, if V4L2_STD_ALL is passed into it. 2) even if V4L2_STD_ALL is used, the driver currently doesn't implement a vidioc_g_std callback. So, the driver cannot return back to userspace and to the bridge driver what standard were detected. Without this information, userspace might use the wrong number of lines when allocating the buffer, and this won't work. Of course, patches for fixing this are welcome. Thanks, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Em 27-09-2011 19:41, Laurent Pinchart escreveu: Hi Mauro, On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. No. Please see my reply to Vaibhav's e-mail. Standard selection should be done on the subdev pads, for the exact same reason why formats and selection rectangles are configured on subdev pads. NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API. Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Mauro Carvalho Chehab wrote:
Em 27-09-2011 19:41, Laurent Pinchart escreveu:
Hi Mauro,
On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote:
Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu:
On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On
Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote:
On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote:
On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
From: Vaibhav Hiremath hvaib...@ti.com
In order to support TVP5146 (for that matter any video decoder),
it is important to support G/S/ENUM_STD ioctl on /dev/videoX
device node.
Why so ? Shouldn't it be queried on the subdev output pad directly ?
Because standard v4l2 application for analog devices will call these
std ioctls on the streaming device node. So it's done on /dev/video to
make the existing apllication work.
Existing applications can't work with the OMAP3 ISP (and similar complex
embedded devices) without userspace support anyway, either in the form
of a GStreamer element or a libv4l plugin. I still believe that analog
video standard operations should be added to the subdev pad operations
and exposed through subdev device nodes, exactly as done with formats.
I completely agree with your point that, existing application will not
work without setting links properly. But I believe the assumption here
is, media-controller should set the links (along with pad formants) and
all existing application should work as is. Isn't it?
Yes.
The way it is being done currently is, set the format at the pad level
which is same as analog standard resolution and use existing application
for streaming...
Yes.
I am ok, if we add s/g/enum_std api support at sub-dev level but this
should also be supported on streaming device node.
Agreed. Standards selection should be done at device node, just like any
other device.
No. Please see my reply to Vaibhav's e-mail. Standard selection should be
done
on the subdev pads, for the exact same reason why formats and selection
rectangles are configured on subdev pads.
NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API.
That has never been the intention.
My proposal is that the {G,S,ENUM}_STD is implemented in the V4L2 subdev
user space interface and then the libv4l plugin redirects the call to
the right subdev, in a similar fashion which is planned for V4L2 controls.
Kind regards,
--
Sakari Ailus
sakari.ai...@maxwell.research.nokia.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Mauro, On Wednesday 28 September 2011 11:59:30 Mauro Carvalho Chehab wrote: Em 27-09-2011 19:41, Laurent Pinchart escreveu: On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. No. Please see my reply to Vaibhav's e-mail. Standard selection should be done on the subdev pads, for the exact same reason why formats and selection rectangles are configured on subdev pads. NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API. I will NACK any patch that implements G/S/ENUM_STD in the OMAP3 ISP driver itself, so we got a deadlock here :-) Maybe it would be easier to discuss this face to face in Prague ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Wednesday, September 28, 2011 6:59 PM To: Mauro Carvalho Chehab Cc: Hiremath, Vaibhav; Ravi, Deepthy; linux-media@vger.kernel.org; g.liakhovet...@gmx.de; Sakari Ailus Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Mauro, On Wednesday 28 September 2011 11:59:30 Mauro Carvalho Chehab wrote: Em 27-09-2011 19:41, Laurent Pinchart escreveu: On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. No. Please see my reply to Vaibhav's e-mail. Standard selection should be done on the subdev pads, for the exact same reason why formats and selection rectangles are configured on subdev pads. NACK. Let's not reinvent the wheel. the MC should not replace the V4L2 API. I will NACK any patch that implements G/S/ENUM_STD in the OMAP3 ISP driver itself, so we got a deadlock here :-) Maybe it would be easier to discuss this face to face in Prague ? [Hiremath, Vaibhav] I am surprised and afraid to say that, we are breaking existing V4L2 standard applications. We are referring to libv4l, which anyway should follow (compliant to) standard V4L2 spec; and for analog device interface we must support G/S/ENUM_STD ioctl interface. What if I don't want to use libv4l and writing my own application? What if I only want to validate my driver (without libv4l) and the underneath TVP5146 device is being used in both MC and non-MC compatible devices? For example, In my case, I have OMAP3 (with MC support) and AM3517 (without MC support), And I do my whole testing with simple and plain C application which works on both without any change/issues. Streaming application is exactly same, only in case of OMAP3, I have to set the links with media-ctl before streaming. I think we had enough discussion on this, and I don't see you are getting into alignment with this (I am surprised). I vote for F2F discussion, but I will not be available. Hope you will update me on this. Thanks, Vaibhav -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Vaibhav, On Monday 19 September 2011 17:31:02 Hiremath, Vaibhav wrote: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? The media controller is an API used (among other things) to set the links. You still need to call it from userspace. That won't happen magically. The userspace component that sets up the links and configures the formats, be it a GStreamer element, a libv4l plugin, or something else, can as well setup the standard on the TVP5146 subdev. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... At then end of the OMAP3 ISP pipeline video data has long lost its analog roots. I don't think standards make sense there. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, September 16, 2011 6:36 PM To: Ravi, Deepthy Cc: linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de; Hiremath, Vaibhav Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Deepthy, On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. [Hiremath, Vaibhav] Laurent, I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Mauro, On Wednesday 28 September 2011 00:31:32 Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: On Friday, September 16, 2011 6:36 PM Laurent Pinchart wrote: On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. No. Please see my reply to Vaibhav's e-mail. Standard selection should be done on the subdev pads, for the exact same reason why formats and selection rectangles are configured on subdev pads. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
On 2011-09-27 16:31, Mauro Carvalho Chehab wrote: Em 19-09-2011 12:31, Hiremath, Vaibhav escreveu: -Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, September 16, 2011 6:36 PM To: Ravi, Deepthy Cc: linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de; Hiremath, Vaibhav Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Deepthy, On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremathhvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. [Hiremath, Vaibhav] Laurent, I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? Yes. The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... Yes. I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Agreed. Standards selection should be done at device node, just like any other device. So how do you handle a part like the TVP5150 that is standard agnostic? That device can sense the standard from the input signal and sets the result appropriately. -- Gary Thomas | Consulting for the MLB Associates |Embedded world -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
-Original Message- From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com] Sent: Friday, September 16, 2011 6:36 PM To: Ravi, Deepthy Cc: linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk; linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de; Hiremath, Vaibhav Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl Hi Deepthy, On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. [Hiremath, Vaibhav] Laurent, I completely agree with your point that, existing application will not work without setting links properly. But I believe the assumption here is, media-controller should set the links (along with pad formants) and all existing application should work as is. Isn't it? The way it is being done currently is, set the format at the pad level which is same as analog standard resolution and use existing application for streaming... I am ok, if we add s/g/enum_std api support at sub-dev level but this should also be supported on streaming device node. Thanks, Vaibhav -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi,
Sorry for the delayed response.
From: Laurent Pinchart [laurent.pinch...@ideasonboard.com]
Sent: Thursday, September 08, 2011 10:51 PM
To: Ravi, Deepthy
Cc: linux-media@vger.kernel.org; t...@atomide.com; li...@arm.linux.org.uk;
linux-o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
linux-ker...@vger.kernel.org; mche...@infradead.org; g.liakhovet...@gmx.de;
Hiremath, Vaibhav
Subject: Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi,
Thanks for the patch.
On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
From: Vaibhav Hiremath hvaib...@ti.com
In order to support TVP5146 (for that matter any video decoder),
it is important to support G/S/ENUM_STD ioctl on /dev/videoX
device node.
Why so ? Shouldn't it be queried on the subdev output pad directly ?
[Deepthy Ravi] Because standard v4l2 application for analog devices will call
these std ioctls on the streaming device node. So it's done on /dev/video to
make the existing apllication work.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Deepthy Ravi deepthy.r...@ti.com
---
drivers/media/video/omap3isp/ispvideo.c | 98
++- drivers/media/video/omap3isp/ispvideo.h |
1 +
2 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c index d5b8236..ff0ffed 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -37,6 +37,7 @@
#include plat/iovmm.h
#include plat/omap-pm.h
+#include media/tvp514x.h
#include ispvideo.h
#include isp.h
@@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh,
unsigned int *input) static int
isp_video_s_input(struct file *file, void *fh, unsigned int input)
{
- return input == 0 ? 0 : -EINVAL;
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ struct v4l2_routing route;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ if (input == 0)
+ route.input = INPUT_CVBS_VI4A;
+ else
+ route.input = INPUT_SVIDEO_VI2C_VI1C;
+ route.output = 0;
+ ret = v4l2_subdev_call(subdev, video,
s_routing,
+ route.input, route.output, 0);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ ret = v4l2_subdev_call(subdev, video, querystd,
+ a);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ vfh-standard.id = *a;
+ return 0;
+}
+
+static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+
+ mutex_lock(video-mutex);
+ *norm = vfh-standard.id;
+ mutex_unlock(video-mutex);
+
+ return 0;
+}
+
+static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Deepthy, On Friday 16 September 2011 15:00:53 Ravi, Deepthy wrote: On Thursday, September 08, 2011 10:51 PM Laurent Pinchart wrote: On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote: From: Vaibhav Hiremath hvaib...@ti.com In order to support TVP5146 (for that matter any video decoder), it is important to support G/S/ENUM_STD ioctl on /dev/videoX device node. Why so ? Shouldn't it be queried on the subdev output pad directly ? Because standard v4l2 application for analog devices will call these std ioctls on the streaming device node. So it's done on /dev/video to make the existing apllication work. Existing applications can't work with the OMAP3 ISP (and similar complex embedded devices) without userspace support anyway, either in the form of a GStreamer element or a libv4l plugin. I still believe that analog video standard operations should be added to the subdev pad operations and exposed through subdev device nodes, exactly as done with formats. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi Deepathy,
Thanks for the patches.
On Thu, Sep 08, 2011 at 07:05:22PM +0530, Deepthy Ravi wrote:
From: Vaibhav Hiremath hvaib...@ti.com
In order to support TVP5146 (for that matter any video decoder),
it is important to support G/S/ENUM_STD ioctl on /dev/videoX
device node.
Why on video nodes rather than the subdev node?
I don't think *_STD ioctls should be handled differently from any others,
i.e. this is directly related to that subdev, so the control should go
through the subdev node.
That said, generic applications aren't necessarily aware of the subdev nodes
and I think this is something that should be handled in a libv4l plugin.
This appears quite generic to me; walking the graph and accessing the right
subdev node can be done in user space.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Deepthy Ravi deepthy.r...@ti.com
---
drivers/media/video/omap3isp/ispvideo.c | 98
++-
drivers/media/video/omap3isp/ispvideo.h |1 +
2 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c
index d5b8236..ff0ffed 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -37,6 +37,7 @@
#include plat/iovmm.h
#include plat/omap-pm.h
+#include media/tvp514x.h
#include ispvideo.h
#include isp.h
@@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh,
unsigned int *input)
static int
isp_video_s_input(struct file *file, void *fh, unsigned int input)
{
- return input == 0 ? 0 : -EINVAL;
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ struct v4l2_routing route;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ if (input == 0)
+ route.input = INPUT_CVBS_VI4A;
+ else
+ route.input = INPUT_SVIDEO_VI2C_VI1C;
+ route.output = 0;
+ ret = v4l2_subdev_call(subdev, video, s_routing,
+ route.input, route.output, 0);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ ret = v4l2_subdev_call(subdev, video, querystd,
+ a);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ vfh-standard.id = *a;
+ return 0;
+}
+
+static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+
+ mutex_lock(video-mutex);
+ *norm = vfh-standard.id;
+ mutex_unlock(video-mutex);
+
+ return 0;
+}
+
+static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ ret = v4l2_subdev_call(subdev, core, s_std,
+ *norm);
+
Re: [PATCH 4/8] ispvideo: Add support for G/S/ENUM_STD ioctl
Hi,
Thanks for the patch.
On Thursday 08 September 2011 15:35:22 Deepthy Ravi wrote:
From: Vaibhav Hiremath hvaib...@ti.com
In order to support TVP5146 (for that matter any video decoder),
it is important to support G/S/ENUM_STD ioctl on /dev/videoX
device node.
Why so ? Shouldn't it be queried on the subdev output pad directly ?
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Deepthy Ravi deepthy.r...@ti.com
---
drivers/media/video/omap3isp/ispvideo.c | 98
++- drivers/media/video/omap3isp/ispvideo.h |
1 +
2 files changed, 98 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/omap3isp/ispvideo.c
b/drivers/media/video/omap3isp/ispvideo.c index d5b8236..ff0ffed 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -37,6 +37,7 @@
#include plat/iovmm.h
#include plat/omap-pm.h
+#include media/tvp514x.h
#include ispvideo.h
#include isp.h
@@ -1136,7 +1137,97 @@ isp_video_g_input(struct file *file, void *fh,
unsigned int *input) static int
isp_video_s_input(struct file *file, void *fh, unsigned int input)
{
- return input == 0 ? 0 : -EINVAL;
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ struct v4l2_routing route;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ if (input == 0)
+ route.input = INPUT_CVBS_VI4A;
+ else
+ route.input = INPUT_SVIDEO_VI2C_VI1C;
+ route.output = 0;
+ ret = v4l2_subdev_call(subdev, video, s_routing,
+ route.input, route.output, 0);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int isp_video_querystd(struct file *file, void *fh, v4l2_std_id *a)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ ret = v4l2_subdev_call(subdev, video, querystd,
+ a);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ vfh-standard.id = *a;
+ return 0;
+}
+
+static int isp_video_g_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video_fh *vfh = to_isp_video_fh(fh);
+ struct isp_video *video = video_drvdata(file);
+
+ mutex_lock(video-mutex);
+ *norm = vfh-standard.id;
+ mutex_unlock(video-mutex);
+
+ return 0;
+}
+
+static int isp_video_s_std(struct file *file, void *fh, v4l2_std_id *norm)
+{
+ struct isp_video *video = video_drvdata(file);
+ struct media_entity *entity = video-video.entity;
+ struct media_entity_graph graph;
+ struct v4l2_subdev *subdev;
+ int ret = 0;
+
+ media_entity_graph_walk_start(graph, entity);
+ while ((entity = media_entity_graph_walk_next(graph))) {
+ if (media_entity_type(entity) ==
+ MEDIA_ENT_T_V4L2_SUBDEV) {
+ subdev = media_entity_to_v4l2_subdev(entity);
+ if (subdev != NULL) {
+ ret = v4l2_subdev_call(subdev, core, s_std,
+ *norm);
+ if (ret 0 ret != -ENOIOCTLCMD)
+ return ret;
+ }
+ }
+ }
+
+ return 0;
}
static const struct v4l2_ioctl_ops isp_video_ioctl_ops = {
@@ -1161,6 +1252,9 @@ static const struct v4l2_ioctl_ops
isp_video_ioctl_ops = { .vidioc_enum_input= isp_video_enum_input,
.vidioc_g_input = isp_video_g_input,
