Message ID | 4D889C61.905@matrix-vision.de (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers |
Return-path: <mchehab@pedra> Envelope-to: mchehab@pedra Delivery-date: Tue, 22 Mar 2011 09:56:38 -0300 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from <mchehab@pedra>) id 1Q218Y-0000xP-KK for mchehab@pedra; Tue, 22 Mar 2011 09:56:38 -0300 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Tue, 22 Mar 2011 09:56:38 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1Q2187-0001S2-1p; Tue, 22 Mar 2011 12:56:11 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755981Ab1CVM4E (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Tue, 22 Mar 2011 08:56:04 -0400 Received: from mail2.matrix-vision.com ([85.214.244.251]:38393 "EHLO mail2.matrix-vision.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755923Ab1CVM4D (ORCPT <rfc822;linux-media@vger.kernel.org>); Tue, 22 Mar 2011 08:56:03 -0400 Received: from mail2.matrix-vision.com (localhost [127.0.0.1]) by mail2.matrix-vision.com (Postfix) with ESMTP id 3B2263F66F; Tue, 22 Mar 2011 13:56:02 +0100 (CET) Received: from erinome (g2.matrix-vision.com [80.152.136.245]) by mail2.matrix-vision.com (Postfix) with ESMTPA id ED4E33F64D; Tue, 22 Mar 2011 13:56:01 +0100 (CET) Received: from erinome (localhost [127.0.0.1]) by erinome (Postfix) with ESMTP id 974126F8A; Tue, 22 Mar 2011 13:56:01 +0100 (CET) Received: by erinome (Postfix, from userid 108) id 8C7AA6F9C; Tue, 22 Mar 2011 13:56:01 +0100 (CET) Received: from [192.168.65.46] (host65-46.intern.matrix-vision.de [192.168.65.46]) by erinome (Postfix) with ESMTPA id 7A8C36F8A; Tue, 22 Mar 2011 13:56:01 +0100 (CET) Message-ID: <4D889C61.905@matrix-vision.de> Date: Tue, 22 Mar 2011 13:56:01 +0100 From: Michael Jones <michael.jones@matrix-vision.de> User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101029 Lightning/1.0b2 Thunderbird/3.1.6 MIME-Version: 1.0 To: Linux Media Mailing List <linux-media@vger.kernel.org> CC: =?ISO-8859-1?Q?Lo=EFc_Akue?= <akue.loic@gmail.com>, Laurent Pinchart <laurent.pinchart@ideasonboard.com> Subject: [PATCH] omap3isp: implement ENUM_FMT Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-MV-Disclaimer: true (erinome) X-AV-Checked: ClamAV using ClamSMTP (erinome) X-AV-Checked: ClamAV using ClamSMTP (mail2) Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org Sender: <mchehab@pedra> |
Commit Message
Michael Jones
March 22, 2011, 12:56 p.m. UTC
From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 From: Michael Jones <michael.jones@matrix-vision.de> Date: Tue, 22 Mar 2011 11:47:22 +0100 Subject: [PATCH] omap3isp: implement ENUM_FMT Whatever format is currently being delivered will be declared as the only possible format Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> --- Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. This patch doesn't enumerate all of the formats which could possibly be set (as is intended by ENUM_FMT), but at least it reports the one that is currently set. This patch applies to Laurent's 'media-2.6.38-0001-omap3isp' branch. drivers/media/video/omap3isp/ispvideo.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-)
Comments
Hi Michael, Thanks for the patch. Michael Jones wrote: > From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 > From: Michael Jones <michael.jones@matrix-vision.de> > Date: Tue, 22 Mar 2011 11:47:22 +0100 > Subject: [PATCH] omap3isp: implement ENUM_FMT > > Whatever format is currently being delivered will be declared as the only > possible format > > Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> > --- > > Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. > This patch doesn't enumerate all of the formats which could possibly be > set (as is intended by ENUM_FMT), but at least it reports the one that > is currently set. What would be the purpose of ENUM_FMT in this case? It provides no additional information to user space, and the information it provides is in fact incomplete. Using other formats is possible, but that requires changes to the format configuration on links. As the relevant format configuration is done on the subdevs and not on the video nodes, the format configuration on the video nodes is very limited and much affected by the state of the formats on the subdev pads (which I think is right). This is not limited to ENUM_FMT but all format related IOCTLs on the OMAP 3 ISP driver. My view is that should a generic application want to change (or enumerate) the format(s) on a video node, the application would need to be using libv4l for that. A compatibility layer implemented in libv4l (plugin, not the main library) needs to configure the links in the first place, so implementing ENUM_FMT in the plugin would not be a big deal. It could even provide useful information. The possible results of the ENUM_FMT would also depend on what kind of pipeline configuration does the plugin support, though. (Cc Yordan and Hans.) I discussed this with Laurent initially and the conclusion was that more discussion is required. :-) Hans: do you have an opinion on this? Best regards,
Hi Sakari, On 03/23/2011 10:52 AM, Sakari Ailus wrote: > Hi Michael, > > Thanks for the patch. > > Michael Jones wrote: >> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 >> From: Michael Jones <michael.jones@matrix-vision.de> >> Date: Tue, 22 Mar 2011 11:47:22 +0100 >> Subject: [PATCH] omap3isp: implement ENUM_FMT >> >> Whatever format is currently being delivered will be declared as the only >> possible format >> >> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >> --- >> >> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. >> This patch doesn't enumerate all of the formats which could possibly be >> set (as is intended by ENUM_FMT), but at least it reports the one that >> is currently set. > > What would be the purpose of ENUM_FMT in this case? It provides no > additional information to user space, and the information it provides is > in fact incomplete. Using other formats is possible, but that requires > changes to the format configuration on links. The only purpose of it was to provide minimum functionality for apps to be able to fetch frames from the ISP after setting up the ISP pipeline with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had also recently asked Laurent about. > > As the relevant format configuration is done on the subdevs and not on > the video nodes, the format configuration on the video nodes is very > limited and much affected by the state of the formats on the subdev pads > (which I think is right). This is not limited to ENUM_FMT but all format > related IOCTLs on the OMAP 3 ISP driver. > > My view is that should a generic application want to change (or > enumerate) the format(s) on a video node, the application would need to > be using libv4l for that. > > A compatibility layer implemented in libv4l (plugin, not the main > library) needs to configure the links in the first place, so > implementing ENUM_FMT in the plugin would not be a big deal. It could > even provide useful information. The possible results of the ENUM_FMT > would also depend on what kind of pipeline configuration does the plugin > support, though. BTW, my GStreamer is using libv4l, although it looked like it's also possible to configure GStreamer to use ioctls directly. I can agree that it would be nice to implement ENUM_FMT and the like in a compatibility layer in libv4l. That would be in the true spirit of ENUM_FMT, where the app could actually see different formats it can set. But is there any work being done on such a compatibility layer? Is there a policy decision that in the future, apps will be required to use libv4l to get images from the ISP? Are we not intending to support using e.g. media-ctl + some v4l2 app, as I'm currently doing during development? In the meantime, I will continue using this patch locally to enable getting a live image with Gstreamer, and it can at least serve as a help to Loïc if he's trying to do the same. > > (Cc Yordan and Hans.) > > I discussed this with Laurent initially and the conclusion was that more > discussion is required. :-) Hans: do you have an opinion on this? > > Best regards, > thanks for the discussion, Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
Hi Michael, On Wednesday 23 March 2011 12:07:12 Michael Jones wrote: > On 03/23/2011 10:52 AM, Sakari Ailus wrote: > > Michael Jones wrote: > >> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 > >> From: Michael Jones <michael.jones@matrix-vision.de> > >> Date: Tue, 22 Mar 2011 11:47:22 +0100 > >> Subject: [PATCH] omap3isp: implement ENUM_FMT > >> > >> Whatever format is currently being delivered will be declared as the > >> only possible format > >> > >> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> > >> --- > >> > >> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. > >> This patch doesn't enumerate all of the formats which could possibly be > >> set (as is intended by ENUM_FMT), but at least it reports the one that > >> is currently set. > > > > What would be the purpose of ENUM_FMT in this case? It provides no > > additional information to user space, and the information it provides is > > in fact incomplete. Using other formats is possible, but that requires > > changes to the format configuration on links. > > The only purpose of it was to provide minimum functionality for apps to > be able to fetch frames from the ISP after setting up the ISP pipeline > with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had > also recently asked Laurent about. > > > As the relevant format configuration is done on the subdevs and not on > > the video nodes, the format configuration on the video nodes is very > > limited and much affected by the state of the formats on the subdev pads > > (which I think is right). This is not limited to ENUM_FMT but all format > > related IOCTLs on the OMAP 3 ISP driver. > > > > My view is that should a generic application want to change (or > > enumerate) the format(s) on a video node, the application would need to > > be using libv4l for that. > > > > A compatibility layer implemented in libv4l (plugin, not the main > > library) needs to configure the links in the first place, so > > implementing ENUM_FMT in the plugin would not be a big deal. It could > > even provide useful information. The possible results of the ENUM_FMT > > would also depend on what kind of pipeline configuration does the plugin > > support, though. > > BTW, my GStreamer is using libv4l, although it looked like it's also > possible to configure GStreamer to use ioctls directly. I can agree > that it would be nice to implement ENUM_FMT and the like in a > compatibility layer in libv4l. That would be in the true spirit of > ENUM_FMT, where the app could actually see different formats it can set. > > But is there any work being done on such a compatibility layer? > > Is there a policy decision that in the future, apps will be required to > use libv4l to get images from the ISP? Are we not intending to support > using e.g. media-ctl + some v4l2 app, as I'm currently doing during > development? Apps should be able to use the V4L2 API directly. However, we can't implement all that API, as most calls don't make sense for the OMA3 ISP driver. Which calls need to be implemented is a grey area at the moment, as there's no detailed semantics on how subdev-level configuration and video device configuration should interact. Your implementation of ENUM_FMT looks correct to me, but the question is whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a required ioctl, so maybe v4l2src shouldn't depend on it. I'm interesting in getting Hans' opinion on this. > In the meantime, I will continue using this patch locally to enable > getting a live image with Gstreamer, and it can at least serve as a help > to Loïc if he's trying to do the same.
Hi Laurent, On 03/23/2011 01:16 PM, Laurent Pinchart wrote: > Hi Michael, > [snip] >> >> Is there a policy decision that in the future, apps will be required to >> use libv4l to get images from the ISP? Are we not intending to support >> using e.g. media-ctl + some v4l2 app, as I'm currently doing during >> development? > > Apps should be able to use the V4L2 API directly. However, we can't implement > all that API, as most calls don't make sense for the OMA3 ISP driver. Which > calls need to be implemented is a grey area at the moment, as there's no > detailed semantics on how subdev-level configuration and video device > configuration should interact. > > Your implementation of ENUM_FMT looks correct to me, but the question is > whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a required > ioctl, so maybe v4l2src shouldn't depend on it. I'm interesting in getting > Hans' opinion on this. > I only implemented it after I saw that ENUM_FMT _was_ required by V4L2. From http://v4l2spec.bytesex.org/spec/x1859.htm#AEN1894 : "The VIDIOC_ENUM_FMT ioctl must be supported by all drivers exchanging image data with applications." -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
On Thursday, March 24, 2011 08:28:31 Michael Jones wrote: > Hi Laurent, > > On 03/23/2011 01:16 PM, Laurent Pinchart wrote: > > Hi Michael, > > > [snip] > >> > >> Is there a policy decision that in the future, apps will be required to > >> use libv4l to get images from the ISP? Are we not intending to support > >> using e.g. media-ctl + some v4l2 app, as I'm currently doing during > >> development? > > > > Apps should be able to use the V4L2 API directly. However, we can't implement > > all that API, as most calls don't make sense for the OMA3 ISP driver. Which > > calls need to be implemented is a grey area at the moment, as there's no > > detailed semantics on how subdev-level configuration and video device > > configuration should interact. We definitely need to discuss this in the near future. It's indeed a grey area at the moment that needs to be clarified. > > Your implementation of ENUM_FMT looks correct to me, but the question is > > whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a required > > ioctl, so maybe v4l2src shouldn't depend on it. I'm interesting in getting > > Hans' opinion on this. > > > > I only implemented it after I saw that ENUM_FMT _was_ required by V4L2. > From http://v4l2spec.bytesex.org/spec/x1859.htm#AEN1894 : > "The VIDIOC_ENUM_FMT ioctl must be supported by all drivers exchanging > image data with applications." If you can call S_FMT on a device node, then you also have to implement ENUM_FMT. I am assuming applications need to call S_FMT for omap3 video nodes, right? Because that defines the result of the DMA engine. Or is the result always fixed, based on the current pipeline configuration? In the latter case I would still expect to see an ENUM_FMT, but one that just returns the current format. And S/TRY_FMT would also return the current format. Regards, Hans -- 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
Michael Jones wrote: > Hi Sakari, Hi Michael, > On 03/23/2011 10:52 AM, Sakari Ailus wrote: >> Hi Michael, >> >> Thanks for the patch. >> >> Michael Jones wrote: >>> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 >>> From: Michael Jones <michael.jones@matrix-vision.de> >>> Date: Tue, 22 Mar 2011 11:47:22 +0100 >>> Subject: [PATCH] omap3isp: implement ENUM_FMT >>> >>> Whatever format is currently being delivered will be declared as the only >>> possible format >>> >>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >>> --- >>> >>> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. >>> This patch doesn't enumerate all of the formats which could possibly be >>> set (as is intended by ENUM_FMT), but at least it reports the one that >>> is currently set. >> >> What would be the purpose of ENUM_FMT in this case? It provides no >> additional information to user space, and the information it provides is >> in fact incomplete. Using other formats is possible, but that requires >> changes to the format configuration on links. > > The only purpose of it was to provide minimum functionality for apps to > be able to fetch frames from the ISP after setting up the ISP pipeline > with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had > also recently asked Laurent about. Are you familiar with the mcsrc? <URL:http://meego.gitorious.org/maemo-multimedia/gst-nokia-videosrc> I think it's Also, I think v4lsrc could be fixed to cope with lack of VIDIOC_ENUM_FMT. Besides what Laurent already mentioned, how the applications are expected to select a video node from the 7 (or 8) that the ISP has is not resolved yet. The requested pixelformat depends on the video node in this case. >> As the relevant format configuration is done on the subdevs and not on >> the video nodes, the format configuration on the video nodes is very >> limited and much affected by the state of the formats on the subdev pads >> (which I think is right). This is not limited to ENUM_FMT but all format >> related IOCTLs on the OMAP 3 ISP driver. >> >> My view is that should a generic application want to change (or >> enumerate) the format(s) on a video node, the application would need to >> be using libv4l for that. >> >> A compatibility layer implemented in libv4l (plugin, not the main >> library) needs to configure the links in the first place, so >> implementing ENUM_FMT in the plugin would not be a big deal. It could >> even provide useful information. The possible results of the ENUM_FMT >> would also depend on what kind of pipeline configuration does the plugin >> support, though. > > BTW, my GStreamer is using libv4l, although it looked like it's also > possible to configure GStreamer to use ioctls directly. I can agree > that it would be nice to implement ENUM_FMT and the like in a > compatibility layer in libv4l. That would be in the true spirit of > ENUM_FMT, where the app could actually see different formats it can set. > > But is there any work being done on such a compatibility layer? Yes. Yordan is working on that. There's a plugin interface patch for libv4l and a MC-aware plugin that uses the interface. <URL:http://www.spinics.net/lists/linux-media/msg28925.html> I think so far only the plugin interface patch is on the list. > Is there a policy decision that in the future, apps will be required to > use libv4l to get images from the ISP? Are we not intending to support > using e.g. media-ctl + some v4l2 app, as I'm currently doing during > development? If we speak about specific applications that are aware of the MC and the OMAP 3 ISP itself, I'd guess those will continue to use the interfaces provided by the driver directly. But generic applications would likely be better off using libv4l. This way they could use whatever there is in the libv4l plugin for the platform (automatic exposure and white balance, for example). I don't think generic applications _shouldn't_ be allowed to do without libv4l. It's just that the functionality provided by the driver in this case will be limited to a default pipeline and fixed format (as was your patch for ENUM_FMT). > In the meantime, I will continue using this patch locally to enable > getting a live image with Gstreamer, and it can at least serve as a help > to Loïc if he's trying to do the same. Are there other applications that really need ENUM_FMT? The spec says of ENUM_FMT that: "To enumerate image formats applications initialize the type and index field of struct v4l2_fmtdesc and call the VIDIOC_ENUM_FMT ioctl with a pointer to this structure. Drivers fill the rest of the structure or return an EINVAL error code. All formats are enumerable by beginning at index zero and incrementing by one until EINVAL is returned." As Laurent pointed out, I also agree that the patch itself is correct but the information provided to the user space is correct but incomplete. Coming up with a list of formats on a video node in the ISP requires information from different parts of the MC graph. I somehow feel it shouldn't be the responsibility of the driver to provide this since using those formats requires different pad formats. Various parts of the driver would need to be notified when links change somewhere else. Also, if a video node isn't connected to anything through an enabled link or the entity it's connected to, isn't, what should ENUM_FMT provide then? An empty list? Regards,
Hans Verkuil wrote: > On Thursday, March 24, 2011 08:28:31 Michael Jones wrote: >> Hi Laurent, >> >> On 03/23/2011 01:16 PM, Laurent Pinchart wrote: >>> Hi Michael, >>> >> [snip] >>>> >>>> Is there a policy decision that in the future, apps will be required to >>>> use libv4l to get images from the ISP? Are we not intending to support >>>> using e.g. media-ctl + some v4l2 app, as I'm currently doing during >>>> development? >>> >>> Apps should be able to use the V4L2 API directly. However, we can't implement >>> all that API, as most calls don't make sense for the OMA3 ISP driver. Which >>> calls need to be implemented is a grey area at the moment, as there's no >>> detailed semantics on how subdev-level configuration and video device >>> configuration should interact. > > We definitely need to discuss this in the near future. It's indeed a grey > area at the moment that needs to be clarified. I fully agree. >>> Your implementation of ENUM_FMT looks correct to me, but the question is >>> whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a required >>> ioctl, so maybe v4l2src shouldn't depend on it. I'm interesting in getting >>> Hans' opinion on this. >>> >> >> I only implemented it after I saw that ENUM_FMT _was_ required by V4L2. >> From http://v4l2spec.bytesex.org/spec/x1859.htm#AEN1894 : >> "The VIDIOC_ENUM_FMT ioctl must be supported by all drivers exchanging >> image data with applications." > > If you can call S_FMT on a device node, then you also have to implement > ENUM_FMT. I think the issue here is that it's not possible (or feasible) to implement ENUM_FMT in a way applications would obtain the information they're interested in. Available pixel formats are dictated by the formats on links (validation must be done first in a generic case!) and in the case of OMAP 3 ISP there's always just one. S_FMT and TRY_FMT behave more or less the way applications like. The pixelformat or size may not change, though, and this information is also used to prepare the buffers. > I am assuming applications need to call S_FMT for omap3 video nodes, right? > Because that defines the result of the DMA engine. Or is the result always > fixed, based on the current pipeline configuration? In the latter case I > would still expect to see an ENUM_FMT, but one that just returns the current > format. And S/TRY_FMT would also return the current format. There are some options in the format that is not defined by the v4l2_mbus_pixelcode already. Padding is such and I don't think there should be anything else than that since the v4l2_mbus_pixelcode conversion and scaling takes places in the subdevs. Laurent? :-) Cheers,
Hi, On Thursday 24 March 2011 09:13:20 Sakari Ailus wrote: > Hans Verkuil wrote: > > On Thursday, March 24, 2011 08:28:31 Michael Jones wrote: > >> On 03/23/2011 01:16 PM, Laurent Pinchart wrote: > >>> Hi Michael, > >> > >> [snip] > >> > >>>> Is there a policy decision that in the future, apps will be required > >>>> to use libv4l to get images from the ISP? Are we not intending to > >>>> support using e.g. media-ctl + some v4l2 app, as I'm currently doing > >>>> during development? > >>> > >>> Apps should be able to use the V4L2 API directly. However, we can't > >>> implement all that API, as most calls don't make sense for the OMA3 > >>> ISP driver. Which calls need to be implemented is a grey area at the > >>> moment, as there's no detailed semantics on how subdev-level > >>> configuration and video device configuration should interact. > > > > We definitely need to discuss this in the near future. It's indeed a grey > > area at the moment that needs to be clarified. > > I fully agree. > > >>> Your implementation of ENUM_FMT looks correct to me, but the question > >>> is whether ENUM_FMT should be implemented. I don't think ENUM_FMT is a > >>> required ioctl, so maybe v4l2src shouldn't depend on it. I'm > >>> interesting in getting Hans' opinion on this. > >> > >> I only implemented it after I saw that ENUM_FMT _was_ required by V4L2. > >> > >> From http://v4l2spec.bytesex.org/spec/x1859.htm#AEN1894 : > >> "The VIDIOC_ENUM_FMT ioctl must be supported by all drivers exchanging > >> image data with applications." Good point. > > If you can call S_FMT on a device node, then you also have to implement > > ENUM_FMT. > > I think the issue here is that it's not possible (or feasible) to > implement ENUM_FMT in a way applications would obtain the information > they're interested in. Available pixel formats are dictated by the > formats on links (validation must be done first in a generic case!) and > in the case of OMAP 3 ISP there's always just one. > > S_FMT and TRY_FMT behave more or less the way applications like. The > pixelformat or size may not change, though, and this information is also > used to prepare the buffers. > > > I am assuming applications need to call S_FMT for omap3 video nodes, > > right? Because that defines the result of the DMA engine. Or is the > > result always fixed, based on the current pipeline configuration? In the > > latter case I would still expect to see an ENUM_FMT, but one that just > > returns the current format. And S/TRY_FMT would also return the current > > format. > > There are some options in the format that is not defined by the > v4l2_mbus_pixelcode already. Padding is such and I don't think there > should be anything else than that since the v4l2_mbus_pixelcode > conversion and scaling takes places in the subdevs. Laurent? :-) Padding at end of line can be configured through S_FMT. Other than that, all other options (width, height, pixelcode) are fixed for a given mbus format *for the ISP driver*. Other drivers might support different pixel codes for a given mbus code (with different padding and/or endianness). Application either need to be aware of the media controller framework, in which case they will know how to deal with mbus formats and pixel formats, or need to be run after an external application takes care of pipeline configuration. In the second case I suppose it's reasonable to assume that no application will touch the pipeline while the pure V4L2 runs. In that case I think your implementation of ENUM_FMT makes sense.
Hi Sakari, On 03/24/2011 09:04 AM, Sakari Ailus wrote: > Michael Jones wrote: >> Hi Sakari, > > Hi Michael, > >> On 03/23/2011 10:52 AM, Sakari Ailus wrote: >>> Hi Michael, >>> >>> Thanks for the patch. >>> >>> Michael Jones wrote: >>>> From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001 >>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>> Date: Tue, 22 Mar 2011 11:47:22 +0100 >>>> Subject: [PATCH] omap3isp: implement ENUM_FMT >>>> >>>> Whatever format is currently being delivered will be declared as the only >>>> possible format >>>> >>>> Signed-off-by: Michael Jones <michael.jones@matrix-vision.de> >>>> --- >>>> >>>> Some V4L2 apps require ENUM_FMT, which is a mandatory ioctl for V4L2. >>>> This patch doesn't enumerate all of the formats which could possibly be >>>> set (as is intended by ENUM_FMT), but at least it reports the one that >>>> is currently set. >>> >>> What would be the purpose of ENUM_FMT in this case? It provides no >>> additional information to user space, and the information it provides is >>> in fact incomplete. Using other formats is possible, but that requires >>> changes to the format configuration on links. >> >> The only purpose of it was to provide minimum functionality for apps to >> be able to fetch frames from the ISP after setting up the ISP pipeline >> with media-ctl. By "apps", I mean Gstreamer in my case, which Loïc had >> also recently asked Laurent about. > > Are you familiar with the mcsrc? No, I was not familiar with that, thank you. That looks very useful, I'll have to look into it. > > <URL:http://meego.gitorious.org/maemo-multimedia/gst-nokia-videosrc> > > I think it's > > Also, I think v4lsrc could be fixed to cope with lack of VIDIOC_ENUM_FMT. Yes, it could be. But ENUM_FMT is allowed and I think it's reasonable for a V4L2 app to require the use of ENUM_FMT. > > Besides what Laurent already mentioned, how the applications are > expected to select a video node from the 7 (or 8) that the ISP has is > not resolved yet. The requested pixelformat depends on the video node in > this case. > >>> As the relevant format configuration is done on the subdevs and not on >>> the video nodes, the format configuration on the video nodes is very >>> limited and much affected by the state of the formats on the subdev pads >>> (which I think is right). This is not limited to ENUM_FMT but all format >>> related IOCTLs on the OMAP 3 ISP driver. >>> >>> My view is that should a generic application want to change (or >>> enumerate) the format(s) on a video node, the application would need to >>> be using libv4l for that. >>> >>> A compatibility layer implemented in libv4l (plugin, not the main >>> library) needs to configure the links in the first place, so >>> implementing ENUM_FMT in the plugin would not be a big deal. It could >>> even provide useful information. The possible results of the ENUM_FMT >>> would also depend on what kind of pipeline configuration does the plugin >>> support, though. >> >> BTW, my GStreamer is using libv4l, although it looked like it's also >> possible to configure GStreamer to use ioctls directly. I can agree >> that it would be nice to implement ENUM_FMT and the like in a >> compatibility layer in libv4l. That would be in the true spirit of >> ENUM_FMT, where the app could actually see different formats it can set. >> >> But is there any work being done on such a compatibility layer? > > Yes. Yordan is working on that. > > There's a plugin interface patch for libv4l and a MC-aware plugin that > uses the interface. > > <URL:http://www.spinics.net/lists/linux-media/msg28925.html> > > I think so far only the plugin interface patch is on the list. > >> Is there a policy decision that in the future, apps will be required to >> use libv4l to get images from the ISP? Are we not intending to support >> using e.g. media-ctl + some v4l2 app, as I'm currently doing during >> development? > > If we speak about specific applications that are aware of the MC and the > OMAP 3 ISP itself, I'd guess those will continue to use the interfaces > provided by the driver directly. > > But generic applications would likely be better off using libv4l. This > way they could use whatever there is in the libv4l plugin for the > platform (automatic exposure and white balance, for example). > > I don't think generic applications _shouldn't_ be allowed to do without > libv4l. It's just that the functionality provided by the driver in this > case will be limited to a default pipeline and fixed format (as was your > patch for ENUM_FMT). > >> In the meantime, I will continue using this patch locally to enable >> getting a live image with Gstreamer, and it can at least serve as a help >> to Loïc if he's trying to do the same. > > Are there other applications that really need ENUM_FMT? I don't know of others, but surely the same would apply to them as to GStreamer- you _could_ change them so they don't need it. But they conform to the V4L2 spec as is. > > The spec says of ENUM_FMT that: > > "To enumerate image formats applications initialize the type and index > field of struct v4l2_fmtdesc and call the VIDIOC_ENUM_FMT ioctl with a > pointer to this structure. Drivers fill the rest of the structure or > return an EINVAL error code. All formats are enumerable by beginning at > index zero and incrementing by one until EINVAL is returned." > > As Laurent pointed out, I also agree that the patch itself is correct > but the information provided to the user space is correct but > incomplete. Coming up with a list of formats on a video node in the ISP > requires information from different parts of the MC graph. I realize that. > > I somehow feel it shouldn't be the responsibility of the driver to > provide this since using those formats requires different pad formats. > > Various parts of the driver would need to be notified when links change > somewhere else. Also, if a video node isn't connected to anything > through an enabled link or the entity it's connected to, isn't, what > should ENUM_FMT provide then? An empty list? Good quesion, I guess -EINVAL. At that point you're talking about a V4L device which can't provide data anyway. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
Hi Laurent, On 03/24/2011 11:36 AM, Laurent Pinchart wrote: > Hi, > [snip] > > Padding at end of line can be configured through S_FMT. Other than that, all > other options (width, height, pixelcode) are fixed for a given mbus format > *for the ISP driver*. Other drivers might support different pixel codes for a > given mbus code (with different padding and/or endianness). > > Application either need to be aware of the media controller framework, in > which case they will know how to deal with mbus formats and pixel formats, or > need to be run after an external application takes care of pipeline > configuration. In the second case I suppose it's reasonable to assume that no > application will touch the pipeline while the pure V4L2 runs. In that case I > think your implementation of ENUM_FMT makes sense. > It is this second case which I am currently using, and why I submitted this patch. I think supporting ENUM_FMT in some form at /dev/videoX makes sense unless this second case is deemed obsolete and unsupported. But then that seems like a bigger break from V4L. -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner -- 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
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c index a0bb5db..8e25f47 100644 --- a/drivers/media/video/omap3isp/ispvideo.c +++ b/drivers/media/video/omap3isp/ispvideo.c @@ -642,6 +642,28 @@ isp_video_get_format(struct file *file, void *fh, struct v4l2_format *format) } static int +isp_video_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *fmtdesc) +{ + struct isp_video_fh *vfh = to_isp_video_fh(fh); + struct isp_video *video = video_drvdata(file); + + if (fmtdesc->index) + return -EINVAL; + + if (fmtdesc->type != video->type) + return -EINVAL; + + fmtdesc->flags = 0; + fmtdesc->description[0] = 0; + + mutex_lock(&video->mutex); + fmtdesc->pixelformat = vfh->format.fmt.pix.pixelformat; + mutex_unlock(&video->mutex); + + return 0; +} + +static int isp_video_set_format(struct file *file, void *fh, struct v4l2_format *format) { struct isp_video_fh *vfh = to_isp_video_fh(fh); @@ -1059,6 +1081,7 @@ isp_video_s_input(struct file *file, void *fh, unsigned int input) static const struct v4l2_ioctl_ops isp_video_ioctl_ops = { .vidioc_querycap = isp_video_querycap, + .vidioc_enum_fmt_vid_cap = isp_video_enum_format, .vidioc_g_fmt_vid_cap = isp_video_get_format, .vidioc_s_fmt_vid_cap = isp_video_set_format, .vidioc_try_fmt_vid_cap = isp_video_try_format,