[PATCH] omap3isp: implement ENUM_FMT

[PATCH] omap3isp: implement ENUM_FMT

[Michael Jones]

>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(-)

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,
-- 
1.7.4.1


MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Re: [PATCH] omap3isp: implement ENUM_FMT

[Sakari Ailus]

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,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [PATCH] omap3isp: implement ENUM_FMT

[Michael Jones]

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

Re: [PATCH] omap3isp: implement ENUM_FMT

[Laurent Pinchart]

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.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] omap3isp: implement ENUM_FMT

[Michael Jones]

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

Re: [PATCH] omap3isp: implement ENUM_FMT

[Hans Verkuil]

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

Re: [PATCH] omap3isp: implement ENUM_FMT

[Sakari Ailus]

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,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [PATCH] omap3isp: implement ENUM_FMT

[Laurent Pinchart]

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.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] omap3isp: implement ENUM_FMT

[Michael Jones]

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

Re: [PATCH] omap3isp: implement ENUM_FMT

[Sakari Ailus]

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,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: [PATCH] omap3isp: implement ENUM_FMT

[Michael Jones]

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