* [REVIEW PATCHv1 2/2] DocBook: fix various validation errors
2013-01-07 12:09 ` [REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation Hans Verkuil
@ 2013-01-07 12:09 ` Hans Verkuil
2013-01-07 19:56 ` [REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation Laurent Pinchart
1 sibling, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2013-01-07 12:09 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Fixed the following errors (with exception of the SVG errors):
GEN /home/hans/work/src/v4l/media-git/Documentation/DocBook//v4l2.xml
rm -rf Documentation/DocBook/index.html; echo '<h1>Linux Kernel HTML Documentation</h1>' >> Documentation/DocBook/index.html && echo '<h2>Kernel Version: 3.8.0-rc1</h2>' >> Documentation/DocBook/index.html && cat Documentation/DocBook/media_api.html >> Documentation/DocBook/index.html
/tmp/x.xml:883: element revremark: validity error : Element structname is not declared in revremark list of possible children
/tmp/x.xml:883: element revremark: validity error : Element xref is not declared in revremark list of possible children
/tmp/x.xml:1829: element footnote: validity error : Element footnote content does not follow the DTD, expecting (calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable)+, got (para para errorcode CDATA para )
/tmp/x.xml:9580: element xref: validity error : Element xref was declared EMPTY this one has content
/tmp/x.xml:13508: element link: validity error : Element link does not carry attribute linkend
/tmp/x.xml:13508: element link: validity error : No declaration for attribute linked of element link
/tmp/x.xml:16986: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:17003: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:17022: element imagedata: validity error : Value "SVG" for attribute format of imagedata is not among the enumerated set
/tmp/x.xml:26795: element refsect1: validity error : Element refsect1 content does not follow the DTD, expecting (refsect1info? , (title , subtitle? , titleabbrev?) , (((calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | caution | important | note | tip | warning | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable | equation | example | figure | table | msgset | procedure | sidebar | qandaset | anchor | bridgehead | remark | highlights | abstract | authorblurb | epigraph | indexterm | beginpage)+ , refsect2*) | refsect2+)), got (section )
/tmp/x.xml:26852: element refsect1: validity error : Element refsect1 content does not follow the DTD, expecting (refsect1info? , (title , subtitle? , titleabbrev?) , (((calloutlist | glosslist | itemizedlist | orderedlist | segmentedlist | simplelist | variablelist | caution | important | note | tip | warning | literallayout | programlisting | programlistingco | screen | screenco | screenshot | synopsis | cmdsynopsis | funcsynopsis | classsynopsis | fieldsynopsis | constructorsynopsis | destructorsynopsis | methodsynopsis | formalpara | para | simpara | address | blockquote | graphic | graphicco | mediaobject | mediaobjectco | informalequation | informalexample | informalfigure | informaltable | equation | example | figure | table | msgset | procedure | sidebar | qandaset | anchor | bridgehead | remark | highlights | abstract | authorblurb | epigraph | indexterm | beginpage)+ , refsect2*) | refsect2+)), got (table )
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
Documentation/DocBook/media/v4l/common.xml | 2 +-
Documentation/DocBook/media/v4l/io.xml | 4 +--
.../DocBook/media/v4l/pixfmt-srggb10alaw8.xml | 2 +-
Documentation/DocBook/media/v4l/v4l2.xml | 5 +---
Documentation/DocBook/media/v4l/vidioc-expbuf.xml | 28 +++++++++-----------
5 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml
index 73c6847..ae06afb 100644
--- a/Documentation/DocBook/media/v4l/common.xml
+++ b/Documentation/DocBook/media/v4l/common.xml
@@ -609,7 +609,7 @@ to zero and the <constant>VIDIOC_G_STD</constant>,
<para>Applications can make use of the <xref linkend="input-capabilities" /> and
<xref linkend="output-capabilities"/> flags to determine whether the video standard ioctls
are available for the device.</para>
-&ENOTTY;.
+
<para>See <xref linkend="buffer" /> for a rationale. Probably
even USB cameras follow some well known video standard. It might have
been better to explicitly indicate elsewhere if a device cannot live
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c4646d..e6c5855 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -477,7 +477,7 @@ rest should be evident.</para>
<note>
<title>Experimental</title>
- <para>This is an <link linkend="experimental"> experimental </link>
+ <para>This is an <link linkend="experimental">experimental</link>
interface and may change in the future.</para>
</note>
@@ -488,7 +488,7 @@ DMA buffer from userspace using a file descriptor previously exported for a
different or the same device (known as the importer role), or both. This
section describes the DMABUF importer role API in V4L2.</para>
- <para>Refer to <link linked="vidioc-expbuf"> DMABUF exporting </link> for
+ <para>Refer to <link linkend="vidioc-expbuf">DMABUF exporting</link> for
details about exporting V4L2 buffers as DMABUF file descriptors.</para>
<para>Input and output devices support the streaming I/O method when the
diff --git a/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml b/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
index c934192..29acc20 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-srggb10alaw8.xml
@@ -29,6 +29,6 @@
formats with 10 bits per color compressed to 8 bits each,
using the A-LAW algorithm. Each color component consumes 8
bits of memory. In other respects this format is similar to
- <xref linkend="V4L2-PIX-FMT-SRGGB8">.</xref></para>
+ <xref linkend="V4L2-PIX-FMT-SRGGB8"></xref>.</para>
</refsect1>
</refentry>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 8fe2942..94ab0e1 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -143,10 +143,7 @@ applications. -->
<revnumber>3.9</revnumber>
<date>2012-12-03</date>
<authorinitials>sa</authorinitials>
- <revremark>Added timestamp types to
- <structname>v4l2_buffer</structname>, see <xref
- linkend="buffer-flags" />.
- </revremark>
+ <revremark>Added timestamp types to v4l2_buffer.</revremark>
</revision>
<revision>
diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
index 72dfbd2..e287c8f 100644
--- a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
@@ -83,15 +83,14 @@ descriptor. The application may pass it to other DMABUF-aware devices. Refer to
<link linkend="dmabuf">DMABUF importing</link> for details about importing
DMABUF files into V4L2 nodes. It is recommended to close a DMABUF file when it
is no longer used to allow the associated memory to be reclaimed. </para>
-
</refsect1>
+
<refsect1>
- <section>
- <title>Examples</title>
+ <title>Examples</title>
- <example>
- <title>Exporting a buffer.</title>
- <programlisting>
+ <example>
+ <title>Exporting a buffer.</title>
+ <programlisting>
int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
{
&v4l2-exportbuffer; expbuf;
@@ -108,12 +107,12 @@ int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
return 0;
}
- </programlisting>
- </example>
+ </programlisting>
+ </example>
- <example>
- <title>Exporting a buffer using the multi-planar API.</title>
- <programlisting>
+ <example>
+ <title>Exporting a buffer using the multi-planar API.</title>
+ <programlisting>
int buffer_export_mp(int v4lfd, &v4l2-buf-type; bt, int index,
int dmafd[], int n_planes)
{
@@ -137,12 +136,9 @@ int buffer_export_mp(int v4lfd, &v4l2-buf-type; bt, int index,
return 0;
}
- </programlisting>
- </example>
- </section>
- </refsect1>
+ </programlisting>
+ </example>
- <refsect1>
<table pgwide="1" frame="none" id="v4l2-exportbuffer">
<title>struct <structname>v4l2_exportbuffer</structname></title>
<tgroup cols="3">
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation.
2013-01-07 12:09 ` [REVIEW PATCHv1 1/2] DocBook: improve the error_idx field documentation Hans Verkuil
2013-01-07 12:09 ` [REVIEW PATCHv1 2/2] DocBook: fix various validation errors Hans Verkuil
@ 2013-01-07 19:56 ` Laurent Pinchart
2013-01-11 11:48 ` Hans Verkuil
1 sibling, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2013-01-07 19:56 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Hans Verkuil
Hi Hans,
Thanks for the patch.
On Monday 07 January 2013 13:09:47 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The documentation of the error_idx field was incomplete and confusing.
> This patch improves it.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 51 ++++++++++++++---
> 1 file changed, 44 insertions(+), 7 deletions(-)
(Text reflowed for easier review)
> diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml index
> 0a4b90f..c07c657 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> @@ -199,13 +199,50 @@ also be zero.</entry>
> <row>
> <entry>__u32</entry>
> <entry><structfield>error_idx</structfield></entry>
> - <entry>Set by the driver in case of an error. If it is equal
> -to <structfield>count</structfield>, then no actual changes were made to
> -controls. In other words, the error was not associated with setting a
> -particular control. If it is another value, then only the controls up to
> -<structfield>error_idx-1</structfield> were modified and control
> -<structfield>error_idx</structfield> is the one that caused the error. The
> -<structfield>error_idx</structfield> value is undefined if the ioctl
> -returned 0 (success).</entry>
> + <entry><para>Set by the driver in case of an error. If the error is
> +associated with a particular control, then
> +<structfield>error_idx</structfield> is set to the index of that control.
> +If the error is not related to a specific control, then
> +<structfield>error_idx</structfield> is set to
> +<structfield>count</structfield>.</para>
> +<para>The behavior is different for <constant>VIDIOC_G_EXT_CTRLS</constant>
> +and <constant>VIDIOC_S_EXT_CTRLS</constant>: if
> +<structfield>error_idx</structfield> is equal to
> +<structfield>count</structfield>, then no actual changes were made to the
> +controls. For example, if you try to write to a read-only control, then
> +<constant>VIDIOC_TRY_EXT_CTRLS</constant> will set
> +<structfield>error_idx</structfield> to the index of that read-only
> +control, but <constant>VIDIOC_S_EXT_CTRLS</constant> will set
> +<structfield>error_idx</structfield> to <structfield>count</structfield>
> +instead and none of the controls in the list will be set.</para>
I find this a bit confusing (although I understand what you mean, as I'm
familiar with the API). You start by mentioning [GS]_EXT_CTRLS only, and then
you introduce TRY_EXT_CTRLS in the example. I think it would be clearer if you
restructed the explanation as follows:
- explain the error_idx principle globally, withotu examples
- explain that [GS]_EXT_CTRLS will perform pre-validation and will thus set
error_idx to count in specific cases (they should be listed)
- explain that TRY_EXT_CTRLS doesn't perform pre-validation and will thus set
error_idx to the index of the erroneous control in all cases, including the
specific cases listed for [GS]_EXT_CTRLS
> +<para>The same is true when trying to e.g. read a write-only control:
> +<constant>VIDIOC_G_EXT_CTRLS</constant> will set
> +<structfield>error_idx</structfield> to <structfield>count</structfield>
> +and none of the controls in the list will have been retrieved.</para>
> +
> +<para>The reason for this behavior is that it is important when setting and
> +getting controls to do this as atomically as possible, so simple sanity
> +checks like testing for read-only controls are done first before an
> +attempt is made to apply/retrieve the new control values to/from the
> +hardware. It is important for an application to know whether
> +<constant>VIDIOC_S_EXT_CTRLS</constant> or
> +<constant>VIDIOC_G_EXT_CTRLS</constant> actually made changes to controls
> +or not. So if <structfield>error_idx</structfield> is equal to
> +<structfield>count</structfield>, then you know that no actual controls
> +were set or retrieved. In the case of
> +<constant>VIDIOC_S_EXT_CTRLS</constant> you can call
> +<constant>VIDIOC_TRY_EXT_CTRLS</constant> with the same control list to
> +see if the problem was with a specific control or not
> +(<constant>VIDIOC_TRY_EXT_CTRLS</constant> never modifies controls, so
> +that ioctl will just set <structfield>error_idx</structfield> to the
> +control that caused the problem). No such option exists for
> +<constant>VIDIOC_G_EXT_CTRLS</constant> though, unfortunately.</para>
It's very unfortunate indeed :-) Given that the hardware state must not be
changed by a read operation, are you sure G_EXT_CTRLS couldn't retrieve
controls up to the erroneous one ? ;-)
I think some flexibility should still probably be left to drivers (and I'm not
talking about UVC here), as some drivers might not be able to know that a
control is write-only before trying to read it and getting an error.
> +<para>If <structfield>error_idx</structfield> as returned by
> +<constant>VIDIOC_S_EXT_CTRLS</constant> or
> +<constant>VIDIOC_G_EXT_CTRLS</constant> is less than
> +<structfield>count</structfield>, then only the controls up to
> +<structfield>error_idx-1</structfield> were modified and control
> +<structfield>error_idx</structfield> is the one that caused the error. In
> +the case of <constant>VIDIOC_S_EXT_CTRLS</constant> this might have left
> +the hardware in an inconsistent state. These types of errors should not
> +normally happen, and such errors are typically caused by problems in
> +communicating with the hardware.</para>
> +
> +<para>Finally, note that the <structfield>error_idx</structfield> value is
> +undefined if the ioctl returned 0 (success).</para></entry>
> </row>
> <row>
> <entry>__u32</entry>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread