Skip to contentSkip to conversation

omap3isp: implement ENUM_FMT

PATCH In review Michael Jones 4 participants 11 messages Mar 22, 2011 – Mar 25, 2011 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 omap3isp: implement ENUM_FMT

Quoted 1 line

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001

From: Michael Jones <[email protected]>
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

---

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(-)
drivers/media/video/omap3isp/ispvideo.c +23 -0
index a0bb5db..8e25f47 100644
@@ -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,
Signature · 6 lines
1.7.4.1


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

Hi Michael,

Thanks for the patch.

Quoted 8 lines · Michael Jones

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones <[email protected]>
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

Quoted 7 lines

---

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,

Signature · 2 lines
Sakari Ailus
[email protected]
#2 lore raw

Hi Sakari,

Quoted 21 lines · Sakari Ailus

Hi Michael,

Thanks for the patch.

Quoted 8 lines · Michael Jones

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones <[email protected]>
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

Quoted 7 lines

---

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.

Quoted 4 lines

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.

Quoted 11 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

(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

#3 lore raw

Hi Michael,

Quoted 51 lines · On Wednesday 23 March 2011 12:07:12 Michael Jones wrote:
Quoted 17 lines · Sakari Ailus
Quoted 8 lines · Michael Jones

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones <[email protected]>
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

Quoted 7 lines

---

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.

Quoted 4 lines

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.

Quoted 10 lines

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.

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Signature · 3 lines
Regards,

Laurent Pinchart
#4 lore raw

Hi Laurent,

Quoted 2 lines · Laurent Pinchart

Hi Michael,

[snip]

Quoted 12 lines
Quoted 5 lines

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.

Quoted 5 lines

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

#5 lore raw
Quoted 17 lines · On Thursday, March 24, 2011 08:28:31 Michael Jones wrote:

Hi Laurent,

Quoted 2 lines · Laurent Pinchart

Hi Michael,

[snip]

Quoted 11 lines
Quoted 5 lines

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.

Quoted 10 lines
Quoted 5 lines

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

#6 lore raw
Quoted 13 lines · Hans Verkuil
Quoted 12 lines · On Thursday, March 24, 2011 08:28:31 Michael Jones wrote:

Hi Laurent,

Quoted 2 lines · Laurent Pinchart

Hi Michael,

[snip]

Quoted 6 lines
Quoted 5 lines

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?

Quoted 8 lines
Quoted 5 lines
Quoted 5 lines

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.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 5 lines

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,

Signature · 2 lines
Sakari Ailus
[email protected]
#7 lore raw

Hi,

Quoted 18 lines · On Thursday 24 March 2011 09:13:20 Sakari Ailus wrote:
Quoted 17 lines · Hans Verkuil
Quoted 15 lines · On Thursday, March 24, 2011 08:28:31 Michael Jones wrote:
Quoted 1 line · Laurent Pinchart

Hi Michael,

[snip]

Quoted 10 lines
Quoted 4 lines

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.

Quoted 15 lines
Quoted 2 lines

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.

Quoted 10 lines
Quoted 10 lines
Quoted 4 lines

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.

Quoted 20 lines
Quoted 2 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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.

Signature · 3 lines
Regards,

Laurent Pinchart
#8 lore raw

Hi Laurent,

Quoted 2 lines · Laurent Pinchart

Hi,

[snip]

Quoted 6 lines

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

Quoted 7 lines

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

#9 lore raw
In reply to Michael Jones
Quoted 1 line · Michael Jones

Hi Sakari,

Hi Michael,

Quoted 27 lines · Sakari Ailus
Quoted 21 lines

Hi Michael,

Thanks for the patch.

Quoted 8 lines · Michael Jones

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones <[email protected]>
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

Quoted 7 lines

---

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.

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 17 lines
Quoted 10 lines

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.

Quoted 6 lines

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.

Quoted 7 lines

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.

Quoted 4 lines

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

Quoted 3 lines

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,

Signature · 2 lines
Sakari Ailus
[email protected]
#10 lore raw

Hi Sakari,

Quoted 32 lines · Sakari Ailus
Quoted 1 line · Michael Jones

Hi Sakari,

Hi Michael,

Quoted 27 lines · Sakari Ailus
Quoted 25 lines

Hi Michael,

Thanks for the patch.

Quoted 8 lines · Michael Jones

From dccbd4a0a717ee72a3271075b1e3456a9c67ca0e Mon Sep 17 00:00:00 2001
From: Michael Jones <[email protected]>
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

Quoted 7 lines

---

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.

Quoted 6 lines
Quoted 4 lines

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.

Quoted 6 lines

<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.

Quoted 58 lines

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.

Quoted 17 lines
Quoted 10 lines

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.

Quoted 6 lines

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.

Quoted 7 lines

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.

Quoted 4 lines

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

Quoted 5 lines
Quoted 3 lines

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.

Quoted 9 lines

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."

Quoted 4 lines

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.

Quoted 8 lines

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